Split external identifier access into resolving and code generation.

2 files changed, 30 insertions, 32 deletions

diff --git a/libsolidity/analysis/ReferencesResolver.cpp b/libsolidity/analysis/ReferencesResolver.cpp
index 37bcb2d9..5ff4ee2d 100644
--- a/libsolidity/analysis/ReferencesResolver.cpp
+++ b/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;
-		_inlineAssembly.annotation().externalReferences[&_identifier] = declarations.front();
-		// At this stage we neither know the code to generate nor the stack size of the identifier,
-		// so we do not modify assembly.
-		return true;
-	});
+			return size_t(-1);
+		_inlineAssembly.annotation().externalReferences[&_identifier].declaration = declarations.front();
+		// At this stage we do not yet know the stack size of the identifier, so we just return 1.
+		return size_t(1);
+	};
+	codeGen.typeCheck(identifierAccess);
 	return false;
 }
 
diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index b37db7b7..808914ae 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/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::CodeGenerator::IdentifierContext _context
+		assembly::IdentifierContext _context
 	)
 	{
 		auto ref = _inlineAssembly.annotation().externalReferences.find(&_identifier);
 		if (ref == _inlineAssembly.annotation().externalReferences.end())
-			return false;
-		Declaration const* declaration = ref->second;
+			return size_t(-1);
+		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;
-				pushes = 1;
+					return size_t(-1);
+				valueSize = 1;
 			}
 			else
-				return false;
-			for (unsigned i = 0; i < pushes; ++i)
-				_assembly.append(u256(0)); // just to verify the stack height
+				return size_t(-1);
 		}
 		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
-				for (unsigned i = 0; i < declaration->type()->sizeOnStack(); ++i)
-					_assembly.append(Instruction::POP); // remove value just to verify the stack height
+					return size_t(-1); // only local variables are inline-assembly lvalues
+				valueSize = size_t(declaration->type()->sizeOnStack());
 			}
 			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))