Merge pull request #1620 from ethereum/refactorEntry

Refactor NameAndTypeResolver and SyntaxChecker to allow other entry points.

libsolidity/analysis/NameAndTypeResolver.cpp

@@ -34,8 +34,10 @@ namespace solidity
 
 NameAndTypeResolver::NameAndTypeResolver(
 	vector<Declaration const*> const& _globals,
+	map<ASTNode const*, shared_ptr<DeclarationContainer>>& _scopes,
 	ErrorList& _errors
 ) :
+	m_scopes(_scopes),
 	m_errors(_errors)
 {
 	if (!m_scopes[nullptr])
@@ -44,18 +46,12 @@ NameAndTypeResolver::NameAndTypeResolver(
 		m_scopes[nullptr]->registerDeclaration(*declaration);
 }
 
-bool NameAndTypeResolver::registerDeclarations(SourceUnit& _sourceUnit)
+bool NameAndTypeResolver::registerDeclarations(ASTNode& _sourceUnit, ASTNode const* _currentScope)
 {
-	if (!m_scopes[&_sourceUnit])
-		// By importing, it is possible that the container already exists.
-		m_scopes[&_sourceUnit].reset(new DeclarationContainer(nullptr, m_scopes[nullptr].get()));
-	m_currentScope = m_scopes[&_sourceUnit].get();
-
 	// The helper registers all declarations in m_scopes as a side-effect of its construction.
 	try
 	{
-		DeclarationRegistrationHelper registrar(m_scopes, _sourceUnit, m_errors);
-		_sourceUnit.annotation().exportedSymbols = m_scopes[&_sourceUnit]->declarations();
+		DeclarationRegistrationHelper registrar(m_scopes, _sourceUnit, m_errors, _currentScope);
 	}
 	catch (FatalError const&)
 	{
@@ -132,27 +128,28 @@ bool NameAndTypeResolver::performImports(SourceUnit& _sourceUnit, map<string, So
 	return !error;
 }
 
-bool NameAndTypeResolver::resolveNamesAndTypes(ContractDefinition& _contract)
+bool NameAndTypeResolver::resolveNamesAndTypes(ASTNode& _node, bool _resolveInsideCode)
 {
+	bool success = true;
 	try
 	{
-		m_currentScope = m_scopes[_contract.scope()].get();
+		if (ContractDefinition* contract = dynamic_cast<ContractDefinition*>(&_node))
+		{
+			m_currentScope = m_scopes[contract->scope()].get();
 			solAssert(!!m_currentScope, "");
 
-		ReferencesResolver resolver(m_errors, *this, nullptr);
-		bool success = true;
-		for (ASTPointer<InheritanceSpecifier> const& baseContract: _contract.baseContracts())
-			if (!resolver.resolve(*baseContract))
+			for (ASTPointer<InheritanceSpecifier> const& baseContract: contract->baseContracts())
+				if (!resolveNamesAndTypes(*baseContract, true))
 					success = false;
 
-		m_currentScope = m_scopes[&_contract].get();
+			m_currentScope = m_scopes[contract].get();
 
 			if (success)
 			{
-			linearizeBaseContracts(_contract);
+				linearizeBaseContracts(*contract);
 				vector<ContractDefinition const*> properBases(
-				++_contract.annotation().linearizedBaseContracts.begin(),
-				_contract.annotation().linearizedBaseContracts.end()
+					++contract->annotation().linearizedBaseContracts.begin(),
+					contract->annotation().linearizedBaseContracts.end()
 				);
 
 				for (ContractDefinition const* base: properBases)
@@ -160,40 +157,35 @@ bool NameAndTypeResolver::resolveNamesAndTypes(ContractDefinition& _contract)
 			}
 
 			// these can contain code, only resolve parameters for now
-		for (ASTPointer<ASTNode> const& node: _contract.subNodes())
+			for (ASTPointer<ASTNode> const& node: contract->subNodes())
 			{
-			m_currentScope = m_scopes[m_scopes.count(node.get()) ? node.get() : &_contract].get();
-			if (!resolver.resolve(*node))
+				m_currentScope = m_scopes[contract].get();
+				if (!resolveNamesAndTypes(*node, false))
 					success = false;
 			}
 
 			if (!success)
 				return false;
 
-		m_currentScope = m_scopes[&_contract].get();
+			if (!_resolveInsideCode)
+				return success;
+
+			m_currentScope = m_scopes[contract].get();
 
 			// now resolve references inside the code
-		for (ModifierDefinition const* modifier: _contract.functionModifiers())
+			for (ASTPointer<ASTNode> const& node: contract->subNodes())
 			{
-			m_currentScope = m_scopes[modifier].get();
-			ReferencesResolver resolver(m_errors, *this, nullptr, true);
-			if (!resolver.resolve(*modifier))
+				m_currentScope = m_scopes[contract].get();
+				if (!resolveNamesAndTypes(*node, true))
 					success = false;
 			}
-
-		for (FunctionDefinition const* function: _contract.definedFunctions())
-		{
-			m_currentScope = m_scopes[function].get();
-			if (!ReferencesResolver(
-				m_errors,
-				*this,
-				function->returnParameterList().get(),
-				true
-			).resolve(*function))
-				success = false;
 		}
-		if (!success)
-			return false;
+		else
+		{
+			if (m_scopes.count(&_node))
+				m_currentScope = m_scopes[&_node].get();
+			return ReferencesResolver(m_errors, *this, _resolveInsideCode).resolve(_node);
+		}
 	}
 	catch (FatalError const&)
 	{
@@ -201,7 +193,7 @@ bool NameAndTypeResolver::resolveNamesAndTypes(ContractDefinition& _contract)
 			throw; // Something is weird here, rather throw again.
 		return false;
 	}
-	return true;
+	return success;
 }
 
 bool NameAndTypeResolver::updateDeclaration(Declaration const& _declaration)
@@ -459,14 +451,30 @@ void NameAndTypeResolver::reportFatalTypeError(Error const& _e)
 DeclarationRegistrationHelper::DeclarationRegistrationHelper(
 	map<ASTNode const*, shared_ptr<DeclarationContainer>>& _scopes,
 	ASTNode& _astRoot,
-	ErrorList& _errors
+	ErrorList& _errors,
+	ASTNode const* _currentScope
 ):
 	m_scopes(_scopes),
-	m_currentScope(&_astRoot),
+	m_currentScope(_currentScope),
 	m_errors(_errors)
 {
-	solAssert(!!m_scopes.at(m_currentScope), "");
 	_astRoot.accept(*this);
+	solAssert(m_currentScope == _currentScope, "Scopes not correctly closed.");
+}
+
+bool DeclarationRegistrationHelper::visit(SourceUnit& _sourceUnit)
+{
+	if (!m_scopes[&_sourceUnit])
+		// By importing, it is possible that the container already exists.
+		m_scopes[&_sourceUnit].reset(new DeclarationContainer(m_currentScope, m_scopes[m_currentScope].get()));
+	m_currentScope = &_sourceUnit;
+	return true;
+}
+
+void DeclarationRegistrationHelper::endVisit(SourceUnit& _sourceUnit)
+{
+	_sourceUnit.annotation().exportedSymbols = m_scopes[&_sourceUnit]->declarations();
+	closeCurrentScope();
 }
 
 bool DeclarationRegistrationHelper::visit(ImportDirective& _import)
@@ -587,12 +595,13 @@ void DeclarationRegistrationHelper::enterNewSubScope(Declaration const& _declara
 
 void DeclarationRegistrationHelper::closeCurrentScope()
 {
-	solAssert(m_currentScope, "Closed non-existing scope.");
+	solAssert(m_currentScope && m_scopes.count(m_currentScope), "Closed non-existing scope.");
 	m_currentScope = m_scopes[m_currentScope]->enclosingNode();
 }
 
 void DeclarationRegistrationHelper::registerDeclaration(Declaration& _declaration, bool _opensScope)
 {
+	solAssert(m_currentScope && m_scopes.count(m_currentScope), "No current scope.");
 	if (!m_scopes[m_currentScope]->registerDeclaration(_declaration, nullptr, !_declaration.isVisibleInContract()))
 	{
 		SourceLocation firstDeclarationLocation;

libsolidity/analysis/NameAndTypeResolver.h

@@ -42,15 +42,27 @@ namespace solidity
 class NameAndTypeResolver: private boost::noncopyable
 {
 public:
-	NameAndTypeResolver(std::vector<Declaration const*> const& _globals, ErrorList& _errors);
-	/// Registers all declarations found in the source unit.
+	/// Creates the resolver with the given declarations added to the global scope.
+	/// @param _scopes mapping of scopes to be used (usually default constructed), these
+	/// are filled during the lifetime of this object.
+	NameAndTypeResolver(
+		std::vector<Declaration const*> const& _globals,
+		std::map<ASTNode const*, std::shared_ptr<DeclarationContainer>>& _scopes,
+		ErrorList& _errors
+	);
+	/// Registers all declarations found in the AST node, usually a source unit.
 	/// @returns false in case of error.
-	bool registerDeclarations(SourceUnit& _sourceUnit);
+	/// @param _currentScope should be nullptr but can be used to inject new declarations into
+	/// existing scopes, used by the snippets feature.
+	bool registerDeclarations(ASTNode& _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 contract.
+	/// 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.
 	/// @returns false in case of error.
-	bool resolveNamesAndTypes(ContractDefinition& _contract);
+	bool resolveNamesAndTypes(ASTNode& _node, bool _resolveInsideCode = true);
 	/// 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.
@@ -77,8 +89,6 @@ public:
 	);
 
 private:
-	void reset();
-
 	/// Imports all members declared directly in the given contract (i.e. does not import inherited members)
 	/// into the current scope if they are not present already.
 	void importInheritedScope(ContractDefinition const& _base);
@@ -112,7 +122,7 @@ private:
 	/// where nullptr denotes the global scope. Note that structs are not scope since they do
 	/// not contain code.
 	/// Aliases (for example `import "x" as y;`) create multiple pointers to the same scope.
-	std::map<ASTNode const*, std::shared_ptr<DeclarationContainer>> m_scopes;
+	std::map<ASTNode const*, std::shared_ptr<DeclarationContainer>>& m_scopes;
 
 	DeclarationContainer* m_currentScope = nullptr;
 	ErrorList& m_errors;
@@ -125,13 +135,20 @@ private:
 class DeclarationRegistrationHelper: private ASTVisitor
 {
 public:
+	/// Registers declarations in their scopes and creates new scopes as a side-effect
+	/// of construction.
+	/// @param _currentScope should be nullptr if we start at SourceUnit, but can be different
+	/// to inject new declarations into an existing scope, used by snippets.
 	DeclarationRegistrationHelper(
 		std::map<ASTNode const*, std::shared_ptr<DeclarationContainer>>& _scopes,
 		ASTNode& _astRoot,
-		ErrorList& _errors
+		ErrorList& _errors,
+		ASTNode const* _currentScope = nullptr
 	);
 
 private:
+	bool visit(SourceUnit& _sourceUnit) override;
+	void endVisit(SourceUnit& _sourceUnit) override;
 	bool visit(ImportDirective& _declaration) override;
 	bool visit(ContractDefinition& _contract) override;
 	void endVisit(ContractDefinition& _contract) override;

libsolidity/analysis/ReferencesResolver.cpp

@@ -65,6 +65,30 @@ bool ReferencesResolver::visit(ElementaryTypeName const& _typeName)
 	return true;
 }
 
+bool ReferencesResolver::visit(FunctionDefinition const& _functionDefinition)
+{
+	m_returnParameters.push_back(_functionDefinition.returnParameterList().get());
+	return true;
+}
+
+void ReferencesResolver::endVisit(FunctionDefinition const&)
+{
+	solAssert(!m_returnParameters.empty(), "");
+	m_returnParameters.pop_back();
+}
+
+bool ReferencesResolver::visit(ModifierDefinition const&)
+{
+	m_returnParameters.push_back(nullptr);
+	return true;
+}
+
+void ReferencesResolver::endVisit(ModifierDefinition const&)
+{
+	solAssert(!m_returnParameters.empty(), "");
+	m_returnParameters.pop_back();
+}
+
 void ReferencesResolver::endVisit(UserDefinedTypeName const& _typeName)
 {
 	Declaration const* declaration = m_resolver.pathFromCurrentScope(_typeName.namePath());
@@ -161,7 +185,8 @@ bool ReferencesResolver::visit(InlineAssembly const& _inlineAssembly)
 
 bool ReferencesResolver::visit(Return const& _return)
 {
-	_return.annotation().functionReturnParameters = m_returnParameters;
+	solAssert(!m_returnParameters.empty(), "");
+	_return.annotation().functionReturnParameters = m_returnParameters.back();
 	return true;
 }
 

libsolidity/analysis/ReferencesResolver.h

@@ -45,12 +45,10 @@ public:
 	ReferencesResolver(
 		ErrorList& _errors,
 		NameAndTypeResolver& _resolver,
-		ParameterList const* _returnParameters,
 		bool _resolveInsideCode = false
 	):
 		m_errors(_errors),
 		m_resolver(_resolver),
-		m_returnParameters(_returnParameters),
 		m_resolveInsideCode(_resolveInsideCode)
 	{}
 
@@ -61,6 +59,10 @@ private:
 	virtual bool visit(Block const&) override { return m_resolveInsideCode; }
 	virtual bool visit(Identifier const& _identifier) override;
 	virtual bool visit(ElementaryTypeName const& _typeName) override;
+	virtual bool visit(FunctionDefinition const& _functionDefinition) override;
+	virtual void endVisit(FunctionDefinition const& _functionDefinition) override;
+	virtual bool visit(ModifierDefinition const& _modifierDefinition) override;
+	virtual void endVisit(ModifierDefinition const& _modifierDefinition) override;
 	virtual void endVisit(UserDefinedTypeName const& _typeName) override;
 	virtual void endVisit(FunctionTypeName const& _typeName) override;
 	virtual void endVisit(Mapping const& _typeName) override;
@@ -83,7 +85,8 @@ private:
 
 	ErrorList& m_errors;
 	NameAndTypeResolver& m_resolver;
-	ParameterList const* m_returnParameters;
+	/// Stack of return parameters.
+	std::vector<ParameterList const*> m_returnParameters;
 	bool const m_resolveInsideCode;
 	bool m_errorOccurred = false;
 };

libsolidity/analysis/SyntaxChecker.cpp

@@ -26,9 +26,9 @@ using namespace dev;
 using namespace dev::solidity;
 
 
-bool SyntaxChecker::checkSyntax(SourceUnit const& _sourceUnit)
+bool SyntaxChecker::checkSyntax(ASTNode const& _astRoot)
 {
-	_sourceUnit.accept(*this);
+	_astRoot.accept(*this);
 	return Error::containsOnlyWarnings(m_errors);
 }
 

libsolidity/analysis/SyntaxChecker.h

@@ -39,7 +39,7 @@ public:
 	/// @param _errors the reference to the list of errors and warnings to add them found during type checking.
 	SyntaxChecker(ErrorList& _errors): m_errors(_errors) {}
 
-	bool checkSyntax(SourceUnit const& _sourceUnit);
+	bool checkSyntax(ASTNode const& _astRoot);
 
 private:
 	/// Adds a new error to the list of errors.

libsolidity/analysis/TypeChecker.cpp

@@ -32,11 +32,11 @@ using namespace dev;
 using namespace dev::solidity;
 
 
-bool TypeChecker::checkTypeRequirements(ContractDefinition const& _contract)
+bool TypeChecker::checkTypeRequirements(ASTNode const& _contract)
 {
 	try
 	{
-		visit(_contract);
+		_contract.accept(*this);
 	}
 	catch (FatalError const&)
 	{
@@ -427,7 +427,9 @@ bool TypeChecker::visit(StructDefinition const& _struct)
 
 bool TypeChecker::visit(FunctionDefinition const& _function)
 {
-	bool isLibraryFunction = dynamic_cast<ContractDefinition const&>(*_function.scope()).isLibrary();
+	bool isLibraryFunction =
+		dynamic_cast<ContractDefinition const*>(_function.scope()) &&
+		dynamic_cast<ContractDefinition const*>(_function.scope())->isLibrary();
 	if (_function.isPayable())
 	{
 		if (isLibraryFunction)

libsolidity/analysis/TypeChecker.h

@@ -47,7 +47,7 @@ public:
 
 	/// Performs type checking on the given contract 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(ContractDefinition const& _contract);
+	bool checkTypeRequirements(ASTNode const& _contract);
 
 	/// @returns the type of an expression and asserts that it is present.
 	TypePointer const& type(Expression const& _expression) const;

libsolidity/interface/CompilerStack.cpp

@@ -88,6 +88,7 @@ void CompilerStack::reset(bool _keepSources)
 	m_optimize = false;
 	m_optimizeRuns = 200;
 	m_globalContext.reset();
+	m_scopes.clear();
 	m_sourceOrder.clear();
 	m_contracts.clear();
 	m_errors.clear();
@@ -165,7 +166,7 @@ bool CompilerStack::parse()
 			noErrors = false;
 
 	m_globalContext = make_shared<GlobalContext>();
-	NameAndTypeResolver resolver(m_globalContext->declarations(), m_errors);
+	NameAndTypeResolver resolver(m_globalContext->declarations(), m_scopes, m_errors);
 	for (Source const* source: m_sourceOrder)
 		if (!resolver.registerDeclarations(*source->ast))
 			return false;

libsolidity/interface/CompilerStack.h

@@ -52,6 +52,7 @@ namespace solidity
 
 // forward declarations
 class Scanner;
+class ASTNode;
 class ContractDefinition;
 class FunctionDefinition;
 class SourceUnit;
@@ -59,6 +60,7 @@ class Compiler;
 class GlobalContext;
 class InterfaceHandler;
 class Error;
+class DeclarationContainer;
 
 enum class DocumentationType: uint8_t
 {
@@ -271,6 +273,7 @@ private:
 	bool m_parseSuccessful;
 	std::map<std::string const, Source> m_sources;
 	std::shared_ptr<GlobalContext> m_globalContext;
+	std::map<ASTNode const*, std::shared_ptr<DeclarationContainer>> m_scopes;
 	std::vector<Source const*> m_sourceOrder;
 	std::map<std::string const, Contract> m_contracts;
 	std::string m_formalTranslation;

test/libsolidity/Assembly.cpp

@@ -53,7 +53,8 @@ eth::AssemblyItems compileContract(const string& _sourceCode)
 	BOOST_REQUIRE_NO_THROW(sourceUnit = parser.parse(make_shared<Scanner>(CharStream(_sourceCode))));
 	BOOST_CHECK(!!sourceUnit);
 
-	NameAndTypeResolver resolver({}, errors);
+	map<ASTNode const*, shared_ptr<DeclarationContainer>> scopes;
+	NameAndTypeResolver resolver({}, scopes, errors);
 	solAssert(Error::containsOnlyWarnings(errors), "");
 	resolver.registerDeclarations(*sourceUnit);
 	for (ASTPointer<ASTNode> const& node: sourceUnit->nodes())

test/libsolidity/SolidityExpressionCompiler.cpp

@@ -114,7 +114,8 @@ bytes compileFirstExpression(
 		declarations.push_back(variable.get());
 
 	ErrorList errors;
-	NameAndTypeResolver resolver(declarations, errors);
+	map<ASTNode const*, shared_ptr<DeclarationContainer>> scopes;
+	NameAndTypeResolver resolver(declarations, scopes, errors);
 	resolver.registerDeclarations(*sourceUnit);
 
 	vector<ContractDefinition const*> inheritanceHierarchy;

test/libsolidity/SolidityNameAndTypeResolution.cpp

@@ -66,7 +66,8 @@ parseAnalyseAndReturnError(string const& _source, bool _reportWarnings = false,
 			return make_pair(sourceUnit, errors.at(0));
 
 		std::shared_ptr<GlobalContext> globalContext = make_shared<GlobalContext>();
-		NameAndTypeResolver resolver(globalContext->declarations(), errors);
+		map<ASTNode const*, shared_ptr<DeclarationContainer>> scopes;
+		NameAndTypeResolver resolver(globalContext->declarations(), scopes, errors);
 		solAssert(Error::containsOnlyWarnings(errors), "");
 		resolver.registerDeclarations(*sourceUnit);
 
@@ -1080,7 +1081,7 @@ BOOST_AUTO_TEST_CASE(modifier_returns_value)
 			modifier mod(uint a) { _; return 7; }
 		}
 	)";
-	CHECK_ERROR(text, TypeError, "");
+	CHECK_ERROR(text, TypeError, "Return arguments not allowed.");
 }
 
 BOOST_AUTO_TEST_CASE(state_variable_accessors)