Re-allow multiple modifiers per function.

6 files changed, 41 insertions, 9 deletions

diff --git a/Changelog.md b/Changelog.md
index e765b583..ebd74288 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 23f01752..0d04c26d 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -510,8 +510,6 @@ bool TypeChecker::visit(FunctionDefinition const& _function)
 		{
 			if (dynamic_cast<ContractDefinition const*>(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 9aaf5844..97b51542 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 1968c1e1..13821f67 100644
--- a/libsolidity/codegen/CompilerContext.h
+++ b/libsolidity/codegen/CompilerContext.h
@@ -272,7 +272,10 @@ private:
 	/// Storage offsets of state variables
 	std::map<Declaration const*, std::pair<u256, unsigned>> m_stateVariables;
 	/// Offsets of local variables on the stack (relative to stack base).
-	std::map<Declaration const*, unsigned> 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<Declaration const*, std::vector<unsigned>> m_localVariables;
 	/// List of current inheritance hierarchy from derived to base.
 	std::vector<ContractDefinition const*> 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 94d3e168..5bcde441 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 48fe4d24..accf86c6 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)