From 5916cf1e0ace5d9855af4d785c22c742cf106b8a Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 13 Feb 2018 09:49:50 +0100 Subject: Allow `this.f.selector` to be pure. --- Changelog.md | 1 + libsolidity/analysis/ViewPureChecker.cpp | 16 ++++++++ libsolidity/analysis/ViewPureChecker.h | 1 + libsolidity/codegen/ExpressionCompiler.cpp | 24 +++++++++++ test/libsolidity/SolidityNameAndTypeResolution.cpp | 10 +---- test/libsolidity/ViewPureChecker.cpp | 46 ++++++++++++++++++++++ 6 files changed, 89 insertions(+), 9 deletions(-) diff --git a/Changelog.md b/Changelog.md index db425291..44e35f5f 100644 --- a/Changelog.md +++ b/Changelog.md @@ -11,6 +11,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: * Parser: Disallow event declarations with no parameter list. diff --git a/libsolidity/analysis/ViewPureChecker.cpp b/libsolidity/analysis/ViewPureChecker.cpp index 6257ac6d..13c3ab68 100644 --- a/libsolidity/analysis/ViewPureChecker.cpp +++ b/libsolidity/analysis/ViewPureChecker.cpp @@ -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.expression())) + if (auto const* exprInt = dynamic_cast(&expr->expression())) + if (exprInt->name() == "this") + // Do not continue visiting. + return false; + return true; +} + void ViewPureChecker::endVisit(MemberAccess const& _memberAccess) { StateMutability mutability = StateMutability::Pure; diff --git a/libsolidity/analysis/ViewPureChecker.h b/libsolidity/analysis/ViewPureChecker.h index fec060b6..0b882cd8 100644 --- a/libsolidity/analysis/ViewPureChecker.h +++ b/libsolidity/analysis/ViewPureChecker.h @@ -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; diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp index bb8c4a94..8e1cf019 100644 --- a/libsolidity/codegen/ExpressionCompiler.cpp +++ b/libsolidity/codegen/ExpressionCompiler.cpp @@ -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.expression())) + if (auto const* exprInt = dynamic_cast(&expr->expression())) + if (exprInt->name() == "this") + if (Declaration const* declaration = expr->annotation().referencedDeclaration) + { + u256 identifier; + if (auto const* variable = dynamic_cast(declaration)) + identifier = FunctionType(*variable).externalIdentifier(); + else if (auto const* function = dynamic_cast(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()) diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 1953195c..3f72e847 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -6904,15 +6904,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; } } diff --git a/test/libsolidity/ViewPureChecker.cpp b/test/libsolidity/ViewPureChecker.cpp index e91e713c..0fee95c5 100644 --- a/test/libsolidity/ViewPureChecker.cpp +++ b/test/libsolidity/ViewPureChecker.cpp @@ -326,6 +326,52 @@ BOOST_AUTO_TEST_CASE(function_types) CHECK_SUCCESS_NO_WARNINGS(text); } +BOOST_AUTO_TEST_CASE(selector) +{ + string text = R"( + contract C { + function f() payable public { + } + function g() pure public returns (bytes4) { + return this.f.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"( -- cgit