Register functions.

3 files changed, 90 insertions, 33 deletions

diff --git a/libsolidity/inlineasm/AsmAnalysis.cpp b/libsolidity/inlineasm/AsmAnalysis.cpp
index a3ddb61d..07167ea4 100644
--- a/libsolidity/inlineasm/AsmAnalysis.cpp
+++ b/libsolidity/inlineasm/AsmAnalysis.cpp
@@ -62,12 +62,22 @@ bool Scope::registerFunction(string const& _name, size_t _arguments, size_t _ret
 
 Scope::Identifier* Scope::lookup(string const& _name)
 {
-	if (identifiers.count(_name))
-		return &identifiers[_name];
-	else if (superScope && !closedScope)
-		return superScope->lookup(_name);
-	else
-		return nullptr;
+	bool crossedFunctionBoundary = false;
+	for (Scope* s = this; s; s = s->superScope)
+	{
+		auto id = identifiers.find(_name);
+		if (id != identifiers.end())
+		{
+			if (crossedFunctionBoundary && id->second.type() == typeid(Scope::Variable))
+				return nullptr;
+			else
+				return &id->second;
+		}
+
+		if (s->functionScope)
+			crossedFunctionBoundary = true;
+	}
+	return nullptr;
 }
 
 bool Scope::exists(string const& _name)
@@ -84,11 +94,10 @@ AsmAnalyzer::AsmAnalyzer(AsmAnalyzer::Scopes& _scopes, ErrorList& _errors):
 	m_scopes(_scopes), m_errors(_errors)
 {
 	// Make the Solidity ErrorTag available to inline assembly
-	m_scopes[nullptr] = make_shared<Scope>();
 	Scope::Label errorLabel;
 	errorLabel.id = Scope::Label::errorLabelId;
-	m_scopes[nullptr]->identifiers["invalidJumpLabel"] = errorLabel;
-	m_currentScope = m_scopes[nullptr].get();
+	scope(nullptr).identifiers["invalidJumpLabel"] = errorLabel;
+	m_currentScope = &scope(nullptr);
 }
 
 bool AsmAnalyzer::operator()(assembly::Literal const& _literal)
@@ -138,38 +147,52 @@ bool AsmAnalyzer::operator()(FunctionalAssignment const& _assignment)
 bool AsmAnalyzer::operator()(assembly::VariableDeclaration const& _varDecl)
 {
 	bool success = boost::apply_visitor(*this, *_varDecl.value);
-	if (!m_currentScope->registerVariable(_varDecl.name))
+	if (!registerVariable(_varDecl.name, _varDecl.location, *m_currentScope))
+		success = false;
+	return success;
+}
+
+bool AsmAnalyzer::operator()(assembly::FunctionDefinition const& _funDef)
+{
+	bool success = true;
+	if (!m_currentScope->registerFunction(_funDef.name, _funDef.arguments.size(), _funDef.returns.size()))
 	{
 		//@TODO secondary location
 		m_errors.push_back(make_shared<Error>(
 			Error::Type::DeclarationError,
-			"Variable name " + _varDecl.name + " already taken in this scope.",
-			_varDecl.location
+			"Function name " + _funDef.name + " already taken in this scope.",
+			_funDef.location
 		));
 		success = false;
 	}
-	return success;
-}
+	Scope& body = scope(&_funDef.body);
+	body.superScope = m_currentScope;
+	body.functionScope = true;
+	for (auto const& var: _funDef.arguments + _funDef.returns)
+		if (!registerVariable(var, _funDef.location, body))
+			success = false;
 
-bool AsmAnalyzer::operator()(assembly::FunctionDefinition const&)
-{
-	// TODO - we cannot throw an exception here because of some tests.
-	return true;
+	(*this)(_funDef.body);
+
+	return success;
 }
 
-bool AsmAnalyzer::operator()(assembly::FunctionCall const&)
+bool AsmAnalyzer::operator()(assembly::FunctionCall const& _funCall)
 {
-	// TODO - we cannot throw an exception here because of some tests.
-	return true;
+	bool success = true;
+	for (auto const& arg: _funCall.arguments | boost::adaptors::reversed)
+		if (!boost::apply_visitor(*this, arg))
+			success = false;
+	// TODO actually look up the function (can only be done in a second pass)
+	// and check that the number of arguments and of returns matches the context
+	return success;
 }
 
 bool AsmAnalyzer::operator()(Block const& _block)
 {
 	bool success = true;
-	auto scope = make_shared<Scope>();
-	scope->superScope = m_currentScope;
-	m_scopes[&_block] = scope;
-	m_currentScope = scope.get();
+	scope(&_block).superScope = m_currentScope;
+	m_currentScope = &scope(&_block);
 
 	for (auto const& s: _block.statements)
 		if (!boost::apply_visitor(*this, s))
@@ -178,3 +201,26 @@ bool AsmAnalyzer::operator()(Block const& _block)
 	m_currentScope = m_currentScope->superScope;
 	return success;
 }
+
+bool AsmAnalyzer::registerVariable(string const& _name, SourceLocation const& _location, Scope& _scope)
+{
+	if (!_scope.registerVariable(_name))
+	{
+		//@TODO secondary location
+		m_errors.push_back(make_shared<Error>(
+			Error::Type::DeclarationError,
+			"Variable name " + _name + " already taken in this scope.",
+			_location
+		));
+		return false;
+	}
+	return true;
+}
+
+Scope& AsmAnalyzer::scope(Block const* _block)
+{
+	auto& scope = m_scopes[_block];
+	if (!scope)
+		scope = make_shared<Scope>();
+	return *scope;
+}
diff --git a/libsolidity/inlineasm/AsmAnalysis.h b/libsolidity/inlineasm/AsmAnalysis.h
index 9726210d..8658a477 100644
--- a/libsolidity/inlineasm/AsmAnalysis.h
+++ b/libsolidity/inlineasm/AsmAnalysis.h
@@ -101,12 +101,15 @@ struct Scope
 	bool registerLabel(std::string const& _name);
 	bool registerFunction(std::string const& _name, size_t _arguments, size_t _returns);
 
-	/// Looks up the identifier in this or super scopes (stops and function and assembly boundaries)
-	/// and returns a valid pointer if found or a nullptr if not found.
+	/// Looks up the identifier in this or super scopes and returns a valid pointer if found
+	/// or a nullptr if not found. Variable lookups up across function boundaries will fail, as
+	/// will any lookups across assembly boundaries.
 	/// The pointer will be invalidated if the scope is modified.
+	/// @param _crossedFunction if true, we already crossed a function boundary during recursive lookup
 	Identifier* lookup(std::string const& _name);
-	/// Looks up the identifier in this and super scopes (stops and function and assembly boundaries)
-	/// and calls the visitor, returns false if not found.
+	/// Looks up the identifier in this and super scopes (will not find variables across function
+	/// boundaries and generally stops at assembly boundaries) and calls the visitor, returns
+	/// false if not found.
 	template <class V>
 	bool lookup(std::string const& _name, V const& _visitor)
 	{
@@ -122,9 +125,9 @@ struct Scope
 	/// across function and assembly boundaries).
 	bool exists(std::string const& _name);
 	Scope* superScope = nullptr;
-	/// If true, identifiers from the super scope are not visible here, but they are still
-	/// taken into account to prevent shadowing.
-	bool closedScope = false;
+	/// If true, variables from the super scope are not visible here (other identifiers are),
+	/// but they are still taken into account to prevent shadowing.
+	bool functionScope = false;
 	std::map<std::string, Identifier> identifiers;
 };
 
@@ -148,6 +151,14 @@ public:
 	bool operator()(assembly::Block const& _block);
 
 private:
+	bool registerVariable(
+		std::string const& _name,
+		SourceLocation const& _location,
+		Scope& _scope
+	);
+
+	Scope& scope(assembly::Block const* _block);
+
 	Scope* m_currentScope = nullptr;
 	Scopes& m_scopes;
 	ErrorList& m_errors;
diff --git a/libsolidity/inlineasm/AsmCodeGen.cpp b/libsolidity/inlineasm/AsmCodeGen.cpp
index fdfd9abe..d5931960 100644
--- a/libsolidity/inlineasm/AsmCodeGen.cpp
+++ b/libsolidity/inlineasm/AsmCodeGen.cpp
@@ -66,7 +66,7 @@ struct GeneratorState
 		return size_t(id);
 	}
 
-	std::map<assembly::Block const*, shared_ptr<Scope>> scopes;
+	AsmAnalyzer::Scopes scopes;
 	ErrorList& errors;
 	eth::Assembly& assembly;
 };