Refactor assembly analysis into scope filling and checking.

9 files changed, 654 insertions, 246 deletions

diff --git a/libsolidity/inlineasm/AsmAnalysis.cpp b/libsolidity/inlineasm/AsmAnalysis.cpp
index 07167ea4..51105ad2 100644
--- a/libsolidity/inlineasm/AsmAnalysis.cpp
+++ b/libsolidity/inlineasm/AsmAnalysis.cpp
@@ -21,6 +21,8 @@
 #include <libsolidity/inlineasm/AsmAnalysis.h>
 
 #include <libsolidity/inlineasm/AsmData.h>
+#include <libsolidity/inlineasm/AsmScopeFiller.h>
+#include <libsolidity/inlineasm/AsmScope.h>
 
 #include <libsolidity/interface/Exceptions.h>
 #include <libsolidity/interface/Utils.h>
@@ -36,81 +38,70 @@ using namespace dev::solidity;
 using namespace dev::solidity::assembly;
 
 
-bool Scope::registerLabel(string const& _name)
+AsmAnalyzer::AsmAnalyzer(AsmAnalyzer::Scopes& _scopes, ErrorList& _errors, bool _allowFailedLookups):
+	m_allowFailedLookups(_allowFailedLookups), m_scopes(_scopes), m_errors(_errors)
 {
-	if (exists(_name))
-		return false;
-	identifiers[_name] = Label();
-	return true;
 }
 
-bool Scope::registerVariable(string const& _name)
+bool AsmAnalyzer::analyze(Block const& _block)
 {
-	if (exists(_name))
+	if (!(ScopeFiller(m_scopes, m_errors))(_block))
 		return false;
-	identifiers[_name] = Variable();
-	return true;
+	return (*this)(_block);
 }
 
-bool Scope::registerFunction(string const& _name, size_t _arguments, size_t _returns)
+bool AsmAnalyzer::operator()(assembly::Literal const& _literal)
 {
-	if (exists(_name))
+	if (!_literal.isNumber && _literal.value.size() > 32)
+	{
+		m_errors.push_back(make_shared<Error>(
+			Error::Type::TypeError,
+			"String literal too long (" + boost::lexical_cast<std::string>(_literal.value.size()) + " > 32)"
+		));
 		return false;
-	identifiers[_name] = Function(_arguments, _returns);
+	}
 	return true;
 }
 
-Scope::Identifier* Scope::lookup(string const& _name)
+bool AsmAnalyzer::operator()(assembly::Identifier const& _identifier)
 {
-	bool crossedFunctionBoundary = false;
-	for (Scope* s = this; s; s = s->superScope)
-	{
-		auto id = identifiers.find(_name);
-		if (id != identifiers.end())
+	bool success = true;
+	if (m_currentScope->lookup(_identifier.name, Scope::Visitor(
+		[&](Scope::Variable const& _var)
 		{
-			if (crossedFunctionBoundary && id->second.type() == typeid(Scope::Variable))
-				return nullptr;
-			else
-				return &id->second;
+			if (!_var.active)
+			{
+				m_errors.push_back(make_shared<Error>(
+					Error::Type::DeclarationError,
+					"Variable " + _identifier.name + " used before it was declared.",
+					_identifier.location
+				));
+				success = false;
+			}
+		},
+		[&](Scope::Label const&) {},
+		[&](Scope::Function const&)
+		{
+			m_errors.push_back(make_shared<Error>(
+				Error::Type::TypeError,
+				"Function " + _identifier.name + " used without being called.",
+			_identifier.location
+			));
+			success = false;
 		}
-
-		if (s->functionScope)
-			crossedFunctionBoundary = true;
+	)))
+	{
 	}
-	return nullptr;
-}
-
-bool Scope::exists(string const& _name)
-{
-	if (identifiers.count(_name))
-		return true;
-	else if (superScope)
-		return superScope->exists(_name);
-	else
-		return false;
-}
-
-AsmAnalyzer::AsmAnalyzer(AsmAnalyzer::Scopes& _scopes, ErrorList& _errors):
-	m_scopes(_scopes), m_errors(_errors)
-{
-	// Make the Solidity ErrorTag available to inline assembly
-	Scope::Label errorLabel;
-	errorLabel.id = Scope::Label::errorLabelId;
-	scope(nullptr).identifiers["invalidJumpLabel"] = errorLabel;
-	m_currentScope = &scope(nullptr);
-}
-
-bool AsmAnalyzer::operator()(assembly::Literal const& _literal)
-{
-	if (!_literal.isNumber && _literal.value.size() > 32)
+	else if (!m_allowFailedLookups)
 	{
 		m_errors.push_back(make_shared<Error>(
-			Error::Type::TypeError,
-			"String literal too long (" + boost::lexical_cast<std::string>(_literal.value.size()) + " > 32)"
+			Error::Type::DeclarationError,
+			"Identifier not found.",
+			_identifier.location
 		));
-		return false;
+		success = false;
 	}
-	return true;
+	return success;
 }
 
 bool AsmAnalyzer::operator()(FunctionalInstruction const& _instr)
@@ -124,74 +115,100 @@ bool AsmAnalyzer::operator()(FunctionalInstruction const& _instr)
 	return success;
 }
 
-bool AsmAnalyzer::operator()(Label const& _item)
+bool AsmAnalyzer::operator()(assembly::Assignment const& _assignment)
 {
-	if (!m_currentScope->registerLabel(_item.name))
-	{
-		//@TODO secondary location
-		m_errors.push_back(make_shared<Error>(
-			Error::Type::DeclarationError,
-			"Label name " + _item.name + " already taken in this scope.",
-			_item.location
-		));
-		return false;
-	}
-	return true;
+	return checkAssignment(_assignment.variableName);
 }
 
 bool AsmAnalyzer::operator()(FunctionalAssignment const& _assignment)
 {
-	return boost::apply_visitor(*this, *_assignment.value);
+	bool success = boost::apply_visitor(*this, *_assignment.value);
+	if (!checkAssignment(_assignment.variableName))
+		success = false;
+	return success;
 }
 
 bool AsmAnalyzer::operator()(assembly::VariableDeclaration const& _varDecl)
 {
 	bool success = boost::apply_visitor(*this, *_varDecl.value);
-	if (!registerVariable(_varDecl.name, _varDecl.location, *m_currentScope))
-		success = false;
+	boost::get<Scope::Variable>(m_currentScope->identifiers.at(_varDecl.name)).active = true;
 	return success;
 }
 
 bool AsmAnalyzer::operator()(assembly::FunctionDefinition const& _funDef)
 {
+	Scope& bodyScope = scope(&_funDef.body);
+	for (auto const& var: _funDef.arguments + _funDef.returns)
+		boost::get<Scope::Variable>(bodyScope.identifiers.at(var)).active = true;
+
+	return (*this)(_funDef.body);
+}
+
+bool AsmAnalyzer::operator()(assembly::FunctionCall const& _funCall)
+{
 	bool success = true;
-	if (!m_currentScope->registerFunction(_funDef.name, _funDef.arguments.size(), _funDef.returns.size()))
+	size_t arguments = 0;
+	size_t returns = 0;
+	if (!m_currentScope->lookup(_funCall.functionName.name, Scope::Visitor(
+		[&](Scope::Variable const&)
+		{
+			m_errors.push_back(make_shared<Error>(
+				Error::Type::TypeError,
+				"Attempt to call variable instead of function.",
+				_funCall.functionName.location
+			));
+			success = false;
+		},
+		[&](Scope::Label const&)
+		{
+			m_errors.push_back(make_shared<Error>(
+				Error::Type::TypeError,
+				"Attempt to call label instead of function.",
+				_funCall.functionName.location
+				));
+			success = false;
+		},
+		[&](Scope::Function const& _fun)
+		{
+			arguments = _fun.arguments;
+			returns = _fun.returns;
+		}
+	)))
 	{
-		//@TODO secondary location
 		m_errors.push_back(make_shared<Error>(
 			Error::Type::DeclarationError,
-			"Function name " + _funDef.name + " already taken in this scope.",
-			_funDef.location
+			"Function not found.",
+			_funCall.functionName.location
 		));
 		success = false;
 	}
-	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))
+	if (success)
+	{
+		if (_funCall.arguments.size() != arguments)
+		{
+			m_errors.push_back(make_shared<Error>(
+				Error::Type::TypeError,
+				"Expected " +
+				boost::lexical_cast<string>(arguments) +
+				" arguments but got " +
+				boost::lexical_cast<string>(_funCall.arguments.size()) +
+				".",
+				_funCall.functionName.location
+				));
 			success = false;
-
-	(*this)(_funDef.body);
-
-	return success;
-}
-
-bool AsmAnalyzer::operator()(assembly::FunctionCall const& _funCall)
-{
-	bool success = true;
+		}
+		//@todo check the number of returns - depends on context and should probably
+		// be only done once we have stack height checks
+	}
 	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;
-	scope(&_block).superScope = m_currentScope;
 	m_currentScope = &scope(&_block);
 
 	for (auto const& s: _block.statements)
@@ -202,25 +219,29 @@ bool AsmAnalyzer::operator()(Block const& _block)
 	return success;
 }
 
-bool AsmAnalyzer::registerVariable(string const& _name, SourceLocation const& _location, Scope& _scope)
+bool AsmAnalyzer::checkAssignment(assembly::Identifier const& _variable)
 {
-	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
-		));
+	if (!(*this)(_variable))
 		return false;
+	else if (!m_allowFailedLookups)
+	{
+		// Check that it is a variable
+		if (m_currentScope->lookup(_variable.name)->type() != typeid(Scope::Variable))
+		{
+			m_errors.push_back(make_shared<Error>(
+				Error::Type::TypeError,
+				"Assignment requires variable.",
+				_variable.location
+			));
+			return false;
+		}
 	}
 	return true;
 }
 
 Scope& AsmAnalyzer::scope(Block const* _block)
 {
-	auto& scope = m_scopes[_block];
-	if (!scope)
-		scope = make_shared<Scope>();
-	return *scope;
+	auto scopePtr = m_scopes.at(_block);
+	solAssert(scopePtr, "Scope requested but not present.");
+	return *scopePtr;
 }
diff --git a/libsolidity/inlineasm/AsmAnalysis.h b/libsolidity/inlineasm/AsmAnalysis.h
index 8658a477..c81b7a82 100644
--- a/libsolidity/inlineasm/AsmAnalysis.h
+++ b/libsolidity/inlineasm/AsmAnalysis.h
@@ -46,104 +46,29 @@ struct Assignment;
 struct FunctionDefinition;
 struct FunctionCall;
 
-template <class...>
-struct GenericVisitor{};
-
-template <class Visitable, class... Others>
-struct GenericVisitor<Visitable, Others...>: public GenericVisitor<Others...>
-{
-	using GenericVisitor<Others...>::operator ();
-	explicit GenericVisitor(
-		std::function<void(Visitable&)> _visitor,
-		std::function<void(Others&)>... _otherVisitors
-	):
-		GenericVisitor<Others...>(_otherVisitors...),
-		m_visitor(_visitor)
-	{}
-
-	void operator()(Visitable& _v) const { m_visitor(_v); }
-
-	std::function<void(Visitable&)> m_visitor;
-};
-template <>
-struct GenericVisitor<>: public boost::static_visitor<> {
-	void operator()() const {}
-};
-
-
-struct Scope
-{
-	struct Variable
-	{
-		int stackHeight = 0;
-		bool active = false;
-	};
-
-	struct Label
-	{
-		size_t id = unassignedLabelId;
-		static const size_t errorLabelId = -1;
-		static const size_t unassignedLabelId = 0;
-	};
-
-	struct Function
-	{
-		Function(size_t _arguments, size_t _returns): arguments(_arguments), returns(_returns) {}
-		size_t arguments = 0;
-		size_t returns = 0;
-	};
-
-	using Identifier = boost::variant<Variable, Label, Function>;
-	using Visitor = GenericVisitor<Variable const, Label const, Function const>;
-	using NonconstVisitor = GenericVisitor<Variable, Label, Function>;
-
-	bool registerVariable(std::string const& _name);
-	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 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 (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)
-	{
-		if (Identifier* id = lookup(_name))
-		{
-			boost::apply_visitor(_visitor, *id);
-			return true;
-		}
-		else
-			return false;
-	}
-	/// @returns true if the name exists in this scope or in super scopes (also searches
-	/// across function and assembly boundaries).
-	bool exists(std::string const& _name);
-	Scope* superScope = nullptr;
-	/// 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;
-};
-
+struct Scope;
 
+/**
+ * Performs the full analysis stage, calls the ScopeFiller internally, then resolves
+ * references and performs other checks.
+ * @todo Does not yet check for stack height issues.
+ */
 class AsmAnalyzer: public boost::static_visitor<bool>
 {
 public:
 	using Scopes = std::map<assembly::Block const*, std::shared_ptr<Scope>>;
-	AsmAnalyzer(Scopes& _scopes, ErrorList& _errors);
+	/// @param _allowFailedLookups if true, allow failed lookups for variables (they
+	/// will be provided from the environment later on)
+	AsmAnalyzer(Scopes& _scopes, ErrorList& _errors, bool _allowFailedLookups);
+
+	bool analyze(assembly::Block const& _block);
 
 	bool operator()(assembly::Instruction const&) { return true; }
 	bool operator()(assembly::Literal const& _literal);
-	bool operator()(assembly::Identifier const&) { return true; }
+	bool operator()(assembly::Identifier const&);
 	bool operator()(assembly::FunctionalInstruction const& _functionalInstruction);
-	bool operator()(assembly::Label const& _label);
-	bool operator()(assembly::Assignment const&) { return true; }
+	bool operator()(assembly::Label const&) { return true; }
+	bool operator()(assembly::Assignment const&);
 	bool operator()(assembly::FunctionalAssignment const& _functionalAssignment);
 	bool operator()(assembly::VariableDeclaration const& _variableDeclaration);
 	bool operator()(assembly::FunctionDefinition const& _functionDefinition);
@@ -151,14 +76,10 @@ public:
 	bool operator()(assembly::Block const& _block);
 
 private:
-	bool registerVariable(
-		std::string const& _name,
-		SourceLocation const& _location,
-		Scope& _scope
-	);
-
+	bool checkAssignment(assembly::Identifier const& _assignment);
 	Scope& scope(assembly::Block const* _block);
 
+	bool m_allowFailedLookups = false;
 	Scope* m_currentScope = nullptr;
 	Scopes& m_scopes;
 	ErrorList& m_errors;
diff --git a/libsolidity/inlineasm/AsmCodeGen.cpp b/libsolidity/inlineasm/AsmCodeGen.cpp
index d5931960..1caaa677 100644
--- a/libsolidity/inlineasm/AsmCodeGen.cpp
+++ b/libsolidity/inlineasm/AsmCodeGen.cpp
@@ -24,6 +24,7 @@
 
 #include <libsolidity/inlineasm/AsmParser.h>
 #include <libsolidity/inlineasm/AsmData.h>
+#include <libsolidity/inlineasm/AsmScope.h>
 #include <libsolidity/inlineasm/AsmAnalysis.h>
 
 #include <libevmasm/Assembly.h>
@@ -153,12 +154,14 @@ public:
 			},
 			[=](Scope::Function&)
 			{
-				solAssert(false, "Not yet implemented");
+				solAssert(false, "Function not removed during desugaring.");
 			}
 		)))
 		{
+			return;
 		}
-		else if (!m_identifierAccess || !m_identifierAccess(_identifier, m_state.assembly, CodeGenerator::IdentifierContext::RValue))
+		solAssert(m_identifierAccess, "Identifier not found and no external access available.");
+		if (!m_identifierAccess(_identifier, m_state.assembly, CodeGenerator::IdentifierContext::RValue))
 		{
 			m_state.addError(
 				Error::Type::DeclarationError,
@@ -186,7 +189,7 @@ public:
 	{
 		m_state.assembly.setSourceLocation(_label.location);
 		solAssert(m_scope.identifiers.count(_label.name), "");
-		Scope::Label& label = boost::get<Scope::Label>(m_scope.identifiers[_label.name]);
+		Scope::Label& label = boost::get<Scope::Label>(m_scope.identifiers.at(_label.name));
 		assignLabelIdIfUnset(label);
 		m_state.assembly.append(eth::AssemblyItem(eth::Tag, label.id));
 	}
@@ -208,8 +211,7 @@ public:
 		int height = m_state.assembly.deposit();
 		boost::apply_visitor(*this, *_varDecl.value);
 		expectDeposit(1, height, locationOf(*_varDecl.value));
-		solAssert(m_scope.identifiers.count(_varDecl.name), "");
-		auto& var = boost::get<Scope::Variable>(m_scope.identifiers[_varDecl.name]);
+		auto& var = boost::get<Scope::Variable>(m_scope.identifiers.at(_varDecl.name));
 		var.stackHeight = height;
 		var.active = true;
 	}
@@ -225,31 +227,17 @@ public:
 private:
 	void generateAssignment(assembly::Identifier const& _variableName, SourceLocation const& _location)
 	{
-		if (m_scope.lookup(_variableName.name, Scope::Visitor(
-			[=](Scope::Variable const& _var)
-			{
-				if (int heightDiff = variableHeightDiff(_var, _location, true))
-					m_state.assembly.append(solidity::swapInstruction(heightDiff - 1));
-				m_state.assembly.append(solidity::Instruction::POP);
-			},
-			[=](Scope::Label const&)
-			{
-				m_state.addError(
-					Error::Type::DeclarationError,
-					"Label \"" + string(_variableName.name) + "\" used as variable."
-				);
-			},
-			[=](Scope::Function const&)
-			{
-				m_state.addError(
-					Error::Type::DeclarationError,
-					"Function \"" + string(_variableName.name) + "\" used as variable."
-				);
-			}
-		)))
+		auto var = m_scope.lookup(_variableName.name);
+		if (var)
 		{
+			Scope::Variable const& _var = boost::get<Scope::Variable>(*var);
+			if (int heightDiff = variableHeightDiff(_var, _location, true))
+				m_state.assembly.append(solidity::swapInstruction(heightDiff - 1));
+			m_state.assembly.append(solidity::Instruction::POP);
+			return;
 		}
-		else if (!m_identifierAccess || !m_identifierAccess(_variableName, m_state.assembly, CodeGenerator::IdentifierContext::LValue))
+		solAssert(m_identifierAccess, "Identifier not found and no external access available.");
+		if (!m_identifierAccess(_variableName, m_state.assembly, CodeGenerator::IdentifierContext::LValue))
 			m_state.addError(
 				Error::Type::DeclarationError,
 				"Identifier \"" + string(_variableName.name) + "\" not found, not unique or not lvalue."
@@ -261,11 +249,6 @@ private:
 	/// errors and the (positive) stack height difference otherwise.
 	int variableHeightDiff(Scope::Variable const& _var, SourceLocation const& _location, bool _forSwap)
 	{
-		if (!_var.active)
-		{
-			m_state.addError( Error::Type::TypeError, "Variable used before it was declared", _location);
-			return 0;
-		}
 		int heightDiff = m_state.assembly.deposit() - _var.stackHeight;
 		if (heightDiff <= (_forSwap ? 1 : 0) || heightDiff > (_forSwap ? 17 : 16))
 		{
@@ -314,7 +297,7 @@ bool assembly::CodeGenerator::typeCheck(assembly::CodeGenerator::IdentifierAcces
 	size_t initialErrorLen = m_errors.size();
 	eth::Assembly assembly;
 	GeneratorState state(m_errors, assembly);
-	if (!(AsmAnalyzer(state.scopes, m_errors))(m_parsedData))
+	if (!(AsmAnalyzer(state.scopes, m_errors, !!_identifierAccess)).analyze(m_parsedData))
 		return false;
 	CodeTransform(state, m_parsedData, _identifierAccess);
 	return m_errors.size() == initialErrorLen;
@@ -324,7 +307,7 @@ eth::Assembly assembly::CodeGenerator::assemble(assembly::CodeGenerator::Identif
 {
 	eth::Assembly assembly;
 	GeneratorState state(m_errors, assembly);
-	if (!(AsmAnalyzer(state.scopes, m_errors))(m_parsedData))
+	if (!(AsmAnalyzer(state.scopes, m_errors, !!_identifierAccess)).analyze(m_parsedData))
 		solAssert(false, "Assembly error");
 	CodeTransform(state, m_parsedData, _identifierAccess);
 	return assembly;
@@ -333,7 +316,7 @@ eth::Assembly assembly::CodeGenerator::assemble(assembly::CodeGenerator::Identif
 void assembly::CodeGenerator::assemble(eth::Assembly& _assembly, assembly::CodeGenerator::IdentifierAccess const& _identifierAccess)
 {
 	GeneratorState state(m_errors, _assembly);
-	if (!(AsmAnalyzer(state.scopes, m_errors))(m_parsedData))
+	if (!(AsmAnalyzer(state.scopes, m_errors, !!_identifierAccess)).analyze(m_parsedData))
 		solAssert(false, "Assembly error");
 	CodeTransform(state, m_parsedData, _identifierAccess);
 }
diff --git a/libsolidity/inlineasm/AsmScope.cpp b/libsolidity/inlineasm/AsmScope.cpp
new file mode 100644
index 00000000..609dca16
--- /dev/null
+++ b/libsolidity/inlineasm/AsmScope.cpp
@@ -0,0 +1,79 @@
+/*
+	This file is part of solidity.
+
+	solidity is free software: you can redistribute it and/or modify
+	it under the terms of the GNU General Public License as published by
+	the Free Software Foundation, either version 3 of the License, or
+	(at your option) any later version.
+
+	solidity is distributed in the hope that it will be useful,
+	but WITHOUT ANY WARRANTY; without even the implied warranty of
+	MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+	GNU General Public License for more details.
+
+	You should have received a copy of the GNU General Public License
+	along with solidity.  If not, see <http://www.gnu.org/licenses/>.
+*/
+/**
+ * Scopes for identifiers.
+ */
+
+#include <libsolidity/inlineasm/AsmScope.h>
+
+using namespace std;
+using namespace dev::solidity::assembly;
+
+
+bool Scope::registerLabel(string const& _name)
+{
+	if (exists(_name))
+		return false;
+	identifiers[_name] = Label();
+	return true;
+}
+
+bool Scope::registerVariable(string const& _name)
+{
+	if (exists(_name))
+		return false;
+	identifiers[_name] = Variable();
+	return true;
+}
+
+bool Scope::registerFunction(string const& _name, size_t _arguments, size_t _returns)
+{
+	if (exists(_name))
+		return false;
+	identifiers[_name] = Function(_arguments, _returns);
+	return true;
+}
+
+Scope::Identifier* Scope::lookup(string const& _name)
+{
+	bool crossedFunctionBoundary = false;
+	for (Scope* s = this; s; s = s->superScope)
+	{
+		auto id = s->identifiers.find(_name);
+		if (id != s->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)
+{
+	if (identifiers.count(_name))
+		return true;
+	else if (superScope)
+		return superScope->exists(_name);
+	else
+		return false;
+}
diff --git a/libsolidity/inlineasm/AsmScope.h b/libsolidity/inlineasm/AsmScope.h
new file mode 100644
index 00000000..37e0f0b8
--- /dev/null
+++ b/libsolidity/inlineasm/AsmScope.h
@@ -0,0 +1,128 @@
+/*
+	This file is part of solidity.
+
+	solidity is free software: you can redistribute it and/or modify
+	it under the terms of the GNU General Public License as published by
+	the Free Software Foundation, either version 3 of the License, or
+	(at your option) any later version.
+
+	solidity is distributed in the hope that it will be useful,
+	but WITHOUT ANY WARRANTY; without even the implied warranty of
+	MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+	GNU General Public License for more details.
+
+	You should have received a copy of the GNU General Public License
+	along with solidity.  If not, see <http://www.gnu.org/licenses/>.
+*/
+/**
+ * Scopes for identifiers.
+ */
+
+#pragma once
+
+#include <libsolidity/interface/Exceptions.h>
+
+#include <boost/variant.hpp>
+
+#include <functional>
+#include <memory>
+
+namespace dev
+{
+namespace solidity
+{
+namespace assembly
+{
+
+template <class...>
+struct GenericVisitor{};
+
+template <class Visitable, class... Others>
+struct GenericVisitor<Visitable, Others...>: public GenericVisitor<Others...>
+{
+	using GenericVisitor<Others...>::operator ();
+	explicit GenericVisitor(
+		std::function<void(Visitable&)> _visitor,
+		std::function<void(Others&)>... _otherVisitors
+	):
+		GenericVisitor<Others...>(_otherVisitors...),
+		m_visitor(_visitor)
+	{}
+
+	void operator()(Visitable& _v) const { m_visitor(_v); }
+
+	std::function<void(Visitable&)> m_visitor;
+};
+template <>
+struct GenericVisitor<>: public boost::static_visitor<> {
+	void operator()() const {}
+};
+
+
+struct Scope
+{
+	struct Variable
+	{
+		/// Used during code generation to store the stack height. @todo move there.
+		int stackHeight = 0;
+		/// Used during analysis to check whether we already passed the declaration inside the block.
+		/// @todo move there.
+		bool active = false;
+	};
+
+	struct Label
+	{
+		size_t id = unassignedLabelId;
+		static const size_t errorLabelId = -1;
+		static const size_t unassignedLabelId = 0;
+	};
+
+	struct Function
+	{
+		Function(size_t _arguments, size_t _returns): arguments(_arguments), returns(_returns) {}
+		size_t arguments = 0;
+		size_t returns = 0;
+	};
+
+	using Identifier = boost::variant<Variable, Label, Function>;
+	using Visitor = GenericVisitor<Variable const, Label const, Function const>;
+	using NonconstVisitor = GenericVisitor<Variable, Label, Function>;
+
+	bool registerVariable(std::string const& _name);
+	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 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 (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)
+	{
+		if (Identifier* id = lookup(_name))
+		{
+			boost::apply_visitor(_visitor, *id);
+			return true;
+		}
+		else
+			return false;
+	}
+	/// @returns true if the name exists in this scope or in super scopes (also searches
+	/// across function and assembly boundaries).
+	bool exists(std::string const& _name);
+
+	Scope* superScope = nullptr;
+	/// 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;
+};
+
+}
+}
+}
diff --git a/libsolidity/inlineasm/AsmScopeFiller.cpp b/libsolidity/inlineasm/AsmScopeFiller.cpp
new file mode 100644
index 00000000..66a217ea
--- /dev/null
+++ b/libsolidity/inlineasm/AsmScopeFiller.cpp
@@ -0,0 +1,158 @@
+/*
+	This file is part of solidity.
+
+	solidity is free software: you can redistribute it and/or modify
+	it under the terms of the GNU General Public License as published by
+	the Free Software Foundation, either version 3 of the License, or
+	(at your option) any later version.
+
+	solidity is distributed in the hope that it will be useful,
+	but WITHOUT ANY WARRANTY; without even the implied warranty of
+	MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+	GNU General Public License for more details.
+
+	You should have received a copy of the GNU General Public License
+	along with solidity.  If not, see <http://www.gnu.org/licenses/>.
+*/
+/**
+ * Module responsible for registering identifiers inside their scopes.
+ */
+
+#include <libsolidity/inlineasm/AsmScopeFiller.h>
+
+#include <libsolidity/inlineasm/AsmData.h>
+#include <libsolidity/inlineasm/AsmScope.h>
+
+#include <libsolidity/interface/Exceptions.h>
+#include <libsolidity/interface/Utils.h>
+
+#include <boost/range/adaptor/reversed.hpp>
+
+#include <memory>
+#include <functional>
+
+using namespace std;
+using namespace dev;
+using namespace dev::solidity;
+using namespace dev::solidity::assembly;
+
+ScopeFiller::ScopeFiller(ScopeFiller::Scopes& _scopes, ErrorList& _errors):
+	m_scopes(_scopes), m_errors(_errors)
+{
+	// Make the Solidity ErrorTag available to inline assembly
+	Scope::Label errorLabel;
+	errorLabel.id = Scope::Label::errorLabelId;
+	scope(nullptr).identifiers["invalidJumpLabel"] = errorLabel;
+	m_currentScope = &scope(nullptr);
+}
+
+bool ScopeFiller::operator()(FunctionalInstruction const& _instr)
+{
+	bool success = true;
+	for (auto const& arg: _instr.arguments | boost::adaptors::reversed)
+		if (!boost::apply_visitor(*this, arg))
+			success = false;
+	if (!(*this)(_instr.instruction))
+		success = false;
+	return success;
+}
+
+bool ScopeFiller::operator()(Label const& _item)
+{
+	if (!m_currentScope->registerLabel(_item.name))
+	{
+		//@TODO secondary location
+		m_errors.push_back(make_shared<Error>(
+			Error::Type::DeclarationError,
+			"Label name " + _item.name + " already taken in this scope.",
+			_item.location
+		));
+		return false;
+	}
+	return true;
+}
+
+bool ScopeFiller::operator()(FunctionalAssignment const& _assignment)
+{
+	return boost::apply_visitor(*this, *_assignment.value);
+}
+
+bool ScopeFiller::operator()(assembly::VariableDeclaration const& _varDecl)
+{
+	bool success = boost::apply_visitor(*this, *_varDecl.value);
+	if (!registerVariable(_varDecl.name, _varDecl.location, *m_currentScope))
+		success = false;
+	return success;
+}
+
+bool ScopeFiller::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,
+			"Function name " + _funDef.name + " already taken in this scope.",
+			_funDef.location
+		));
+		success = false;
+	}
+	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;
+
+	if (!(*this)(_funDef.body))
+		success = false;
+
+	return success;
+}
+
+bool ScopeFiller::operator()(assembly::FunctionCall const& _funCall)
+{
+	bool success = true;
+	for (auto const& arg: _funCall.arguments | boost::adaptors::reversed)
+		if (!boost::apply_visitor(*this, arg))
+			success = false;
+	return success;
+}
+
+bool ScopeFiller::operator()(Block const& _block)
+{
+	bool success = true;
+	scope(&_block).superScope = m_currentScope;
+	m_currentScope = &scope(&_block);
+
+	for (auto const& s: _block.statements)
+		if (!boost::apply_visitor(*this, s))
+			success = false;
+
+	m_currentScope = m_currentScope->superScope;
+	return success;
+}
+
+bool ScopeFiller::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& ScopeFiller::scope(Block const* _block)
+{
+	auto& scope = m_scopes[_block];
+	if (!scope)
+		scope = make_shared<Scope>();
+	return *scope;
+}
diff --git a/libsolidity/inlineasm/AsmScopeFiller.h b/libsolidity/inlineasm/AsmScopeFiller.h
new file mode 100644
index 00000000..3747af0a
--- /dev/null
+++ b/libsolidity/inlineasm/AsmScopeFiller.h
@@ -0,0 +1,89 @@
+/*
+	This file is part of solidity.
+
+	solidity is free software: you can redistribute it and/or modify
+	it under the terms of the GNU General Public License as published by
+	the Free Software Foundation, either version 3 of the License, or
+	(at your option) any later version.
+
+	solidity is distributed in the hope that it will be useful,
+	but WITHOUT ANY WARRANTY; without even the implied warranty of
+	MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+	GNU General Public License for more details.
+
+	You should have received a copy of the GNU General Public License
+	along with solidity.  If not, see <http://www.gnu.org/licenses/>.
+*/
+/**
+ * Module responsible for registering identifiers inside their scopes.
+ */
+
+#pragma once
+
+#include <libsolidity/interface/Exceptions.h>
+
+#include <boost/variant.hpp>
+
+#include <functional>
+#include <memory>
+
+namespace dev
+{
+namespace solidity
+{
+namespace assembly
+{
+
+struct Literal;
+struct Block;
+struct Label;
+struct FunctionalInstruction;
+struct FunctionalAssignment;
+struct VariableDeclaration;
+struct Instruction;
+struct Identifier;
+struct Assignment;
+struct FunctionDefinition;
+struct FunctionCall;
+
+struct Scope;
+
+/**
+ * Fills scopes with identifiers and checks for name clashes.
+ * Does not resolve references.
+ */
+class ScopeFiller: public boost::static_visitor<bool>
+{
+public:
+	using Scopes = std::map<assembly::Block const*, std::shared_ptr<Scope>>;
+	ScopeFiller(Scopes& _scopes, ErrorList& _errors);
+
+	bool operator()(assembly::Instruction const&) { return true; }
+	bool operator()(assembly::Literal const&) { return true; }
+	bool operator()(assembly::Identifier const&) { return true; }
+	bool operator()(assembly::FunctionalInstruction const& _functionalInstruction);
+	bool operator()(assembly::Label const& _label);
+	bool operator()(assembly::Assignment const&) { return true; }
+	bool operator()(assembly::FunctionalAssignment const& _functionalAssignment);
+	bool operator()(assembly::VariableDeclaration const& _variableDeclaration);
+	bool operator()(assembly::FunctionDefinition const& _functionDefinition);
+	bool operator()(assembly::FunctionCall const& _functionCall);
+	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/AsmStack.cpp b/libsolidity/inlineasm/AsmStack.cpp
index 266136a1..8d011cf8 100644
--- a/libsolidity/inlineasm/AsmStack.cpp
+++ b/libsolidity/inlineasm/AsmStack.cpp
@@ -49,7 +49,7 @@ bool InlineAssemblyStack::parse(shared_ptr<Scanner> const& _scanner)
 
 	*m_parserResult = std::move(*result);
 	AsmAnalyzer::Scopes scopes;
-	return (AsmAnalyzer(scopes, m_errors))(*m_parserResult);
+	return (AsmAnalyzer(scopes, m_errors, false)).analyze(*m_parserResult);
 }
 
 string InlineAssemblyStack::toString()
diff --git a/test/libsolidity/InlineAssembly.cpp b/test/libsolidity/InlineAssembly.cpp
index 9035599b..ab038622 100644
--- a/test/libsolidity/InlineAssembly.cpp
+++ b/test/libsolidity/InlineAssembly.cpp
@@ -63,7 +63,7 @@ boost::optional<Error> parseAndReturnFirstError(string const& _source, bool _ass
 	}
 	if (!success)
 	{
-		BOOST_CHECK_EQUAL(stack.errors().size(), 1);
+		BOOST_REQUIRE_EQUAL(stack.errors().size(), 1);
 		return *stack.errors().front();
 	}
 	else
@@ -162,7 +162,7 @@ BOOST_AUTO_TEST_CASE(vardecl)
 
 BOOST_AUTO_TEST_CASE(assignment)
 {
-	BOOST_CHECK(successParse("{ 7 8 add =: x }"));
+	BOOST_CHECK(successParse("{ let x := 2 7 8 add =: x }"));
 }
 
 BOOST_AUTO_TEST_CASE(label)
@@ -177,22 +177,28 @@ BOOST_AUTO_TEST_CASE(label_complex)
 
 BOOST_AUTO_TEST_CASE(functional)
 {
-	BOOST_CHECK(successParse("{ add(7, mul(6, x)) add mul(7, 8) }"));
+	BOOST_CHECK(successParse("{ let x := 2 add(7, mul(6, x)) mul(7, 8) add }"));
 }
 
 BOOST_AUTO_TEST_CASE(functional_assignment)
 {
-	BOOST_CHECK(successParse("{ x := 7 }"));
+	BOOST_CHECK(successParse("{ let x := 2 x := 7 }"));
 }
 
 BOOST_AUTO_TEST_CASE(functional_assignment_complex)
 {
-	BOOST_CHECK(successParse("{ x := add(7, mul(6, x)) add mul(7, 8) }"));
+	BOOST_CHECK(successParse("{ let x := 2 x := add(7, mul(6, x)) mul(7, 8) add }"));
 }
 
 BOOST_AUTO_TEST_CASE(vardecl_complex)
 {
-	BOOST_CHECK(successParse("{ let x := add(7, mul(6, x)) add mul(7, 8) }"));
+	BOOST_CHECK(successParse("{ let y := 2 let x := add(7, mul(6, y)) add mul(7, 8) }"));
+}
+
+BOOST_AUTO_TEST_CASE(variable_use_before_decl)
+{
+	CHECK_PARSE_ERROR("{ x := 2 let x := 3 }", DeclarationError, "Variable x used before it was declared.");
+	CHECK_PARSE_ERROR("{ let x := mul(2, x) }", DeclarationError, "Variable x used before it was declared.");
 }
 
 BOOST_AUTO_TEST_CASE(blocks)
@@ -212,7 +218,28 @@ BOOST_AUTO_TEST_CASE(function_definitions_multiple_args)
 
 BOOST_AUTO_TEST_CASE(function_calls)
 {
-	BOOST_CHECK(successParse("{ g(1, 2, f(mul(2, 3))) x() }"));
+	BOOST_CHECK(successParse("{ function f(a) {} function g(a, b, c) {} function x() { g(1, 2, f(mul(2, 3))) x() } }"));
+}
+
+BOOST_AUTO_TEST_CASE(opcode_for_functions)
+{
+	CHECK_PARSE_ERROR("{ function gas() { } }", ParserError, "Cannot use instruction names for identifier names.");
+}
+
+BOOST_AUTO_TEST_CASE(opcode_for_function_args)
+{
+	CHECK_PARSE_ERROR("{ function f(gas) { } }", ParserError, "Cannot use instruction names for identifier names.");
+	CHECK_PARSE_ERROR("{ function f() -> (gas) { } }", ParserError, "Cannot use instruction names for identifier names.");
+}
+
+BOOST_AUTO_TEST_CASE(name_clashes)
+{
+	CHECK_PARSE_ERROR("{ let g := 2 function g() { } }", DeclarationError, "Function name g already taken in this scope");
+}
+
+BOOST_AUTO_TEST_CASE(variable_access_cross_functions)
+{
+	CHECK_PARSE_ERROR("{ let x := 2 function g() { x } }", DeclarationError, "Identifier not found.");
 }
 
 BOOST_AUTO_TEST_SUITE_END()
@@ -272,7 +299,9 @@ BOOST_AUTO_TEST_CASE(function_definitions_multiple_args)
 
 BOOST_AUTO_TEST_CASE(function_calls)
 {
-	parsePrintCompare("{\n    g(1, mul(2, x), f(mul(2, 3)))\n    x()\n}");
+	parsePrintCompare(
+		"{\n    function y()\n    {\n    }\n    function f(a)\n    {\n    }\n    function g(a, b, c)\n    {\n    }\n    g(1, mul(2, address), f(mul(2, caller)))\n    y()\n}"
+	);
 }
 
 BOOST_AUTO_TEST_SUITE_END()
@@ -296,8 +325,8 @@ BOOST_AUTO_TEST_CASE(assignment_after_tag)
 
 BOOST_AUTO_TEST_CASE(magic_variables)
 {
-	CHECK_ASSEMBLE_ERROR("{ this pop }", DeclarationError, "Identifier not found or not unique");
-	CHECK_ASSEMBLE_ERROR("{ ecrecover pop }", DeclarationError, "Identifier not found or not unique");
+	CHECK_ASSEMBLE_ERROR("{ this pop }", DeclarationError, "Identifier not found");
+	CHECK_ASSEMBLE_ERROR("{ ecrecover pop }", DeclarationError, "Identifier not found");
 	BOOST_CHECK(successAssemble("{ let ecrecover := 1 ecrecover }"));
 }