Compilation of function modifiers.

7 files changed, 92 insertions, 41 deletions

diff --git a/AST.cpp b/AST.cpp
index 85d7db6e..cc7da715 100644
--- a/AST.cpp
+++ b/AST.cpp
@@ -224,7 +224,7 @@ string FunctionDefinition::getCanonicalSignature() const
 
 Declaration::LValueType VariableDeclaration::getLValueType() const
 {
-	if (dynamic_cast<FunctionDefinition const*>(getScope()))
+	if (dynamic_cast<FunctionDefinition const*>(getScope()) || dynamic_cast<ModifierDefinition const*>(getScope()))
 		return Declaration::LValueType::LOCAL;
 	else
 		return Declaration::LValueType::STORAGE;
@@ -291,7 +291,8 @@ void Return::checkTypeRequirements()
 {
 	if (!m_expression)
 		return;
-	solAssert(m_returnParameters, "Return parameters not assigned.");
+	if (!m_returnParameters)
+		BOOST_THROW_EXCEPTION(createTypeError("Return arguments not allowed."));
 	if (m_returnParameters->getParameters().size() != 1)
 		BOOST_THROW_EXCEPTION(createTypeError("Different number of arguments in return statement "
 											  "than in returns declaration."));
diff --git a/AST.h b/AST.h
index ded30e9c..e047f976 100755
--- a/AST.h
+++ b/AST.h
@@ -722,12 +722,8 @@ public:
 	virtual void accept(ASTConstVisitor& _visitor) const override;
 	virtual void checkTypeRequirements() override;
 
-	void setFunctionReturnParameters(ParameterList const& _parameters) { m_returnParameters = &_parameters; }
-	ParameterList const& getFunctionReturnParameters() const
-	{
-		solAssert(m_returnParameters, "");
-		return *m_returnParameters;
-	}
+	void setFunctionReturnParameters(ParameterList const* _parameters) { m_returnParameters = _parameters; }
+	ParameterList const* getFunctionReturnParameters() const { return m_returnParameters; }
 	Expression const* getExpression() const { return m_expression.get(); }
 
 private:
diff --git a/Compiler.cpp b/Compiler.cpp
index 5a434a71..fa8eb775 100644
--- a/Compiler.cpp
+++ b/Compiler.cpp
@@ -77,6 +77,7 @@ void Compiler::packIntoContractCreator(ContractDefinition const& _contract, Comp
 	std::vector<ContractDefinition const*> const& bases = _contract.getLinearizedBaseContracts();
 	for (ContractDefinition const* contract: bases)
 	{
+		//TODO include modifiers
 		if (FunctionDefinition const* constructor = contract->getConstructor())
 			nodesUsedInConstructors.insert(constructor);
 		for (ASTPointer<InheritanceSpecifier> const& base: contract->getBaseContracts())
@@ -150,11 +151,7 @@ void Compiler::appendBaseConstructorCall(FunctionDefinition const& _constructor,
 	FunctionType constructorType(_constructor);
 	eth::AssemblyItem returnLabel = m_context.pushNewTag();
 	for (unsigned i = 0; i < _arguments.size(); ++i)
-	{
-		compileExpression(*_arguments[i]);
-		ExpressionCompiler::appendTypeConversion(m_context, *_arguments[i]->getType(),
-												 *constructorType.getParameterTypes()[i]);
-	}
+		compileExpression(*_arguments[i], constructorType.getParameterTypes()[i]);
 	m_context.appendJumpTo(m_context.getFunctionEntryLabel(_constructor));
 	m_context << returnLabel;
 }
@@ -280,20 +277,28 @@ bool Compiler::visit(FunctionDefinition const& _function)
 	m_returnTag = m_context.newTag();
 	m_breakTags.clear();
 	m_continueTags.clear();
+	m_stackCleanupForReturn = 0;
+	m_currentFunction = &_function;
+	m_modifierDepth = 0;
 
 	m_context << m_context.getFunctionEntryLabel(_function);
 
 	// stack upon entry: [return address] [arg0] [arg1] ... [argn]
 	// reserve additional slots: [retarg0] ... [retargm] [localvar0] ... [localvarp]
 
+	unsigned parametersSize = CompilerUtils::getSizeOnStack(_function.getParameters());
+	m_context.adjustStackOffset(parametersSize);
 	for (ASTPointer<VariableDeclaration const> const& variable: _function.getParameters())
-		m_context.addVariable(*variable);
+	{
+		m_context.addVariable(*variable, parametersSize);
+		parametersSize -= variable->getType()->getSizeOnStack();
+	}
 	for (ASTPointer<VariableDeclaration const> const& variable: _function.getReturnParameters())
 		m_context.addAndInitializeVariable(*variable);
 	for (VariableDeclaration const* localVariable: _function.getLocalVariables())
 		m_context.addAndInitializeVariable(*localVariable);
 
-	_function.getBody().accept(*this);
+	appendModifierOrFunctionCode();
 
 	m_context << m_returnTag;
 
@@ -420,13 +425,15 @@ bool Compiler::visit(Return const& _return)
 	//@todo modifications are needed to make this work with functions returning multiple values
 	if (Expression const* expression = _return.getExpression())
 	{
-		compileExpression(*expression);
-		VariableDeclaration const& firstVariable = *_return.getFunctionReturnParameters().getParameters().front();
-		ExpressionCompiler::appendTypeConversion(m_context, *expression->getType(), *firstVariable.getType());
-
+		solAssert(_return.getFunctionReturnParameters(), "Invalid return parameters pointer.");
+		VariableDeclaration const& firstVariable = *_return.getFunctionReturnParameters()->getParameters().front();
+		compileExpression(*expression, firstVariable.getType());
 		CompilerUtils(m_context).moveToStackVariable(firstVariable);
 	}
+	for (unsigned i = 0; i < m_stackCleanupForReturn; ++i)
+		m_context << eth::Instruction::POP;
 	m_context.appendJumpTo(m_returnTag);
+	m_context.adjustStackOffset(m_stackCleanupForReturn);
 	return false;
 }
 
@@ -434,10 +441,7 @@ bool Compiler::visit(VariableDefinition const& _variableDefinition)
 {
 	if (Expression const* expression = _variableDefinition.getExpression())
 	{
-		compileExpression(*expression);
-		ExpressionCompiler::appendTypeConversion(m_context,
-												 *expression->getType(),
-												 *_variableDefinition.getDeclaration().getType());
+		compileExpression(*expression, _variableDefinition.getDeclaration().getType());
 		CompilerUtils(m_context).moveToStackVariable(_variableDefinition.getDeclaration());
 	}
 	return false;
@@ -451,9 +455,53 @@ bool Compiler::visit(ExpressionStatement const& _expressionStatement)
 	return false;
 }
 
-void Compiler::compileExpression(Expression const& _expression)
+bool Compiler::visit(PlaceholderStatement const&)
+{
+	++m_modifierDepth;
+	appendModifierOrFunctionCode();
+	--m_modifierDepth;
+}
+
+void Compiler::appendModifierOrFunctionCode()
+{
+	solAssert(m_currentFunction, "");
+	if (m_modifierDepth >= m_currentFunction->getModifiers().size())
+		m_currentFunction->getBody().accept(*this);
+	else
+	{
+		ASTPointer<ModifierInvocation> const& modifierInvocation = m_currentFunction->getModifiers()[m_modifierDepth];
+
+		// TODO get the most derived override of the modifier
+		ModifierDefinition const* modifier = dynamic_cast<ModifierDefinition const*>(
+												 modifierInvocation->getName()->getReferencedDeclaration());
+		solAssert(!!modifier, "Modifier not found.");
+		solAssert(modifier->getParameters().size() == modifierInvocation->getArguments().size(), "");
+		for (unsigned i = 0; i < modifier->getParameters().size(); ++i)
+		{
+			m_context.addVariable(*modifier->getParameters()[i]);
+			compileExpression(*modifierInvocation->getArguments()[i],
+							  modifier->getParameters()[i]->getType());
+		}
+		for (VariableDeclaration const* localVariable: modifier->getLocalVariables())
+			m_context.addAndInitializeVariable(*localVariable);
+
+		unsigned const c_stackSurplus = CompilerUtils::getSizeOnStack(modifier->getParameters()) +
+										CompilerUtils::getSizeOnStack(modifier->getLocalVariables());
+		m_stackCleanupForReturn += c_stackSurplus;
+
+		modifier->getBody().accept(*this);
+
+		for (unsigned i = 0; i < c_stackSurplus; ++i)
+			m_context << eth::Instruction::POP;
+		m_stackCleanupForReturn -= c_stackSurplus;
+	}
+}
+
+void Compiler::compileExpression(Expression const& _expression, TypePointer const& _targetType)
 {
 	ExpressionCompiler::compileExpression(m_context, _expression, m_optimize);
+	if (_targetType)
+		ExpressionCompiler::appendTypeConversion(m_context, *_expression.getType(), *_targetType);
 }
 
 }
diff --git a/Compiler.h b/Compiler.h
index 2bae6b39..83bee904 100644
--- a/Compiler.h
+++ b/Compiler.h
@@ -31,7 +31,8 @@ namespace solidity {
 class Compiler: private ASTConstVisitor
 {
 public:
-	explicit Compiler(bool _optimize = false): m_optimize(_optimize), m_context(), m_returnTag(m_context.newTag()) {}
+	explicit Compiler(bool _optimize = false): m_optimize(_optimize), m_context(),
+		m_returnTag(m_context.newTag()) {}
 
 	void compileContract(ContractDefinition const& _contract,
 						 std::map<ContractDefinition const*, bytes const*> const& _contracts);
@@ -70,8 +71,13 @@ private:
 	virtual bool visit(Return const& _return) override;
 	virtual bool visit(VariableDefinition const& _variableDefinition) override;
 	virtual bool visit(ExpressionStatement const& _expressionStatement) override;
+	virtual bool visit(PlaceholderStatement const&) override;
 
-	void compileExpression(Expression const& _expression);
+	/// Appends one layer of function modifier code of the current function, or the function
+	/// body itself if the last modifier was reached.
+	void appendModifierOrFunctionCode();
+
+	void compileExpression(Expression const& _expression, TypePointer const& _targetType = TypePointer());
 
 	bool const m_optimize;
 	CompilerContext m_context;
@@ -79,6 +85,9 @@ private:
 	std::vector<eth::AssemblyItem> m_breakTags; ///< tag to jump to for a "break" statement
 	std::vector<eth::AssemblyItem> m_continueTags; ///< tag to jump to for a "continue" statement
 	eth::AssemblyItem m_returnTag; ///< tag to jump to for a "return" statement
+	unsigned m_modifierDepth = 0;
+	FunctionDefinition const* m_currentFunction;
+	unsigned m_stackCleanupForReturn; ///< this number of stack elements need to be removed before jump to m_returnTag
 };
 
 }
diff --git a/CompilerContext.cpp b/CompilerContext.cpp
index 27ec3efd..96d1840e 100644
--- a/CompilerContext.cpp
+++ b/CompilerContext.cpp
@@ -43,10 +43,11 @@ void CompilerContext::addStateVariable(VariableDeclaration const& _declaration)
 	m_stateVariablesSize += _declaration.getType()->getStorageSize();
 }
 
-void CompilerContext::addVariable(VariableDeclaration const& _declaration)
+void CompilerContext::addVariable(VariableDeclaration const& _declaration,
+								  unsigned _offsetToCurrent)
 {
-	m_localVariables[&_declaration] = m_localVariablesSize;
-	m_localVariablesSize += _declaration.getType()->getSizeOnStack();
+	solAssert(m_asm.deposit() >= 0 && unsigned(m_asm.deposit()) >= _offsetToCurrent, "");
+	m_localVariables[&_declaration] = unsigned(m_asm.deposit()) - _offsetToCurrent;
 }
 
 void CompilerContext::addAndInitializeVariable(VariableDeclaration const& _declaration)
@@ -56,7 +57,6 @@ void CompilerContext::addAndInitializeVariable(VariableDeclaration const& _decla
 	int const size = _declaration.getType()->getSizeOnStack();
 	for (int i = 0; i < size; ++i)
 		*this << u256(0);
-	m_asm.adjustDeposit(-size);
 }
 
 void CompilerContext::addFunction(FunctionDefinition const& _function)
@@ -75,7 +75,7 @@ bytes const& CompilerContext::getCompiledContract(const ContractDefinition& _con
 
 bool CompilerContext::isLocalVariable(Declaration const* _declaration) const
 {
-	return m_localVariables.count(_declaration) > 0;
+	return m_localVariables.count(_declaration);
 }
 
 eth::AssemblyItem CompilerContext::getFunctionEntryLabel(FunctionDefinition const& _function) const
@@ -96,17 +96,17 @@ unsigned CompilerContext::getBaseStackOffsetOfVariable(Declaration const& _decla
 {
 	auto res = m_localVariables.find(&_declaration);
 	solAssert(res != m_localVariables.end(), "Variable not found on stack.");
-	return m_localVariablesSize - res->second - 1;
+	return res->second;
 }
 
 unsigned CompilerContext::baseToCurrentStackOffset(unsigned _baseOffset) const
 {
-	return _baseOffset + m_asm.deposit();
+	return m_asm.deposit() - _baseOffset - 1;
 }
 
 unsigned CompilerContext::currentToBaseStackOffset(unsigned _offset) const
 {
-	return -baseToCurrentStackOffset(-_offset);
+	return m_asm.deposit() - _offset - 1;
 }
 
 u256 CompilerContext::getStorageLocationOfVariable(const Declaration& _declaration) const
diff --git a/CompilerContext.h b/CompilerContext.h
index cde992d5..39d8c6f6 100644
--- a/CompilerContext.h
+++ b/CompilerContext.h
@@ -42,7 +42,7 @@ public:
 	void addMagicGlobal(MagicVariableDeclaration const& _declaration);
 	void addStateVariable(VariableDeclaration const& _declaration);
 	void startNewFunction() { m_localVariables.clear(); m_asm.setDeposit(0); }
-	void addVariable(VariableDeclaration const& _declaration);
+	void addVariable(VariableDeclaration const& _declaration, unsigned _offsetToCurrent = 0);
 	void addAndInitializeVariable(VariableDeclaration const& _declaration);
 	void addFunction(FunctionDefinition const& _function);
 
@@ -59,7 +59,7 @@ public:
 	eth::AssemblyItem getFunctionEntryLabel(FunctionDefinition const& _function) const;
 	/// @returns the entry label of the given function and takes overrides into account.
 	eth::AssemblyItem getVirtualFunctionEntryLabel(FunctionDefinition const& _function) const;
-	/// Returns the distance of the given local variable from the top of the local variable stack.
+	/// Returns the distance of the given local variable from the bottom of the stack (of the current function).
 	unsigned getBaseStackOffsetOfVariable(Declaration const& _declaration) const;
 	/// If supplied by a value returned by @ref getBaseStackOffsetOfVariable(variable), returns
 	/// the distance of that variable from the current top of the stack.
@@ -112,10 +112,8 @@ private:
 	u256 m_stateVariablesSize = 0;
 	/// Storage offsets of state variables
 	std::map<Declaration const*, u256> m_stateVariables;
-	/// Offsets of local variables on the stack (relative to stack base).
+	/// Positions of local variables on the stack.
 	std::map<Declaration const*, unsigned> m_localVariables;
-	/// Sum of stack sizes of local variables
-	unsigned m_localVariablesSize;
 	/// Labels pointing to the entry points of funcitons.
 	std::map<Declaration const*, eth::AssemblyItem> m_functionEntryLabels;
 	/// Labels pointing to the entry points of function overrides.
diff --git a/NameAndTypeResolver.cpp b/NameAndTypeResolver.cpp
index ea3b43c2..4ca48165 100644
--- a/NameAndTypeResolver.cpp
+++ b/NameAndTypeResolver.cpp
@@ -310,8 +310,7 @@ void ReferencesResolver::endVisit(VariableDeclaration& _variable)
 
 bool ReferencesResolver::visit(Return& _return)
 {
-	solAssert(m_returnParameters, "Return parameters not set.");
-	_return.setFunctionReturnParameters(*m_returnParameters);
+	_return.setFunctionReturnParameters(m_returnParameters);
 	return true;
 }