Merge pull request #246 from chriseth/refactor

Refactoring - more flexible contracts.

5 files changed, 37 insertions, 68 deletions

diff --git a/libsolidity/analysis/NameAndTypeResolver.cpp b/libsolidity/analysis/NameAndTypeResolver.cpp
index fa894456..612989e1 100644
--- a/libsolidity/analysis/NameAndTypeResolver.cpp
+++ b/libsolidity/analysis/NameAndTypeResolver.cpp
@@ -64,7 +64,7 @@ bool NameAndTypeResolver::resolveNamesAndTypes(ContractDefinition& _contract)
 	{
 		m_currentScope = &m_scopes[nullptr];
 
-		ReferencesResolver resolver(m_errors, *this, &_contract, nullptr);
+		ReferencesResolver resolver(m_errors, *this, nullptr);
 		bool success = true;
 		for (ASTPointer<InheritanceSpecifier> const& baseContract: _contract.baseContracts())
 			if (!resolver.resolve(*baseContract))
@@ -84,36 +84,11 @@ bool NameAndTypeResolver::resolveNamesAndTypes(ContractDefinition& _contract)
 				importInheritedScope(*base);
 		}
 
-		for (ASTPointer<StructDefinition> const& structDef: _contract.definedStructs())
-			if (!resolver.resolve(*structDef))
-				success = false;
-		for (ASTPointer<EnumDefinition> const& enumDef: _contract.definedEnums())
-			if (!resolver.resolve(*enumDef))
-				success = false;
-		for (ASTPointer<VariableDeclaration> const& variable: _contract.stateVariables())
-			if (!resolver.resolve(*variable))
-				success = false;
-		for (ASTPointer<EventDefinition> const& event: _contract.events())
-			if (!resolver.resolve(*event))
-				success = false;
 		// these can contain code, only resolve parameters for now
-		for (ASTPointer<ModifierDefinition> const& modifier: _contract.functionModifiers())
+		for (ASTPointer<ASTNode> const& node: _contract.subNodes())
 		{
-			m_currentScope = &m_scopes[modifier.get()];
-			ReferencesResolver resolver(m_errors, *this, &_contract, nullptr);
-			if (!resolver.resolve(*modifier))
-				success = false;
-		}
-
-		for (ASTPointer<FunctionDefinition> const& function: _contract.definedFunctions())
-		{
-			m_currentScope = &m_scopes[function.get()];
-			if (!ReferencesResolver(
-				m_errors,
-				*this,
-				&_contract,
-				function->returnParameterList().get()
-			).resolve(*function))
+			m_currentScope = &m_scopes[m_scopes.count(node.get()) ? node.get() : &_contract];
+			if (!resolver.resolve(*node))
 				success = false;
 		}
 
@@ -123,21 +98,20 @@ bool NameAndTypeResolver::resolveNamesAndTypes(ContractDefinition& _contract)
 		m_currentScope = &m_scopes[&_contract];
 
 		// now resolve references inside the code
-		for (ASTPointer<ModifierDefinition> const& modifier: _contract.functionModifiers())
+		for (ModifierDefinition const* modifier: _contract.functionModifiers())
 		{
-			m_currentScope = &m_scopes[modifier.get()];
-			ReferencesResolver resolver(m_errors, *this, &_contract, nullptr, true);
+			m_currentScope = &m_scopes[modifier];
+			ReferencesResolver resolver(m_errors, *this, nullptr, true);
 			if (!resolver.resolve(*modifier))
 				success = false;
 		}
 
-		for (ASTPointer<FunctionDefinition> const& function: _contract.definedFunctions())
+		for (FunctionDefinition const* function: _contract.definedFunctions())
 		{
-			m_currentScope = &m_scopes[function.get()];
+			m_currentScope = &m_scopes[function];
 			if (!ReferencesResolver(
 				m_errors,
 				*this,
-				&_contract,
 				function->returnParameterList().get(),
 				true
 			).resolve(*function))
diff --git a/libsolidity/analysis/ReferencesResolver.cpp b/libsolidity/analysis/ReferencesResolver.cpp
index f0afc4f9..e5b1c52b 100644
--- a/libsolidity/analysis/ReferencesResolver.cpp
+++ b/libsolidity/analysis/ReferencesResolver.cpp
@@ -50,11 +50,10 @@ bool ReferencesResolver::visit(UserDefinedTypeName const& _typeName)
 
 	_typeName.annotation().referencedDeclaration = declaration;
 
-	_typeName.annotation().contractScope = m_currentContract;
 	return true;
 }
 
-bool ReferencesResolver::resolve(ASTNode& _root)
+bool ReferencesResolver::resolve(ASTNode const& _root)
 {
 	try
 	{
@@ -73,10 +72,7 @@ bool ReferencesResolver::visit(Identifier const& _identifier)
 	if (declarations.empty())
 		fatalDeclarationError(_identifier.location(), "Undeclared identifier.");
 	else if (declarations.size() == 1)
-	{
 		_identifier.annotation().referencedDeclaration = declarations.front();
-		_identifier.annotation().contractScope = m_currentContract;
-	}
 	else
 		_identifier.annotation().overloadedDeclarations =
 			m_resolver.cleanedDeclarations(_identifier, declarations);
diff --git a/libsolidity/analysis/ReferencesResolver.h b/libsolidity/analysis/ReferencesResolver.h
index 62104611..6f5ced8c 100644
--- a/libsolidity/analysis/ReferencesResolver.h
+++ b/libsolidity/analysis/ReferencesResolver.h
@@ -45,19 +45,17 @@ public:
 	ReferencesResolver(
 		ErrorList& _errors,
 		NameAndTypeResolver& _resolver,
-		ContractDefinition const* _currentContract,
 		ParameterList const* _returnParameters,
 		bool _resolveInsideCode = false
 	):
 		m_errors(_errors),
 		m_resolver(_resolver),
-		m_currentContract(_currentContract),
 		m_returnParameters(_returnParameters),
 		m_resolveInsideCode(_resolveInsideCode)
 	{}
 
 	/// @returns true if no errors during resolving
-	bool resolve(ASTNode& _root);
+	bool resolve(ASTNode const& _root);
 
 private:
 	virtual bool visit(Block const&) override { return m_resolveInsideCode; }
@@ -83,7 +81,6 @@ private:
 
 	ErrorList& m_errors;
 	NameAndTypeResolver& m_resolver;
-	ContractDefinition const* m_currentContract;
 	ParameterList const* m_returnParameters;
 	bool const m_resolveInsideCode;
 	bool m_errorOccurred = false;
diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index 42fdad91..1d2d0258 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -60,7 +60,10 @@ TypePointer const& TypeChecker::type(VariableDeclaration const& _variable) const
 
 bool TypeChecker::visit(ContractDefinition const& _contract)
 {
+	m_scope = &_contract;
+
 	// We force our own visiting order here.
+	//@TODO structs will be visited again below, but it is probably fine.
 	ASTNode::listAccept(_contract.definedStructs(), *this);
 	ASTNode::listAccept(_contract.baseContracts(), *this);
 
@@ -74,7 +77,7 @@ bool TypeChecker::visit(ContractDefinition const& _contract)
 		typeError(function->returnParameterList()->location(), "Non-empty \"returns\" directive for constructor.");
 
 	FunctionDefinition const* fallbackFunction = nullptr;
-	for (ASTPointer<FunctionDefinition> const& function: _contract.definedFunctions())
+	for (FunctionDefinition const* function: _contract.definedFunctions())
 	{
 		if (function->name().empty())
 		{
@@ -86,7 +89,7 @@ bool TypeChecker::visit(ContractDefinition const& _contract)
 			}
 			else
 			{
-				fallbackFunction = function.get();
+				fallbackFunction = function;
 				if (!fallbackFunction->parameters().empty())
 					typeError(fallbackFunction->parameterList().location(), "Fallback function cannot take parameters.");
 			}
@@ -95,10 +98,7 @@ bool TypeChecker::visit(ContractDefinition const& _contract)
 			_contract.annotation().isFullyImplemented = false;
 	}
 
-	ASTNode::listAccept(_contract.stateVariables(), *this);
-	ASTNode::listAccept(_contract.events(), *this);
-	ASTNode::listAccept(_contract.functionModifiers(), *this);
-	ASTNode::listAccept(_contract.definedFunctions(), *this);
+	ASTNode::listAccept(_contract.subNodes(), *this);
 
 	checkContractExternalTypeClashes(_contract);
 	// check for hash collisions in function signatures
@@ -125,8 +125,8 @@ void TypeChecker::checkContractDuplicateFunctions(ContractDefinition const& _con
 	/// Checks that two functions with the same name defined in this contract have different
 	/// argument types and that there is at most one constructor.
 	map<string, vector<FunctionDefinition const*>> functions;
-	for (ASTPointer<FunctionDefinition> const& function: _contract.definedFunctions())
-		functions[function->name()].push_back(function.get());
+	for (FunctionDefinition const* function: _contract.definedFunctions())
+		functions[function->name()].push_back(function);
 
 	// Constructor
 	if (functions[_contract.name()].size() > 1)
@@ -170,7 +170,7 @@ void TypeChecker::checkContractAbstractFunctions(ContractDefinition const& _cont
 
 	// Search from base to derived
 	for (ContractDefinition const* contract: boost::adaptors::reverse(_contract.annotation().linearizedBaseContracts))
-		for (ASTPointer<FunctionDefinition> const& function: contract->definedFunctions())
+		for (FunctionDefinition const* function: contract->definedFunctions())
 		{
 			auto& overloads = functions[function->name()];
 			FunctionTypePointer funType = make_shared<FunctionType>(*function);
@@ -246,7 +246,7 @@ void TypeChecker::checkContractIllegalOverrides(ContractDefinition const& _contr
 	// We search from derived to base, so the stored item causes the error.
 	for (ContractDefinition const* contract: _contract.annotation().linearizedBaseContracts)
 	{
-		for (ASTPointer<FunctionDefinition> const& function: contract->definedFunctions())
+		for (FunctionDefinition const* function: contract->definedFunctions())
 		{
 			if (function->isConstructor())
 				continue; // constructors can neither be overridden nor override anything
@@ -267,14 +267,14 @@ void TypeChecker::checkContractIllegalOverrides(ContractDefinition const& _contr
 				)
 					typeError(overriding->location(), "Override changes extended function signature.");
 			}
-			functions[name].push_back(function.get());
+			functions[name].push_back(function);
 		}
-		for (ASTPointer<ModifierDefinition> const& modifier: contract->functionModifiers())
+		for (ModifierDefinition const* modifier: contract->functionModifiers())
 		{
 			string const& name = modifier->name();
 			ModifierDefinition const*& override = modifiers[name];
 			if (!override)
-				override = modifier.get();
+				override = modifier;
 			else if (ModifierType(*override) != ModifierType(*modifier))
 				typeError(override->location(), "Override changes modifier signature.");
 			if (!functions[name].empty())
@@ -288,20 +288,20 @@ void TypeChecker::checkContractExternalTypeClashes(ContractDefinition const& _co
 	map<string, vector<pair<Declaration const*, FunctionTypePointer>>> externalDeclarations;
 	for (ContractDefinition const* contract: _contract.annotation().linearizedBaseContracts)
 	{
-		for (ASTPointer<FunctionDefinition> const& f: contract->definedFunctions())
+		for (FunctionDefinition const* f: contract->definedFunctions())
 			if (f->isPartOfExternalInterface())
 			{
 				auto functionType = make_shared<FunctionType>(*f);
 				externalDeclarations[functionType->externalSignature()].push_back(
-					make_pair(f.get(), functionType)
+					make_pair(f, functionType)
 				);
 			}
-		for (ASTPointer<VariableDeclaration> const& v: contract->stateVariables())
+		for (VariableDeclaration const* v: contract->stateVariables())
 			if (v->isPartOfExternalInterface())
 			{
 				auto functionType = make_shared<FunctionType>(*v);
 				externalDeclarations[functionType->externalSignature()].push_back(
-					make_pair(v.get(), functionType)
+					make_pair(v, functionType)
 				);
 			}
 	}
@@ -1057,13 +1057,13 @@ void TypeChecker::endVisit(NewExpression const& _newExpression)
 		if (!contract->annotation().isFullyImplemented)
 			typeError(_newExpression.location(), "Trying to create an instance of an abstract contract.");
 
-		auto scopeContract = contractName->annotation().contractScope;
-		scopeContract->annotation().contractDependencies.insert(contract);
+		solAssert(!!m_scope, "");
+		m_scope->annotation().contractDependencies.insert(contract);
 		solAssert(
 			!contract->annotation().linearizedBaseContracts.empty(),
 			"Linearized base contracts not yet available."
 		);
-		if (contractDependenciesAreCyclic(*scopeContract))
+		if (contractDependenciesAreCyclic(*m_scope))
 			typeError(
 				_newExpression.location(),
 				"Circular reference for contract creation (cannot create instance of derived or same contract)."
@@ -1112,7 +1112,7 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess)
 
 	// Retrieve the types of the arguments if this is used to call a function.
 	auto const& argumentTypes = _memberAccess.annotation().argumentTypes;
-	MemberList::MemberMap possibleMembers = exprType->members().membersByName(memberName);
+	MemberList::MemberMap possibleMembers = exprType->members(m_scope).membersByName(memberName);
 	if (possibleMembers.size() > 1 && argumentTypes)
 	{
 		// do overload resolution
@@ -1131,7 +1131,7 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess)
 			DataLocation::Storage,
 			exprType
 		);
-		if (!storageType->members().membersByName(memberName).empty())
+		if (!storageType->members(m_scope).membersByName(memberName).empty())
 			fatalTypeError(
 				_memberAccess.location(),
 				"Member \"" + memberName + "\" is not available in " +
@@ -1258,7 +1258,7 @@ bool TypeChecker::visit(Identifier const& _identifier)
 
 			for (Declaration const* declaration: annotation.overloadedDeclarations)
 			{
-				TypePointer function = declaration->type(_identifier.annotation().contractScope);
+				TypePointer function = declaration->type();
 				solAssert(!!function, "Requested type not present.");
 				auto const* functionType = dynamic_cast<FunctionType const*>(function.get());
 				if (functionType && functionType->canTakeArguments(*annotation.argumentTypes))
@@ -1277,7 +1277,7 @@ bool TypeChecker::visit(Identifier const& _identifier)
 		"Referenced declaration is null after overload resolution."
 	);
 	annotation.isLValue = annotation.referencedDeclaration->isLValue();
-	annotation.type = annotation.referencedDeclaration->type(_identifier.annotation().contractScope);
+	annotation.type = annotation.referencedDeclaration->type();
 	if (!annotation.type)
 		fatalTypeError(_identifier.location(), "Declaration referenced before type could be determined.");
 	return false;
diff --git a/libsolidity/analysis/TypeChecker.h b/libsolidity/analysis/TypeChecker.h
index 2295bc22..9563d4a9 100644
--- a/libsolidity/analysis/TypeChecker.h
+++ b/libsolidity/analysis/TypeChecker.h
@@ -117,6 +117,8 @@ private:
 	/// Runs type checks on @a _expression to infer its type and then checks that it is an LValue.
 	void requireLValue(Expression const& _expression);
 
+	ContractDefinition const* m_scope = nullptr;
+
 	ErrorList& m_errors;
 };