From 07c116713610e94eba4c1ae9004d2a3c3d24aa10 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 11 Jun 2020 17:17:07 +0200 Subject: [PATCH] Refactor name and type resolution. --- libsolidity/analysis/NameAndTypeResolver.cpp | 17 +++---- libsolidity/analysis/NameAndTypeResolver.h | 7 +-- libsolidity/analysis/TypeChecker.cpp | 4 +- libsolidity/analysis/TypeChecker.h | 4 +- libsolidity/interface/CompilerStack.cpp | 45 +++++++++---------- test/libsolidity/Assembly.cpp | 22 +++------ .../SolidityExpressionCompiler.cpp | 13 ++---- 7 files changed, 46 insertions(+), 66 deletions(-) diff --git a/libsolidity/analysis/NameAndTypeResolver.cpp b/libsolidity/analysis/NameAndTypeResolver.cpp index fa1840d06..9fbd219f9 100644 --- a/libsolidity/analysis/NameAndTypeResolver.cpp +++ b/libsolidity/analysis/NameAndTypeResolver.cpp @@ -123,11 +123,13 @@ bool NameAndTypeResolver::performImports(SourceUnit& _sourceUnit, map 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 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 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 const& node: contract->subNodes()) { setScope(contract); - if (!resolveNamesAndTypes(*node, true)) + if (!resolveNamesAndTypesInternal(*node, true)) success = false; } return success; diff --git a/libsolidity/analysis/NameAndTypeResolver.h b/libsolidity/analysis/NameAndTypeResolver.h index 8a315510f..efb404038 100644 --- a/libsolidity/analysis/NameAndTypeResolver.h +++ b/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 const& _sourceUnits); - /// Resolves all names and types referenced from the given AST 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. + /// Resolves all names and types referenced from the given Source Node. /// @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. diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index c308fadd8..a85f4f9e9 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/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()); } diff --git a/libsolidity/analysis/TypeChecker.h b/libsolidity/analysis/TypeChecker.h index 9997c700a..948d63648 100644 --- a/libsolidity/analysis/TypeChecker.h +++ b/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; diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index a8efdfb0b..eef773095 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/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 - // the special variables "this" and "super" must be set appropriately. + for (Source const* source: m_sourceOrder) + if (source->ast && !resolver.resolveNamesAndTypes(*source->ast)) + return false; + + // Store contract definitions. for (Source const* source: m_sourceOrder) if (source->ast) - for (ASTPointer const& node: source->ast->nodes()) + for ( + ContractDefinition const* contract: + ASTNode::filteredNodes(source->ast->nodes()) + ) { - if (!resolver.resolveNamesAndTypes(*node)) - return false; - if (ContractDefinition* contract = dynamic_cast(node.get())) - { - // Note that we now reference contracts by their fully qualified names, and - // thus contracts can only conflict if declared in the same source file. This - // should already cause a double-declaration error elsewhere. - if (m_contracts.find(contract->fullyQualifiedName()) == m_contracts.end()) - m_contracts[contract->fullyQualifiedName()].contract = contract; - else - solAssert( - m_errorReporter.hasErrors(), - "Contract already present (name clash?), but no error was reported." - ); - } - + // Note that we now reference contracts by their fully qualified names, and + // thus contracts can only conflict if declared in the same source file. This + // should already cause a double-declaration error elsewhere. + if (!m_contracts.count(contract->fullyQualifiedName())) + m_contracts[contract->fullyQualifiedName()].contract = contract; + 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) - for (ASTPointer const& node: source->ast->nodes()) - if (ContractDefinition* contract = dynamic_cast(node.get())) - if (!typeChecker.checkTypeRequirements(*contract)) - noErrors = false; + if (source->ast && !typeChecker.checkTypeRequirements(*source->ast)) + noErrors = false; if (noErrors) { diff --git a/test/libsolidity/Assembly.cpp b/test/libsolidity/Assembly.cpp index 0484bf95c..885f0b2d3 100644 --- a/test/libsolidity/Assembly.cpp +++ b/test/libsolidity/Assembly.cpp @@ -63,27 +63,19 @@ evmasm::AssemblyItems compileContract(std::shared_ptr _sourceCode) DeclarationTypeChecker declarationTypeChecker(errorReporter, solidity::test::CommonOptions::get().evmVersion()); solAssert(Error::containsOnlyWarnings(errorReporter.errors()), ""); resolver.registerDeclarations(*sourceUnit); - for (ASTPointer const& node: sourceUnit->nodes()) - if (ContractDefinition* contract = dynamic_cast(node.get())) - { - BOOST_REQUIRE_NO_THROW(resolver.resolveNamesAndTypes(*contract)); - if (!Error::containsOnlyWarnings(errorReporter.errors())) - return AssemblyItems(); - } + BOOST_REQUIRE_NO_THROW(resolver.resolveNamesAndTypes(*sourceUnit)); + if (!Error::containsOnlyWarnings(errorReporter.errors())) + return AssemblyItems(); for (ASTPointer const& node: sourceUnit->nodes()) { BOOST_REQUIRE_NO_THROW(declarationTypeChecker.check(*node)); if (!Error::containsOnlyWarnings(errorReporter.errors())) return AssemblyItems(); } - for (ASTPointer const& node: sourceUnit->nodes()) - if (ContractDefinition* contract = dynamic_cast(node.get())) - { - TypeChecker checker(solidity::test::CommonOptions::get().evmVersion(), errorReporter); - BOOST_REQUIRE_NO_THROW(checker.checkTypeRequirements(*contract)); - if (!Error::containsOnlyWarnings(errorReporter.errors())) - return AssemblyItems(); - } + TypeChecker checker(solidity::test::CommonOptions::get().evmVersion(), errorReporter); + BOOST_REQUIRE_NO_THROW(checker.checkTypeRequirements(*sourceUnit)); + if (!Error::containsOnlyWarnings(errorReporter.errors())) + return AssemblyItems(); for (ASTPointer const& node: sourceUnit->nodes()) if (ContractDefinition* contract = dynamic_cast(node.get())) { diff --git a/test/libsolidity/SolidityExpressionCompiler.cpp b/test/libsolidity/SolidityExpressionCompiler.cpp index 41c850b4d..4d1e9e23a 100644 --- a/test/libsolidity/SolidityExpressionCompiler.cpp +++ b/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 const& node: sourceUnit->nodes()) - if (ContractDefinition* contract = dynamic_cast(node.get())) - BOOST_REQUIRE_MESSAGE(resolver.resolveNamesAndTypes(*contract), "Resolving names failed"); + BOOST_REQUIRE_MESSAGE(resolver.resolveNamesAndTypes(*sourceUnit), "Resolving names failed"); DeclarationTypeChecker declarationTypeChecker(errorReporter, solidity::test::CommonOptions::get().evmVersion()); for (ASTPointer const& node: sourceUnit->nodes()) BOOST_REQUIRE(declarationTypeChecker.check(*node)); - for (ASTPointer const& node: sourceUnit->nodes()) - if (ContractDefinition* contract = dynamic_cast(node.get())) - { - ErrorReporter errorReporter(errors); - TypeChecker typeChecker(solidity::test::CommonOptions::get().evmVersion(), errorReporter); - BOOST_REQUIRE(typeChecker.checkTypeRequirements(*contract)); - } + TypeChecker typeChecker(solidity::test::CommonOptions::get().evmVersion(), errorReporter); + BOOST_REQUIRE(typeChecker.checkTypeRequirements(*sourceUnit)); for (ASTPointer const& node: sourceUnit->nodes()) if (ContractDefinition* contract = dynamic_cast(node.get())) {