Refactor name and type resolution.

libsolidity/analysis/NameAndTypeResolver.cpp

@@ -123,11 +123,13 @@ bool NameAndTypeResolver::performImports(SourceUnit& _sourceUnit, map<string, So
 	return !error;
 }
 
-bool NameAndTypeResolver::resolveNamesAndTypes(ASTNode& _node, bool _resolveInsideCode)
+bool NameAndTypeResolver::resolveNamesAndTypes(SourceUnit& _source)
 {
 	try
 	{
-		return resolveNamesAndTypesInternal(_node, _resolveInsideCode);
+		for (shared_ptr<ASTNode> const& node: _source.nodes())
+			if (!resolveNamesAndTypesInternal(*node, true))
+				return false;
 	}
 	catch (langutil::FatalError const&)
 	{
@@ -135,6 +137,7 @@ bool NameAndTypeResolver::resolveNamesAndTypes(ASTNode& _node, bool _resolveInsi
 			throw; // Something is weird here, rather throw again.
 		return false;
 	}
+	return true;
 }
 
 bool NameAndTypeResolver::updateDeclaration(Declaration const& _declaration)
@@ -227,13 +230,14 @@ bool NameAndTypeResolver::resolveNamesAndTypesInternal(ASTNode& _node, bool _res
 		bool success = true;
 		setScope(contract->scope());
 		solAssert(!!m_currentScope, "");
+		solAssert(_resolveInsideCode, "");
 
 		m_globalContext.setCurrentContract(*contract);
 		updateDeclaration(*m_globalContext.currentSuper());
 		updateDeclaration(*m_globalContext.currentThis());
 
 		for (ASTPointer<InheritanceSpecifier> const& baseContract: contract->baseContracts())
-			if (!resolveNamesAndTypes(*baseContract, true))
+			if (!resolveNamesAndTypesInternal(*baseContract, true))
 				success = false;
 
 		setScope(contract);
@@ -254,23 +258,20 @@ bool NameAndTypeResolver::resolveNamesAndTypesInternal(ASTNode& _node, bool _res
 		for (ASTPointer<ASTNode> const& node: contract->subNodes())
 		{
 			setScope(contract);
-			if (!resolveNamesAndTypes(*node, false))
+			if (!resolveNamesAndTypesInternal(*node, false))
 				success = false;
 		}
 
 		if (!success)
 			return false;
 
-		if (!_resolveInsideCode)
-			return success;
-
 		setScope(contract);
 
 		// now resolve references inside the code
 		for (ASTPointer<ASTNode> const& node: contract->subNodes())
 		{
 			setScope(contract);
-			if (!resolveNamesAndTypes(*node, true))
+			if (!resolveNamesAndTypesInternal(*node, true))
 				success = false;
 		}
 		return success;

libsolidity/analysis/NameAndTypeResolver.h

@@ -65,12 +65,9 @@ public:
 	bool registerDeclarations(SourceUnit& _sourceUnit, ASTNode const* _currentScope = nullptr);
 	/// Applies the effect of import directives.
 	bool performImports(SourceUnit& _sourceUnit, std::map<std::string, SourceUnit const*> const& _sourceUnits);
-	/// Resolves all names and types referenced from the given AST Node.
+	/// Resolves all names and types referenced from the given Source Node.
-	/// This is usually only called at the contract level, but with a bit of care, it can also
-	/// be called at deeper levels.
-	/// @param _resolveInsideCode if false, does not descend into nodes that contain code.
 	/// @returns false in case of error.
-	bool resolveNamesAndTypes(ASTNode& _node, bool _resolveInsideCode = true);
+	bool resolveNamesAndTypes(SourceUnit& _source);
 	/// Updates the given global declaration (used for "this"). Not to be used with declarations
 	/// that create their own scope.
 	/// @returns false in case of error.

libsolidity/analysis/TypeChecker.cpp

@@ -62,9 +62,9 @@ bool TypeChecker::typeSupportedByOldABIEncoder(Type const& _type, bool _isLibrar
 	return true;
 }
 
-bool TypeChecker::checkTypeRequirements(ASTNode const& _contract)
+bool TypeChecker::checkTypeRequirements(SourceUnit const& _source)
 {
-	_contract.accept(*this);
+	_source.accept(*this);
 	return Error::containsOnlyWarnings(m_errorReporter.errors());
 }
 

libsolidity/analysis/TypeChecker.h

@@ -51,9 +51,9 @@ public:
 		m_errorReporter(_errorReporter)
 	{}
 
-	/// Performs type checking on the given contract and all of its sub-nodes.
+	/// Performs type checking on the given source and all of its sub-nodes.
 	/// @returns true iff all checks passed. Note even if all checks passed, errors() can still contain warnings
-	bool checkTypeRequirements(ASTNode const& _contract);
+	bool checkTypeRequirements(SourceUnit const& _source);
 
 	/// @returns the type of an expression and asserts that it is present.
 	TypePointer const& type(Expression const& _expression) const;

libsolidity/interface/CompilerStack.cpp

@@ -326,28 +326,28 @@ bool CompilerStack::analyze()
 			if (source->ast && !resolver.performImports(*source->ast, sourceUnitsByName))
 				return false;
 
-		// This is the main name and type resolution loop. Needs to be run for every contract, because
+		for (Source const* source: m_sourceOrder)
-		// the special variables "this" and "super" must be set appropriately.
+			if (source->ast && !resolver.resolveNamesAndTypes(*source->ast))
+				return false;
+
+		// Store contract definitions.
 		for (Source const* source: m_sourceOrder)
 			if (source->ast)
-				for (ASTPointer<ASTNode> const& node: source->ast->nodes())
+				for (
+					ContractDefinition const* contract:
+					ASTNode::filteredNodes<ContractDefinition>(source->ast->nodes())
+				)
 				{
-					if (!resolver.resolveNamesAndTypes(*node))
+					// Note that we now reference contracts by their fully qualified names, and
-						return false;
+					// thus contracts can only conflict if declared in the same source file. This
-					if (ContractDefinition* contract = dynamic_cast<ContractDefinition*>(node.get()))
+					// should already cause a double-declaration error elsewhere.
-					{
+					if (!m_contracts.count(contract->fullyQualifiedName()))
-						// Note that we now reference contracts by their fully qualified names, and
+						m_contracts[contract->fullyQualifiedName()].contract = contract;
-						// thus contracts can only conflict if declared in the same source file. This
+					else
-						// should already cause a double-declaration error elsewhere.
+						solAssert(
-						if (m_contracts.find(contract->fullyQualifiedName()) == m_contracts.end())
+							m_errorReporter.hasErrors(),
-							m_contracts[contract->fullyQualifiedName()].contract = contract;
+							"Contract already present (name clash?), but no error was reported."
-						else
+						);
-							solAssert(
-								m_errorReporter.hasErrors(),
-								"Contract already present (name clash?), but no error was reported."
-							);
-					}
-
 				}
 
 		DeclarationTypeChecker declarationTypeChecker(m_errorReporter, m_evmVersion);
@@ -376,11 +376,8 @@ bool CompilerStack::analyze()
 		// which is only done one step later.
 		TypeChecker typeChecker(m_evmVersion, m_errorReporter);
 		for (Source const* source: m_sourceOrder)
-			if (source->ast)
+			if (source->ast && !typeChecker.checkTypeRequirements(*source->ast))
-				for (ASTPointer<ASTNode> const& node: source->ast->nodes())
+				noErrors = false;
-					if (ContractDefinition* contract = dynamic_cast<ContractDefinition*>(node.get()))
-						if (!typeChecker.checkTypeRequirements(*contract))
-							noErrors = false;
 
 		if (noErrors)
 		{

test/libsolidity/Assembly.cpp

@@ -63,27 +63,19 @@ evmasm::AssemblyItems compileContract(std::shared_ptr<CharStream> _sourceCode)
 	DeclarationTypeChecker declarationTypeChecker(errorReporter, solidity::test::CommonOptions::get().evmVersion());
 	solAssert(Error::containsOnlyWarnings(errorReporter.errors()), "");
 	resolver.registerDeclarations(*sourceUnit);
-	for (ASTPointer<ASTNode> const& node: sourceUnit->nodes())
+	BOOST_REQUIRE_NO_THROW(resolver.resolveNamesAndTypes(*sourceUnit));
-		if (ContractDefinition* contract = dynamic_cast<ContractDefinition*>(node.get()))
+	if (!Error::containsOnlyWarnings(errorReporter.errors()))
-		{
+		return AssemblyItems();
-			BOOST_REQUIRE_NO_THROW(resolver.resolveNamesAndTypes(*contract));
-			if (!Error::containsOnlyWarnings(errorReporter.errors()))
-				return AssemblyItems();
-		}
 	for (ASTPointer<ASTNode> const& node: sourceUnit->nodes())
 	{
 		BOOST_REQUIRE_NO_THROW(declarationTypeChecker.check(*node));
 		if (!Error::containsOnlyWarnings(errorReporter.errors()))
 			return AssemblyItems();
 	}
-	for (ASTPointer<ASTNode> const& node: sourceUnit->nodes())
+	TypeChecker checker(solidity::test::CommonOptions::get().evmVersion(), errorReporter);
-		if (ContractDefinition* contract = dynamic_cast<ContractDefinition*>(node.get()))
+	BOOST_REQUIRE_NO_THROW(checker.checkTypeRequirements(*sourceUnit));
-		{
+	if (!Error::containsOnlyWarnings(errorReporter.errors()))
-			TypeChecker checker(solidity::test::CommonOptions::get().evmVersion(), errorReporter);
+		return AssemblyItems();
-			BOOST_REQUIRE_NO_THROW(checker.checkTypeRequirements(*contract));
-			if (!Error::containsOnlyWarnings(errorReporter.errors()))
-				return AssemblyItems();
-		}
 	for (ASTPointer<ASTNode> const& node: sourceUnit->nodes())
 		if (ContractDefinition* contract = dynamic_cast<ContractDefinition*>(node.get()))
 		{

test/libsolidity/SolidityExpressionCompiler.cpp

@@ -118,19 +118,12 @@ bytes compileFirstExpression(
 	GlobalContext globalContext;
 	NameAndTypeResolver resolver(globalContext, solidity::test::CommonOptions::get().evmVersion(), errorReporter);
 	resolver.registerDeclarations(*sourceUnit);
-	for (ASTPointer<ASTNode> const& node: sourceUnit->nodes())
+	BOOST_REQUIRE_MESSAGE(resolver.resolveNamesAndTypes(*sourceUnit), "Resolving names failed");
-		if (ContractDefinition* contract = dynamic_cast<ContractDefinition*>(node.get()))
-			BOOST_REQUIRE_MESSAGE(resolver.resolveNamesAndTypes(*contract), "Resolving names failed");
 	DeclarationTypeChecker declarationTypeChecker(errorReporter, solidity::test::CommonOptions::get().evmVersion());
 	for (ASTPointer<ASTNode> const& node: sourceUnit->nodes())
 		BOOST_REQUIRE(declarationTypeChecker.check(*node));
-	for (ASTPointer<ASTNode> const& node: sourceUnit->nodes())
+	TypeChecker typeChecker(solidity::test::CommonOptions::get().evmVersion(), errorReporter);
-		if (ContractDefinition* contract = dynamic_cast<ContractDefinition*>(node.get()))
+	BOOST_REQUIRE(typeChecker.checkTypeRequirements(*sourceUnit));
-		{
-			ErrorReporter errorReporter(errors);
-			TypeChecker typeChecker(solidity::test::CommonOptions::get().evmVersion(), errorReporter);
-			BOOST_REQUIRE(typeChecker.checkTypeRequirements(*contract));
-		}
 	for (ASTPointer<ASTNode> const& node: sourceUnit->nodes())
 		if (ContractDefinition* contract = dynamic_cast<ContractDefinition*>(node.get()))
 		{