Merge pull request #3498 from ethereum/allowthisfselector

Allow `this.f.selector` to be pure.
This commit is contained in:
chriseth 2018-02-13 17:08:35 +01:00 committed by GitHub
commit 23484ba6a4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 98 additions and 12 deletions

View File

@ -13,6 +13,7 @@ Features:
variables.
* Syntax Checker: Deprecate the ``var`` keyword (and mark it an error as experimental 0.5.0 feature).
* Type Checker: Issue warning for using ``public`` visibility for interface functions.
* Type Checker: Allow `this.f.selector` to be a pure expression.
Bugfixes:
* Error Output: Truncate huge number literals in the middle to avoid output blow-up.

View File

@ -275,6 +275,22 @@ void ViewPureChecker::endVisit(FunctionCall const& _functionCall)
reportMutability(mut, _functionCall.location());
}
bool ViewPureChecker::visit(MemberAccess const& _memberAccess)
{
// Catch the special case of `this.f.selector` which is a pure expression.
ASTString const& member = _memberAccess.memberName();
if (
_memberAccess.expression().annotation().type->category() == Type::Category::Function &&
member == "selector"
)
if (auto const* expr = dynamic_cast<MemberAccess const*>(&_memberAccess.expression()))
if (auto const* exprInt = dynamic_cast<Identifier const*>(&expr->expression()))
if (exprInt->name() == "this")
// Do not continue visiting.
return false;
return true;
}
void ViewPureChecker::endVisit(MemberAccess const& _memberAccess)
{
StateMutability mutability = StateMutability::Pure;

View File

@ -56,6 +56,7 @@ private:
virtual bool visit(ModifierDefinition const& _modifierDef) override;
virtual void endVisit(ModifierDefinition const& _modifierDef) override;
virtual void endVisit(Identifier const& _identifier) override;
virtual bool visit(MemberAccess const& _memberAccess) override;
virtual void endVisit(MemberAccess const& _memberAccess) override;
virtual void endVisit(IndexAccess const& _indexAccess) override;
virtual void endVisit(ModifierInvocation const& _modifier) override;

View File

@ -1014,6 +1014,30 @@ bool ExpressionCompiler::visit(MemberAccess const& _memberAccess)
_memberAccess.expression().accept(*this);
return false;
}
// Another special case for `this.f.selector` which does not need the address.
// There are other uses of `.selector` which do need the address, but we want this
// specific use to be a pure expression.
if (
_memberAccess.expression().annotation().type->category() == Type::Category::Function &&
member == "selector"
)
if (auto const* expr = dynamic_cast<MemberAccess const*>(&_memberAccess.expression()))
if (auto const* exprInt = dynamic_cast<Identifier const*>(&expr->expression()))
if (exprInt->name() == "this")
if (Declaration const* declaration = expr->annotation().referencedDeclaration)
{
u256 identifier;
if (auto const* variable = dynamic_cast<VariableDeclaration const*>(declaration))
identifier = FunctionType(*variable).externalIdentifier();
else if (auto const* function = dynamic_cast<FunctionDefinition const*>(declaration))
identifier = FunctionType(*function).externalIdentifier();
else
solAssert(false, "Contract member is neither variable nor function.");
m_context << identifier;
/// need to store store it as bytes4
utils().leftShiftNumberOnStack(224);
return false;
}
_memberAccess.expression().accept(*this);
switch (_memberAccess.expression().annotation().type->category())

View File

@ -10219,24 +10219,29 @@ BOOST_AUTO_TEST_CASE(function_types_sig)
{
char const* sourceCode = R"(
contract C {
function f() returns (bytes4) {
uint public x;
function f() pure returns (bytes4) {
return this.f.selector;
}
function g() returns (bytes4) {
function () external returns (bytes4) fun = this.f;
function () pure external returns (bytes4) fun = this.f;
return fun.selector;
}
function h() returns (bytes4) {
function () external returns (bytes4) fun = this.f;
function () pure external returns (bytes4) fun = this.f;
var funvar = fun;
return funvar.selector;
}
function i() pure returns (bytes4) {
return this.x.selector;
}
}
)";
compileAndRun(sourceCode, 0, "C");
ABI_CHECK(callContractFunction("f()"), encodeArgs(asString(FixedHash<4>(dev::keccak256("f()")).asBytes())));
ABI_CHECK(callContractFunction("g()"), encodeArgs(asString(FixedHash<4>(dev::keccak256("f()")).asBytes())));
ABI_CHECK(callContractFunction("h()"), encodeArgs(asString(FixedHash<4>(dev::keccak256("f()")).asBytes())));
ABI_CHECK(callContractFunction("i()"), encodeArgs(asString(FixedHash<4>(dev::keccak256("x()")).asBytes())));
}
BOOST_AUTO_TEST_CASE(constant_string)

View File

@ -6991,15 +6991,7 @@ BOOST_AUTO_TEST_CASE(function_types_sig)
CHECK_ERROR(text, TypeError, "Member \"selector\" not found");
text = R"(
contract C {
function f() view external returns (bytes4) {
return this.f.selector;
}
}
)";
CHECK_SUCCESS_NO_WARNINGS(text);
text = R"(
contract C {
function f() view external returns (bytes4) {
function f() pure external returns (bytes4) {
return this.f.selector;
}
}

View File

@ -326,6 +326,53 @@ BOOST_AUTO_TEST_CASE(function_types)
CHECK_SUCCESS_NO_WARNINGS(text);
}
BOOST_AUTO_TEST_CASE(selector)
{
string text = R"(
contract C {
uint public x;
function f() payable public {
}
function g() pure public returns (bytes4) {
return this.f.selector ^ this.x.selector;
}
}
)";
CHECK_SUCCESS_NO_WARNINGS(text);
}
BOOST_AUTO_TEST_CASE(selector_complex)
{
string text = R"(
contract C {
function f(C c) pure public returns (C) {
return c;
}
function g() pure public returns (bytes4) {
// By passing `this`, we read from the state, even if f itself is pure.
return f(this).f.selector;
}
}
)";
CHECK_ERROR(text, TypeError, "reads from the environment or state and thus requires \"view\"");
}
BOOST_AUTO_TEST_CASE(selector_complex2)
{
string text = R"(
contract C {
function f() payable public returns (C) {
return this;
}
function g() pure public returns (bytes4) {
C x = C(0x123);
return x.f.selector;
}
}
)";
CHECK_SUCCESS_NO_WARNINGS(text);
}
BOOST_AUTO_TEST_CASE(creation)
{
string text = R"(