From 7c7c2baa82ec2fa0535381c2ea3418b8623a9062 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 27 Jul 2017 11:52:42 +0200 Subject: [PATCH] Re-allow multiple modifiers per function. --- Changelog.md | 1 + libsolidity/analysis/TypeChecker.cpp | 2 -- libsolidity/codegen/CompilerContext.cpp | 12 ++++---- libsolidity/codegen/CompilerContext.h | 5 +++- test/libsolidity/SolidityEndToEndTest.cpp | 28 +++++++++++++++++++ .../SolidityNameAndTypeResolution.cpp | 2 +- 6 files changed, 41 insertions(+), 9 deletions(-) diff --git a/Changelog.md b/Changelog.md index e765b583c..ebd742881 100644 --- a/Changelog.md +++ b/Changelog.md @@ -14,6 +14,7 @@ Features: Bugfixes: * Type Checker: Fix invalid "specify storage keyword" warning for reference members of structs. * Type Checker: Mark modifiers as internal. + * Type Checker: Re-allow multiple mentions of the same modifier per function. ### 0.4.13 (2017-07-06) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 23f01752b..0d04c26d6 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -510,8 +510,6 @@ bool TypeChecker::visit(FunctionDefinition const& _function) { if (dynamic_cast(decl)) m_errorReporter.declarationError(modifier->location(), "Base constructor already provided."); - else - m_errorReporter.declarationError(modifier->location(), "Modifier already used for this function."); } else modifiers.insert(decl); diff --git a/libsolidity/codegen/CompilerContext.cpp b/libsolidity/codegen/CompilerContext.cpp index 9aaf5844e..97b515423 100644 --- a/libsolidity/codegen/CompilerContext.cpp +++ b/libsolidity/codegen/CompilerContext.cpp @@ -124,14 +124,15 @@ void CompilerContext::addVariable(VariableDeclaration const& _declaration, unsigned _offsetToCurrent) { solAssert(m_asm->deposit() >= 0 && unsigned(m_asm->deposit()) >= _offsetToCurrent, ""); - solAssert(m_localVariables.count(&_declaration) == 0, "Variable already present"); - m_localVariables[&_declaration] = unsigned(m_asm->deposit()) - _offsetToCurrent; + m_localVariables[&_declaration].push_back(unsigned(m_asm->deposit()) - _offsetToCurrent); } void CompilerContext::removeVariable(VariableDeclaration const& _declaration) { - solAssert(!!m_localVariables.count(&_declaration), ""); - m_localVariables.erase(&_declaration); + solAssert(m_localVariables.count(&_declaration) && !m_localVariables[&_declaration].empty(), ""); + m_localVariables[&_declaration].pop_back(); + if (m_localVariables[&_declaration].empty()) + m_localVariables.erase(&_declaration); } eth::Assembly const& CompilerContext::compiledContract(const ContractDefinition& _contract) const @@ -204,7 +205,8 @@ unsigned CompilerContext::baseStackOffsetOfVariable(Declaration const& _declarat { auto res = m_localVariables.find(&_declaration); solAssert(res != m_localVariables.end(), "Variable not found on stack."); - return res->second; + solAssert(!res->second.empty(), ""); + return res->second.back(); } unsigned CompilerContext::baseToCurrentStackOffset(unsigned _baseOffset) const diff --git a/libsolidity/codegen/CompilerContext.h b/libsolidity/codegen/CompilerContext.h index 1968c1e1c..13821f67d 100644 --- a/libsolidity/codegen/CompilerContext.h +++ b/libsolidity/codegen/CompilerContext.h @@ -272,7 +272,10 @@ private: /// Storage offsets of state variables std::map> m_stateVariables; /// Offsets of local variables on the stack (relative to stack base). - std::map m_localVariables; + /// This needs to be a stack because if a modifier contains a local variable and this + /// modifier is applied twice, the position of the variable needs to be restored + /// after the nested modifier is left. + std::map> m_localVariables; /// List of current inheritance hierarchy from derived to base. std::vector m_inheritanceHierarchy; /// Stack of current visited AST nodes, used for location attachment diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 94d3e168a..5bcde4411 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -2696,6 +2696,34 @@ BOOST_AUTO_TEST_CASE(function_modifier_for_constructor) BOOST_CHECK(callContractFunction("getData()") == encodeArgs(4 | 2)); } +BOOST_AUTO_TEST_CASE(function_modifier_multiple_times) +{ + char const* sourceCode = R"( + contract C { + uint public a; + modifier mod(uint x) { a += x; _; } + function f(uint x) mod(2) mod(5) mod(x) returns(uint) { return a; } + } + )"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("f(uint256)", u256(3)) == encodeArgs(2 + 5 + 3)); + BOOST_CHECK(callContractFunction("a()") == encodeArgs(2 + 5 + 3)); +} + +BOOST_AUTO_TEST_CASE(function_modifier_multiple_times_local_vars) +{ + char const* sourceCode = R"( + contract C { + uint public a; + modifier mod(uint x) { uint b = x; a += b; _; a -= b; assert(b == x); } + function f(uint x) mod(2) mod(5) mod(x) returns(uint) { return a; } + } + )"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("f(uint256)", u256(3)) == encodeArgs(2 + 5 + 3)); + BOOST_CHECK(callContractFunction("a()") == encodeArgs(0)); +} + BOOST_AUTO_TEST_CASE(crazy_elementary_typenames_on_stack) { char const* sourceCode = R"( diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 48fe4d242..accf86c63 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -1115,7 +1115,7 @@ BOOST_AUTO_TEST_CASE(function_modifier_double_invocation) modifier mod(uint a) { if (a > 0) _; } } )"; - CHECK_ERROR(text, DeclarationError, "Modifier already used for this function"); + success(text); } BOOST_AUTO_TEST_CASE(base_constructor_double_invocation)