Split external identifier access into resolving and code generation.

libsolidity/analysis/ReferencesResolver.cpp

@@ -158,21 +158,22 @@ void ReferencesResolver::endVisit(ArrayTypeName const& _typeName)
 
 bool ReferencesResolver::visit(InlineAssembly const& _inlineAssembly)
 {
-	// We need to perform a full code generation pass here as inline assembly does not distinguish
-	// reference resolution and code generation.
 	// Errors created in this stage are completely ignored because we do not yet know
 	// the type and size of external identifiers, which would result in false errors.
+	// The only purpose of this step is to fill the inline assembly annotation with
+	// external references.
 	ErrorList errorsIgnored;
 	assembly::CodeGenerator codeGen(_inlineAssembly.operations(), errorsIgnored);
-	codeGen.typeCheck([&](assembly::Identifier const& _identifier, eth::Assembly&, assembly::CodeGenerator::IdentifierContext) {
+	assembly::ExternalIdentifierAccess identifierAccess;
+	identifierAccess.resolve = [&](assembly::Identifier const& _identifier, assembly::IdentifierContext) {
 		auto declarations = m_resolver.nameFromCurrentScope(_identifier.name);
 		if (declarations.size() != 1)
-			return false;
+			return size_t(-1);
-		_inlineAssembly.annotation().externalReferences[&_identifier] = declarations.front();
+		_inlineAssembly.annotation().externalReferences[&_identifier].declaration = declarations.front();
-		// At this stage we neither know the code to generate nor the stack size of the identifier,
+		// At this stage we do not yet know the stack size of the identifier, so we just return 1.
-		// so we do not modify assembly.
+		return size_t(1);
-		return true;
+	};
-	});
+	codeGen.typeCheck(identifierAccess);
 	return false;
 }
 

libsolidity/analysis/TypeChecker.cpp

@@ -628,47 +628,44 @@ void TypeChecker::endVisit(FunctionTypeName const& _funType)
 
 bool TypeChecker::visit(InlineAssembly const& _inlineAssembly)
 {
-	// Inline assembly does not have its own type-checking phase, so we just run the
-	// code-generator and see whether it produces any errors.
 	// External references have already been resolved in a prior stage and stored in the annotation.
-	auto identifierAccess = [&](
+	// We run the resolve step again regardless.
+	assembly::ExternalIdentifierAccess identifierAccess;
+	identifierAccess.resolve = [&](
 		assembly::Identifier const& _identifier,
-		eth::Assembly& _assembly,
+		assembly::IdentifierContext _context
-		assembly::CodeGenerator::IdentifierContext _context
 	)
 	{
 		auto ref = _inlineAssembly.annotation().externalReferences.find(&_identifier);
 		if (ref == _inlineAssembly.annotation().externalReferences.end())
-			return false;
+			return size_t(-1);
-		Declaration const* declaration = ref->second;
+		size_t valueSize = size_t(-1);
+		Declaration const* declaration = ref->second.declaration;
 		solAssert(!!declaration, "");
-		if (_context == assembly::CodeGenerator::IdentifierContext::RValue)
+		if (_context == assembly::IdentifierContext::RValue)
 		{
 			solAssert(!!declaration->type(), "Type of declaration required but not yet determined.");
-			unsigned pushes = 0;
 			if (dynamic_cast<FunctionDefinition const*>(declaration))
-				pushes = 1;
+				valueSize = 1;
 			else if (auto var = dynamic_cast<VariableDeclaration const*>(declaration))
 			{
 				if (var->isConstant())
 					fatalTypeError(SourceLocation(), "Constant variables not yet implemented for inline assembly.");
 				if (var->isLocalVariable())
-					pushes = var->type()->sizeOnStack();
+					valueSize = var->type()->sizeOnStack();
 				else if (!var->type()->isValueType())
-					pushes = 1;
+					valueSize = 1;
 				else
-					pushes = 2; // slot number, intra slot offset
+					valueSize = 2; // slot number, intra slot offset
 			}
 			else if (auto contract = dynamic_cast<ContractDefinition const*>(declaration))
 			{
 				if (!contract->isLibrary())
-					return false;
+					return size_t(-1);
-				pushes = 1;
+				valueSize = 1;
 			}
 			else
-				return false;
+				return size_t(-1);
-			for (unsigned i = 0; i < pushes; ++i)
-				_assembly.append(u256(0)); // just to verify the stack height
 		}
 		else
 		{
@@ -676,14 +673,14 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly)
 			if (auto varDecl = dynamic_cast<VariableDeclaration const*>(declaration))
 			{
 				if (!varDecl->isLocalVariable())
-					return false; // only local variables are inline-assemlby lvalues
+					return size_t(-1); // only local variables are inline-assembly lvalues
-				for (unsigned i = 0; i < declaration->type()->sizeOnStack(); ++i)
+				valueSize = size_t(declaration->type()->sizeOnStack());
-					_assembly.append(Instruction::POP); // remove value just to verify the stack height
 			}
 			else
-				return false;
+				return size_t(-1);
 		}
-		return true;
+		ref->second.valueSize = valueSize;
+		return valueSize;
 	};
 	assembly::CodeGenerator codeGen(_inlineAssembly.operations(), m_errors);
 	if (!codeGen.typeCheck(identifierAccess))

libsolidity/ast/ASTAnnotations.h

@@ -117,8 +117,14 @@ struct Identifier; // forward
 
 struct InlineAssemblyAnnotation: StatementAnnotation
 {
-	/// Mapping containing resolved references to external identifiers.
+	struct ExternalIdentifierInfo
-	std::map<assembly::Identifier const*, Declaration const*> externalReferences;
+	{
+		Declaration const* declaration = nullptr;
+		size_t valueSize = size_t(-1);
+	};
+
+	/// Mapping containing resolved references to external identifiers and their value size
+	std::map<assembly::Identifier const*, ExternalIdentifierInfo> externalReferences;
 };
 
 struct ReturnAnnotation: StatementAnnotation

libsolidity/codegen/CompilerContext.cpp

@@ -265,31 +265,38 @@ void CompilerContext::appendInlineAssembly(
 	}
 
 	unsigned startStackHeight = stackHeight();
-	auto identifierAccess = [&](
+
+	assembly::ExternalIdentifierAccess identifierAccess;
+	identifierAccess.resolve = [&](
 		assembly::Identifier const& _identifier,
-		eth::Assembly& _assembly,
+		assembly::IdentifierContext
-		assembly::CodeGenerator::IdentifierContext _context
 	) {
 		auto it = std::find(_localVariables.begin(), _localVariables.end(), _identifier.name);
-		if (it == _localVariables.end())
+		return it == _localVariables.end() ? size_t(-1) : 1;
-			return false;
+	};
+	identifierAccess.generateCode = [&](
+		assembly::Identifier const& _identifier,
+		assembly::IdentifierContext _context,
+		eth::Assembly& _assembly
+	) {
+		auto it = std::find(_localVariables.begin(), _localVariables.end(), _identifier.name);
+		solAssert(it != _localVariables.end(), "");
 		unsigned stackDepth = _localVariables.end() - it;
 		int stackDiff = _assembly.deposit() - startStackHeight + stackDepth;
-		if (_context == assembly::CodeGenerator::IdentifierContext::LValue)
+		if (_context == assembly::IdentifierContext::LValue)
 			stackDiff -= 1;
 		if (stackDiff < 1 || stackDiff > 16)
 			BOOST_THROW_EXCEPTION(
 				CompilerError() <<
 				errinfo_comment("Stack too deep, try removing local variables.")
 			);
-		if (_context == assembly::CodeGenerator::IdentifierContext::RValue)
+		if (_context == assembly::IdentifierContext::RValue)
 			_assembly.append(dupInstruction(stackDiff));
 		else
 		{
 			_assembly.append(swapInstruction(stackDiff));
 			_assembly.append(Instruction::POP);
 		}
-		return true;
 	};
 
 	solAssert(assembly::InlineAssemblyStack().parseAndAssemble(*assembly, *m_asm, identifierAccess), "Failed to assemble inline assembly block.");

libsolidity/codegen/ContractCompiler.cpp

@@ -522,15 +522,21 @@ bool ContractCompiler::visit(InlineAssembly const& _inlineAssembly)
 	ErrorList errors;
 	assembly::CodeGenerator codeGen(_inlineAssembly.operations(), errors);
 	unsigned startStackHeight = m_context.stackHeight();
-	codeGen.assemble(
+	assembly::ExternalIdentifierAccess identifierAccess;
-		m_context.nonConstAssembly(),
+	identifierAccess.resolve = [&](assembly::Identifier const& _identifier, assembly::IdentifierContext)
-		[&](assembly::Identifier const& _identifier, eth::Assembly& _assembly, assembly::CodeGenerator::IdentifierContext _context) {
+	{
 		auto ref = _inlineAssembly.annotation().externalReferences.find(&_identifier);
 		if (ref == _inlineAssembly.annotation().externalReferences.end())
-				return false;
+			return size_t(-1);
-			Declaration const* decl = ref->second;
+		return ref->second.valueSize;
+	};
+	identifierAccess.generateCode = [&](assembly::Identifier const& _identifier, assembly::IdentifierContext _context, eth::Assembly& _assembly)
+	{
+		auto ref = _inlineAssembly.annotation().externalReferences.find(&_identifier);
+		solAssert(ref != _inlineAssembly.annotation().externalReferences.end(), "");
+		Declaration const* decl = ref->second.declaration;
 		solAssert(!!decl, "");
-			if (_context == assembly::CodeGenerator::IdentifierContext::RValue)
+		if (_context == assembly::IdentifierContext::RValue)
 		{
 			solAssert(!!decl->type(), "Type of declaration required but not yet determined.");
 			if (FunctionDefinition const* functionDef = dynamic_cast<FunctionDefinition const*>(decl))
@@ -605,8 +611,10 @@ bool ContractCompiler::visit(InlineAssembly const& _inlineAssembly)
 				_assembly.append(Instruction::POP);
 			}
 		}
-			return true;
+	};
-		}
+	codeGen.assemble(
+		m_context.nonConstAssembly(),
+		identifierAccess
 	);
 	solAssert(Error::containsOnlyWarnings(errors), "Code generation for inline assembly with errors requested.");
 	m_context.setStackOffset(startStackHeight);

libsolidity/inlineasm/AsmCodeGen.cpp

@@ -81,7 +81,7 @@ public:
 	explicit CodeTransform(
 		GeneratorState& _state,
 		assembly::Block const& _block,
-		assembly::CodeGenerator::IdentifierAccess const& _identifierAccess = assembly::CodeGenerator::IdentifierAccess()
+		assembly::ExternalIdentifierAccess const& _identifierAccess = assembly::ExternalIdentifierAccess()
 	):
 		m_state(_state),
 		m_scope(*m_state.scopes.at(&_block)),
@@ -160,14 +160,22 @@ public:
 		{
 			return;
 		}
-		solAssert(m_identifierAccess, "Identifier not found and no external access available.");
+		solAssert(
-		if (!m_identifierAccess(_identifier, m_state.assembly, CodeGenerator::IdentifierContext::RValue))
+			m_identifierAccess.resolve && m_identifierAccess.generateCode,
+			"Identifier not found and no external access available."
+		);
+		// @TODO refactor: Store resolved identifier.
+		size_t size = m_identifierAccess.resolve(_identifier, IdentifierContext::RValue);
+		if (size != size_t(-1))
+			m_identifierAccess.generateCode(_identifier, IdentifierContext::RValue, m_state.assembly);
+		else
 		{
 			m_state.addError(
 				Error::Type::DeclarationError,
 				"Identifier not found or not unique",
 				_identifier.location
 			);
+			for (size_t i = 0; i < size; ++i)
 				m_state.assembly.append(u256(0));
 		}
 	}
@@ -236,13 +244,21 @@ private:
 			m_state.assembly.append(solidity::Instruction::POP);
 			return;
 		}
-		solAssert(m_identifierAccess, "Identifier not found and no external access available.");
+		solAssert(
-		if (!m_identifierAccess(_variableName, m_state.assembly, CodeGenerator::IdentifierContext::LValue))
+			m_identifierAccess.resolve && m_identifierAccess.generateCode,
+			"Identifier not found and no external access available."
+		);
+		size_t size = m_identifierAccess.resolve(_variableName, IdentifierContext::LValue);
+		if (size != size_t(-1))
+			m_identifierAccess.generateCode(_variableName, IdentifierContext::LValue, m_state.assembly);
+		else
+		{
 			m_state.addError(
 				Error::Type::DeclarationError,
 				"Identifier \"" + string(_variableName.name) + "\" not found, not unique or not lvalue."
 			);
 		}
+	}
 
 	/// Determines the stack height difference to the given variables. Automatically generates
 	/// errors if it is not yet in scope or the height difference is too large. Returns 0 on
@@ -289,34 +305,34 @@ private:
 	GeneratorState& m_state;
 	Scope& m_scope;
 	int const m_initialDeposit;
-	assembly::CodeGenerator::IdentifierAccess m_identifierAccess;
+	ExternalIdentifierAccess m_identifierAccess;
 };
 
-bool assembly::CodeGenerator::typeCheck(assembly::CodeGenerator::IdentifierAccess const& _identifierAccess)
+bool assembly::CodeGenerator::typeCheck(ExternalIdentifierAccess const& _identifierAccess)
 {
 	size_t initialErrorLen = m_errors.size();
 	eth::Assembly assembly;
 	GeneratorState state(m_errors, assembly);
-	if (!(AsmAnalyzer(state.scopes, m_errors, !!_identifierAccess)).analyze(m_parsedData))
+	if (!(AsmAnalyzer(state.scopes, m_errors, !!_identifierAccess.resolve)).analyze(m_parsedData))
 		return false;
 	CodeTransform(state, m_parsedData, _identifierAccess);
 	return m_errors.size() == initialErrorLen;
 }
 
-eth::Assembly assembly::CodeGenerator::assemble(assembly::CodeGenerator::IdentifierAccess const& _identifierAccess)
+eth::Assembly assembly::CodeGenerator::assemble(ExternalIdentifierAccess const& _identifierAccess)
 {
 	eth::Assembly assembly;
 	GeneratorState state(m_errors, assembly);
-	if (!(AsmAnalyzer(state.scopes, m_errors, !!_identifierAccess)).analyze(m_parsedData))
+	if (!(AsmAnalyzer(state.scopes, m_errors, !!_identifierAccess.resolve)).analyze(m_parsedData))
 		solAssert(false, "Assembly error");
 	CodeTransform(state, m_parsedData, _identifierAccess);
 	return assembly;
 }
 
-void assembly::CodeGenerator::assemble(eth::Assembly& _assembly, assembly::CodeGenerator::IdentifierAccess const& _identifierAccess)
+void assembly::CodeGenerator::assemble(eth::Assembly& _assembly, ExternalIdentifierAccess const& _identifierAccess)
 {
 	GeneratorState state(m_errors, _assembly);
-	if (!(AsmAnalyzer(state.scopes, m_errors, !!_identifierAccess)).analyze(m_parsedData))
+	if (!(AsmAnalyzer(state.scopes, m_errors, !!_identifierAccess.resolve)).analyze(m_parsedData))
 		solAssert(false, "Assembly error");
 	CodeTransform(state, m_parsedData, _identifierAccess);
 }

libsolidity/inlineasm/AsmCodeGen.h

@@ -22,8 +22,10 @@
 
 #pragma once
 
-#include <functional>
 #include <libsolidity/interface/Exceptions.h>
+#include <libsolidity/inlineasm/AsmStack.h>
+
+#include <functional>
 
 namespace dev
 {
@@ -36,27 +38,19 @@ namespace solidity
 namespace assembly
 {
 struct Block;
-struct Identifier;
 
 class CodeGenerator
 {
 public:
-	enum class IdentifierContext { LValue, RValue };
-	/// Function type that is called for external identifiers. Such a function should search for
-	/// the identifier and append appropriate assembly items to the assembly. If in lvalue context,
-	/// the value to assign is assumed to be on the stack and an assignment is to be performed.
-	/// If in rvalue context, the function is assumed to append instructions to
-	/// push the value of the identifier onto the stack. On error, the function should return false.
-	using IdentifierAccess = std::function<bool(assembly::Identifier const&, eth::Assembly&, IdentifierContext)>;
 	CodeGenerator(Block const& _parsedData, ErrorList& _errors):
 		m_parsedData(_parsedData), m_errors(_errors) {}
 	/// Performs type checks and @returns false on error.
 	/// Actually runs the full code generation but discards the result.
-	bool typeCheck(IdentifierAccess const& _identifierAccess = IdentifierAccess());
+	bool typeCheck(ExternalIdentifierAccess const& _identifierAccess = ExternalIdentifierAccess());
 	/// Performs code generation and @returns the result.
-	eth::Assembly assemble(IdentifierAccess const& _identifierAccess = IdentifierAccess());
+	eth::Assembly assemble(ExternalIdentifierAccess const& _identifierAccess = ExternalIdentifierAccess());
 	/// Performs code generation and appends generated to to _assembly.
-	void assemble(eth::Assembly& _assembly, IdentifierAccess const& _identifierAccess = IdentifierAccess());
+	void assemble(eth::Assembly& _assembly, ExternalIdentifierAccess const& _identifierAccess = ExternalIdentifierAccess());
 
 private:
 	Block const& m_parsedData;

libsolidity/inlineasm/AsmStack.cpp

@@ -66,7 +66,7 @@ eth::Assembly InlineAssemblyStack::assemble()
 bool InlineAssemblyStack::parseAndAssemble(
 	string const& _input,
 	eth::Assembly& _assembly,
-	CodeGenerator::IdentifierAccess const& _identifierAccess
+	ExternalIdentifierAccess const& _identifierAccess
 )
 {
 	ErrorList errors;

libsolidity/inlineasm/AsmStack.h

@@ -22,10 +22,10 @@
 
 #pragma once
 
+#include <libsolidity/interface/Exceptions.h>
+
 #include <string>
 #include <functional>
-#include <libsolidity/interface/Exceptions.h>
-#include <libsolidity/inlineasm/AsmCodeGen.h>
 
 namespace dev
 {
@@ -39,6 +39,22 @@ class Scanner;
 namespace assembly
 {
 struct Block;
+struct Identifier;
+
+enum class IdentifierContext { LValue, RValue };
+
+/// Object that is used to resolve references and generate code for access to identifiers external
+/// to inline assembly (not used in standalone assembly mode).
+struct ExternalIdentifierAccess
+{
+	/// Resolve a an external reference given by the identifier in the given context.
+	/// @returns the size of the value (number of stack slots) or size_t(-1) if not found.
+	std::function<size_t(assembly::Identifier const&, IdentifierContext)> resolve;
+	/// Generate code for retrieving the value (rvalue context) or storing the value (lvalue context)
+	/// of an identifier. The code should be appended to the assembly. In rvalue context, the value is supposed
+	/// to be put onto the stack, in lvalue context, the value is assumed to be at the top of the stack.
+	std::function<void(assembly::Identifier const&, IdentifierContext, eth::Assembly&)> generateCode;
+};
 
 class InlineAssemblyStack
 {
@@ -56,7 +72,7 @@ public:
 	bool parseAndAssemble(
 		std::string const& _input,
 		eth::Assembly& _assembly,
-		CodeGenerator::IdentifierAccess const& _identifierAccess = CodeGenerator::IdentifierAccess()
+		ExternalIdentifierAccess const& _identifierAccess = ExternalIdentifierAccess()
 	);
 
 	ErrorList const& errors() const { return m_errors; }