Merge pull request #9586 from ethereum/scoper

Assign scopes as a separate step.

libsolidity/CMakeLists.txt

@@ -30,6 +30,8 @@ set(sources
 	analysis/PostTypeChecker.h
 	analysis/ReferencesResolver.cpp
 	analysis/ReferencesResolver.h
+	analysis/Scoper.cpp
+	analysis/Scoper.h
 	analysis/StaticAnalyzer.cpp
 	analysis/StaticAnalyzer.h
 	analysis/SyntaxChecker.cpp

libsolidity/analysis/NameAndTypeResolver.cpp

@@ -519,14 +519,13 @@ bool DeclarationRegistrationHelper::visit(SourceUnit& _sourceUnit)
 	if (!m_scopes[&_sourceUnit])
 		// By importing, it is possible that the container already exists.
 		m_scopes[&_sourceUnit] = make_shared<DeclarationContainer>(m_currentScope, m_scopes[m_currentScope].get());
-	m_currentScope = &_sourceUnit;
+	return ASTVisitor::visit(_sourceUnit);
-	return true;
 }
 
 void DeclarationRegistrationHelper::endVisit(SourceUnit& _sourceUnit)
 {
 	_sourceUnit.annotation().exportedSymbols = m_scopes[&_sourceUnit]->declarations();
-	closeCurrentScope();
+	ASTVisitor::endVisit(_sourceUnit);
 }
 
 bool DeclarationRegistrationHelper::visit(ImportDirective& _import)
@@ -536,8 +535,7 @@ bool DeclarationRegistrationHelper::visit(ImportDirective& _import)
 	if (!m_scopes[importee])
 		m_scopes[importee] = make_shared<DeclarationContainer>(nullptr, m_scopes[nullptr].get());
 	m_scopes[&_import] = m_scopes[importee];
-	registerDeclaration(_import, false);
+	return ASTVisitor::visit(_import);
-	return true;
 }
 
 bool DeclarationRegistrationHelper::visit(ContractDefinition& _contract)
@@ -547,122 +545,17 @@ bool DeclarationRegistrationHelper::visit(ContractDefinition& _contract)
 	m_scopes[nullptr]->registerDeclaration(*m_globalContext.currentSuper(), nullptr, false, true);
 	m_currentContract = &_contract;
 
-	registerDeclaration(_contract, true);
+	return ASTVisitor::visit(_contract);
-	_contract.annotation().canonicalName = currentCanonicalName();
-	return true;
 }
 
-void DeclarationRegistrationHelper::endVisit(ContractDefinition&)
+void DeclarationRegistrationHelper::endVisit(ContractDefinition& _contract)
 {
 	// make "this" and "super" invisible.
 	m_scopes[nullptr]->registerDeclaration(*m_globalContext.currentThis(), nullptr, true, true);
 	m_scopes[nullptr]->registerDeclaration(*m_globalContext.currentSuper(), nullptr, true, true);
 	m_globalContext.resetCurrentContract();
 	m_currentContract = nullptr;
-	closeCurrentScope();
+	ASTVisitor::endVisit(_contract);
-}
-
-bool DeclarationRegistrationHelper::visit(StructDefinition& _struct)
-{
-	registerDeclaration(_struct, true);
-	_struct.annotation().canonicalName = currentCanonicalName();
-	return true;
-}
-
-void DeclarationRegistrationHelper::endVisit(StructDefinition&)
-{
-	closeCurrentScope();
-}
-
-bool DeclarationRegistrationHelper::visit(EnumDefinition& _enum)
-{
-	registerDeclaration(_enum, true);
-	_enum.annotation().canonicalName = currentCanonicalName();
-	return true;
-}
-
-void DeclarationRegistrationHelper::endVisit(EnumDefinition&)
-{
-	closeCurrentScope();
-}
-
-bool DeclarationRegistrationHelper::visit(EnumValue& _value)
-{
-	registerDeclaration(_value, false);
-	return true;
-}
-
-bool DeclarationRegistrationHelper::visit(FunctionDefinition& _function)
-{
-	registerDeclaration(_function, true);
-	m_currentFunction = &_function;
-	return true;
-}
-
-void DeclarationRegistrationHelper::endVisit(FunctionDefinition&)
-{
-	m_currentFunction = nullptr;
-	closeCurrentScope();
-}
-
-bool DeclarationRegistrationHelper::visit(TryCatchClause& _tryCatchClause)
-{
-	_tryCatchClause.annotation().scope = m_currentScope;
-	enterNewSubScope(_tryCatchClause);
-	return true;
-}
-
-void DeclarationRegistrationHelper::endVisit(TryCatchClause&)
-{
-	closeCurrentScope();
-}
-
-bool DeclarationRegistrationHelper::visit(ModifierDefinition& _modifier)
-{
-	registerDeclaration(_modifier, true);
-	m_currentFunction = &_modifier;
-	return true;
-}
-
-void DeclarationRegistrationHelper::endVisit(ModifierDefinition&)
-{
-	m_currentFunction = nullptr;
-	closeCurrentScope();
-}
-
-bool DeclarationRegistrationHelper::visit(FunctionTypeName& _funTypeName)
-{
-	enterNewSubScope(_funTypeName);
-	return true;
-}
-
-void DeclarationRegistrationHelper::endVisit(FunctionTypeName&)
-{
-	closeCurrentScope();
-}
-
-bool DeclarationRegistrationHelper::visit(Block& _block)
-{
-	_block.annotation().scope = m_currentScope;
-	enterNewSubScope(_block);
-	return true;
-}
-
-void DeclarationRegistrationHelper::endVisit(Block&)
-{
-	closeCurrentScope();
-}
-
-bool DeclarationRegistrationHelper::visit(ForStatement& _for)
-{
-	_for.annotation().scope = m_currentScope;
-	enterNewSubScope(_for);
-	return true;
-}
-
-void DeclarationRegistrationHelper::endVisit(ForStatement&)
-{
-	closeCurrentScope();
 }
 
 void DeclarationRegistrationHelper::endVisit(VariableDeclarationStatement& _variableDeclarationStatement)
@@ -673,32 +566,42 @@ void DeclarationRegistrationHelper::endVisit(VariableDeclarationStatement& _vari
 	for (ASTPointer<VariableDeclaration> const& var: _variableDeclarationStatement.declarations())
 		if (var)
 			m_currentFunction->addLocalVariable(*var);
+	ASTVisitor::endVisit(_variableDeclarationStatement);
 }
 
-bool DeclarationRegistrationHelper::visit(VariableDeclaration& _declaration)
+bool DeclarationRegistrationHelper::visitNode(ASTNode& _node)
 {
-	registerDeclaration(_declaration, false);
+	if (auto const* scopable = dynamic_cast<Scopable const*>(&_node))
+		solAssert(scopable->annotation().scope == m_currentScope, "");
+
+	if (auto* declaration = dynamic_cast<Declaration*>(&_node))
+		registerDeclaration(*declaration);
+	if (dynamic_cast<ScopeOpener const*>(&_node))
+		enterNewSubScope(_node);
+
+	if (auto* variableScope = dynamic_cast<VariableScope*>(&_node))
+		m_currentFunction = variableScope;
+	if (auto* annotation = dynamic_cast<TypeDeclarationAnnotation*>(&_node.annotation()))
+		annotation->canonicalName = currentCanonicalName();
+
 	return true;
 }
 
-bool DeclarationRegistrationHelper::visit(EventDefinition& _event)
+void DeclarationRegistrationHelper::endVisitNode(ASTNode& _node)
 {
-	registerDeclaration(_event, true);
+	if (dynamic_cast<ScopeOpener const*>(&_node))
-	return true;
+		closeCurrentScope();
-}
+	if (dynamic_cast<VariableScope*>(&_node))
-
+		m_currentFunction = nullptr;
-void DeclarationRegistrationHelper::endVisit(EventDefinition&)
-{
-	closeCurrentScope();
 }
 
 void DeclarationRegistrationHelper::enterNewSubScope(ASTNode& _subScope)
 {
-	map<ASTNode const*, shared_ptr<DeclarationContainer>>::iterator iter;
-	bool newlyAdded;
 	shared_ptr<DeclarationContainer> container{make_shared<DeclarationContainer>(m_currentScope, m_scopes[m_currentScope].get())};
-	tie(iter, newlyAdded) = m_scopes.emplace(&_subScope, move(container));
+	bool newlyAdded = m_scopes.emplace(&_subScope, move(container)).second;
-	solAssert(newlyAdded, "Unable to add new scope.");
+	// Source units are the only AST nodes for which containers can be created from multiple places
+	// due to imports.
+	solAssert(newlyAdded || dynamic_cast<SourceUnit const*>(&_subScope), "Unable to add new scope.");
 	m_currentScope = &_subScope;
 }
 
@@ -708,7 +611,7 @@ void DeclarationRegistrationHelper::closeCurrentScope()
 	m_currentScope = m_scopes[m_currentScope]->enclosingNode();
 }
 
-void DeclarationRegistrationHelper::registerDeclaration(Declaration& _declaration, bool _opensScope)
+void DeclarationRegistrationHelper::registerDeclaration(Declaration& _declaration)
 {
 	solAssert(m_currentScope && m_scopes.count(m_currentScope), "No current scope.");
 
@@ -729,10 +632,8 @@ void DeclarationRegistrationHelper::registerDeclaration(Declaration& _declaratio
 
 	registerDeclaration(*m_scopes[m_currentScope], _declaration, nullptr, nullptr, warnAboutShadowing, inactive, m_errorReporter);
 
-	_declaration.annotation().scope = m_currentScope;
+	solAssert(_declaration.annotation().scope == m_currentScope, "");
-	_declaration.annotation().contract = m_currentContract;
+	solAssert(_declaration.annotation().contract == m_currentContract, "");
-	if (_opensScope)
-		enterNewSubScope(_declaration);
 }
 
 string DeclarationRegistrationHelper::currentCanonicalName() const

libsolidity/analysis/NameAndTypeResolver.h

@@ -163,31 +163,15 @@ private:
 	bool visit(ImportDirective& _import) override;
 	bool visit(ContractDefinition& _contract) override;
 	void endVisit(ContractDefinition& _contract) override;
-	bool visit(StructDefinition& _struct) override;
-	void endVisit(StructDefinition& _struct) override;
-	bool visit(EnumDefinition& _enum) override;
-	void endVisit(EnumDefinition& _enum) override;
-	bool visit(EnumValue& _value) override;
-	bool visit(FunctionDefinition& _function) override;
-	void endVisit(FunctionDefinition& _function) override;
-	bool visit(TryCatchClause& _tryCatchClause) override;
-	void endVisit(TryCatchClause& _tryCatchClause) override;
-	bool visit(ModifierDefinition& _modifier) override;
-	void endVisit(ModifierDefinition& _modifier) override;
-	bool visit(FunctionTypeName& _funTypeName) override;
-	void endVisit(FunctionTypeName& _funTypeName) override;
-	bool visit(Block& _block) override;
-	void endVisit(Block& _block) override;
-	bool visit(ForStatement& _forLoop) override;
-	void endVisit(ForStatement& _forLoop) override;
 	void endVisit(VariableDeclarationStatement& _variableDeclarationStatement) override;
-	bool visit(VariableDeclaration& _declaration) override;
+
-	bool visit(EventDefinition& _event) override;
+	bool visitNode(ASTNode& _node) override;
-	void endVisit(EventDefinition& _event) override;
+	void endVisitNode(ASTNode& _node) override;
+
 
 	void enterNewSubScope(ASTNode& _subScope);
 	void closeCurrentScope();
-	void registerDeclaration(Declaration& _declaration, bool _opensScope);
+	void registerDeclaration(Declaration& _declaration);
 
 	static bool isOverloadedFunction(Declaration const& _declaration1, Declaration const& _declaration2);
 

libsolidity/analysis/Scoper.cpp

@@ -0,0 +1,63 @@
+/*
+	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/>.
+*/
+// SPDX-License-Identifier: GPL-3.0
+
+#include <libsolidity/analysis/Scoper.h>
+
+#include <libsolidity/ast/AST.h>
+
+using namespace std;
+using namespace solidity;
+using namespace solidity::frontend;
+
+void Scoper::assignScopes(ASTNode const& _astRoot)
+{
+	Scoper scoper;
+	_astRoot.accept(scoper);
+}
+
+bool Scoper::visit(ContractDefinition const& _contract)
+{
+	solAssert(m_contract == nullptr, "");
+	m_contract = &_contract;
+	return ASTConstVisitor::visit(_contract);
+}
+
+void Scoper::endVisit(ContractDefinition const& _contract)
+{
+	solAssert(m_contract == &_contract, "");
+	m_contract = nullptr;
+	ASTConstVisitor::endVisit(_contract);
+}
+
+bool Scoper::visitNode(ASTNode const& _node)
+{
+	if (auto const* scopable = dynamic_cast<Scopable const*>(&_node))
+	{
+		scopable->annotation().scope = m_scopes.empty() ? nullptr : m_scopes.back();
+		scopable->annotation().contract = m_contract;
+	}
+	if (dynamic_cast<ScopeOpener const*>(&_node))
+		m_scopes.push_back(&_node);
+	return true;
+}
+
+void Scoper::endVisitNode(ASTNode const& _node)
+{
+	if (dynamic_cast<ScopeOpener const*>(&_node))
+		m_scopes.pop_back();
+}

libsolidity/analysis/Scoper.h

@@ -0,0 +1,45 @@
+/*
+	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/>.
+*/
+// SPDX-License-Identifier: GPL-3.0
+
+#pragma once
+
+#include <libsolidity/ast/ASTForward.h>
+#include <libsolidity/ast/ASTVisitor.h>
+
+namespace solidity::frontend
+{
+
+/**
+ * AST visitor that assigns syntactic scopes.
+ */
+class Scoper: private ASTConstVisitor
+{
+public:
+	static void assignScopes(ASTNode const& _astRoot);
+
+private:
+	bool visit(ContractDefinition const& _contract) override;
+	void endVisit(ContractDefinition const& _contract) override;
+	bool visitNode(ASTNode const& _node) override;
+	void endVisitNode(ASTNode const& _node) override;
+
+	ContractDefinition const* m_contract = nullptr;
+	std::vector<ASTNode const*> m_scopes;
+};
+
+}

libsolidity/ast/AST.h

@@ -152,10 +152,19 @@ std::vector<T const*> ASTNode::filteredNodes(std::vector<ASTPointer<ASTNode>> co
 	return ret;
 }
 
+/**
+ * Abstract marker class that specifies that this AST node opens a scope.
+ */
+class ScopeOpener
+{
+public:
+	virtual ~ScopeOpener() = default;
+};
+
 /**
  * Source unit containing import directives and contract definitions.
  */
-class SourceUnit: public ASTNode
+class SourceUnit: public ASTNode, public ScopeOpener
 {
 public:
 	SourceUnit(
@@ -455,7 +464,7 @@ protected:
  * document order. It first visits all struct declarations, then all variable declarations and
  * finally all function declarations.
  */
-class ContractDefinition: public Declaration, public StructurallyDocumented
+class ContractDefinition: public Declaration, public StructurallyDocumented, public ScopeOpener
 {
 public:
 	ContractDefinition(
@@ -593,7 +602,7 @@ private:
 	ASTPointer<TypeName> m_typeName;
 };
 
-class StructDefinition: public Declaration
+class StructDefinition: public Declaration, public ScopeOpener
 {
 public:
 	StructDefinition(
@@ -620,7 +629,7 @@ private:
 	std::vector<ASTPointer<VariableDeclaration>> m_members;
 };
 
-class EnumDefinition: public Declaration
+class EnumDefinition: public Declaration, public ScopeOpener
 {
 public:
 	EnumDefinition(
@@ -765,7 +774,7 @@ protected:
 	std::vector<ASTPointer<UserDefinedTypeName>> m_overrides;
 };
 
-class FunctionDefinition: public CallableDeclaration, public StructurallyDocumented, public ImplementationOptional
+class FunctionDefinition: public CallableDeclaration, public StructurallyDocumented, public ImplementationOptional, public ScopeOpener
 {
 public:
 	FunctionDefinition(
@@ -989,7 +998,7 @@ private:
 /**
  * Definition of a function modifier.
  */
-class ModifierDefinition: public CallableDeclaration, public StructurallyDocumented, public ImplementationOptional
+class ModifierDefinition: public CallableDeclaration, public StructurallyDocumented, public ImplementationOptional, public ScopeOpener
 {
 public:
 	ModifierDefinition(
@@ -1061,7 +1070,7 @@ private:
 /**
  * Definition of a (loggable) event.
  */
-class EventDefinition: public CallableDeclaration, public StructurallyDocumented
+class EventDefinition: public CallableDeclaration, public StructurallyDocumented, public ScopeOpener
 {
 public:
 	EventDefinition(
@@ -1199,7 +1208,7 @@ private:
 /**
  * A literal function type. Its source form is "function (paramType1, paramType2) internal / external returns (retType1, retType2)"
  */
-class FunctionTypeName: public TypeName
+class FunctionTypeName: public TypeName, public ScopeOpener
 {
 public:
 	FunctionTypeName(
@@ -1334,7 +1343,7 @@ private:
 /**
  * Brace-enclosed block containing zero or more statements.
  */
-class Block: public Statement, public Scopable
+class Block: public Statement, public Scopable, public ScopeOpener
 {
 public:
 	Block(
@@ -1411,7 +1420,7 @@ private:
  * unsuccessful cases.
  * Names are only allowed for the unsuccessful cases.
  */
-class TryCatchClause: public ASTNode, public Scopable
+class TryCatchClause: public ASTNode, public Scopable, public ScopeOpener
 {
 public:
 	TryCatchClause(
@@ -1526,7 +1535,7 @@ private:
 /**
  * For loop statement
  */
-class ForStatement: public BreakableStatement, public Scopable
+class ForStatement: public BreakableStatement, public Scopable, public ScopeOpener
 {
 public:
 	ForStatement(

libsolidity/ast/ASTAnnotations.h

@@ -108,10 +108,10 @@ struct ScopableAnnotation
 	virtual ~ScopableAnnotation() = default;
 
 	/// The scope this declaration resides in. Can be nullptr if it is the global scope.
-	/// Available only after name and type resolution step.
+	/// Filled by the Scoper.
 	ASTNode const* scope = nullptr;
 	/// Pointer to the contract this declaration resides in. Can be nullptr if the current scope
-	/// is not part of a contract. Available only after name and type resolution step.
+	/// is not part of a contract. Filled by the Scoper.
 	ContractDefinition const* contract = nullptr;
 };
 

libsolidity/ast/ASTForward.h

@@ -38,6 +38,7 @@ namespace solidity::frontend
 {
 
 class ASTNode;
+class ScopeOpener;
 class SourceUnit;
 class PragmaDirective;
 class ImportDirective;

libsolidity/interface/CompilerStack.cpp

@@ -36,6 +36,7 @@
 #include <libsolidity/analysis/PostTypeChecker.h>
 #include <libsolidity/analysis/StaticAnalyzer.h>
 #include <libsolidity/analysis/SyntaxChecker.h>
+#include <libsolidity/analysis/Scoper.h>
 #include <libsolidity/analysis/TypeChecker.h>
 #include <libsolidity/analysis/ViewPureChecker.h>
 #include <libsolidity/analysis/ImmutableValidator.h>
@@ -297,6 +298,10 @@ bool CompilerStack::analyze()
 		BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Must call analyze only after parsing was performed."));
 	resolveImports();
 
+	for (Source const* source: m_sourceOrder)
+		if (source->ast)
+			Scoper::assignScopes(*source->ast);
+
 	bool noErrors = true;
 
 	try

test/libsolidity/Assembly.cpp

@@ -30,6 +30,7 @@
 #include <libsolidity/parsing/Parser.h>
 #include <libsolidity/analysis/DeclarationTypeChecker.h>
 #include <libsolidity/analysis/NameAndTypeResolver.h>
+#include <libsolidity/analysis/Scoper.h>
 #include <libsolidity/codegen/Compiler.h>
 #include <libsolidity/ast/AST.h>
 #include <libsolidity/analysis/TypeChecker.h>
@@ -59,6 +60,7 @@ evmasm::AssemblyItems compileContract(std::shared_ptr<CharStream> _sourceCode)
 	BOOST_REQUIRE_NO_THROW(sourceUnit = parser.parse(make_shared<Scanner>(_sourceCode)));
 	BOOST_CHECK(!!sourceUnit);
 
+	Scoper::assignScopes(*sourceUnit);
 	GlobalContext globalContext;
 	NameAndTypeResolver resolver(globalContext, solidity::test::CommonOptions::get().evmVersion(), errorReporter);
 	DeclarationTypeChecker declarationTypeChecker(errorReporter, solidity::test::CommonOptions::get().evmVersion());

test/libsolidity/SolidityExpressionCompiler.cpp

@@ -26,6 +26,7 @@
 #include <liblangutil/Scanner.h>
 #include <libsolidity/parsing/Parser.h>
 #include <libsolidity/analysis/NameAndTypeResolver.h>
+#include <libsolidity/analysis/Scoper.h>
 #include <libsolidity/analysis/DeclarationTypeChecker.h>
 #include <libsolidity/codegen/CompilerContext.h>
 #include <libsolidity/codegen/ExpressionCompiler.h>
@@ -117,6 +118,7 @@ bytes compileFirstExpression(
 	ErrorList errors;
 	ErrorReporter errorReporter(errors);
 	GlobalContext globalContext;
+	Scoper::assignScopes(*sourceUnit);
 	NameAndTypeResolver resolver(globalContext, solidity::test::CommonOptions::get().evmVersion(), errorReporter);
 	resolver.registerDeclarations(*sourceUnit);
 	BOOST_REQUIRE_MESSAGE(resolver.resolveNamesAndTypes(*sourceUnit), "Resolving names failed");