Move constructor argument override check to TypeChecker and reuse annotations in ContractCompiler.

4 files changed, 60 insertions, 65 deletions

diff --git a/libsolidity/analysis/StaticAnalyzer.cpp b/libsolidity/analysis/StaticAnalyzer.cpp
index 700c8a71..33b0e296 100644
--- a/libsolidity/analysis/StaticAnalyzer.cpp
+++ b/libsolidity/analysis/StaticAnalyzer.cpp
@@ -90,48 +90,6 @@ void StaticAnalyzer::endVisit(FunctionDefinition const&)
 	m_localVarUseCount.clear();
 }
 
-bool StaticAnalyzer::visit(ModifierInvocation const& _modifier)
-{
-	if (!m_constructor)
-		return true;
-
-	bool const v050 = m_currentContract->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050);
-
-	if (auto contract = dynamic_cast<ContractDefinition const*>(_modifier.name()->annotation().referencedDeclaration))
-		for (auto const& base: m_currentContract->annotation().linearizedBaseContracts)
-			for (auto const& specifier: base->baseContracts())
-			{
-				Declaration const* parent = specifier->name().annotation().referencedDeclaration;
-				if (contract == parent && !specifier->arguments().empty())
-				{
-					if (v050)
-					{
-						SecondarySourceLocation ssl;
-						ssl.append("Second constructor call is here:", specifier->location());
-
-						m_errorReporter.declarationError(
-							_modifier.location(),
-							ssl,
-							"Duplicated super constructor call."
-						);
-					}
-					else
-					{
-						SecondarySourceLocation ssl;
-						ssl.append("Overridden constructor call is here:", specifier->location());
-
-						m_errorReporter.warning(
-							_modifier.location(),
-							"Duplicated super constructor calls are deprecated.",
-							ssl
-						);
-					}
-				}
-			}
-
-	return true;
-}
-
 bool StaticAnalyzer::visit(Identifier const& _identifier)
 {
 	if (m_currentFunction)
diff --git a/libsolidity/analysis/StaticAnalyzer.h b/libsolidity/analysis/StaticAnalyzer.h
index e68325bc..0a806bbd 100644
--- a/libsolidity/analysis/StaticAnalyzer.h
+++ b/libsolidity/analysis/StaticAnalyzer.h
@@ -57,7 +57,6 @@ private:
 
 	virtual bool visit(FunctionDefinition const& _function) override;
 	virtual void endVisit(FunctionDefinition const& _function) override;
-	virtual bool visit(ModifierInvocation const& _modifier) override;
 
 	virtual bool visit(ExpressionStatement const& _statement) override;
 	virtual bool visit(VariableDeclaration const& _variable) override;
diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index a252742d..c6868a0a 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -101,7 +101,7 @@ bool TypeChecker::visit(ContractDefinition const& _contract)
 	checkContractDuplicateEvents(_contract);
 	checkContractIllegalOverrides(_contract);
 	checkContractAbstractFunctions(_contract);
-	checkContractAbstractConstructors(_contract);
+	checkContractBaseConstructorArguments(_contract);
 
 	FunctionDefinition const* function = _contract.constructor();
 	if (function)
@@ -291,42 +291,75 @@ void TypeChecker::checkContractAbstractFunctions(ContractDefinition const& _cont
 			}
 }
 
-void TypeChecker::checkContractAbstractConstructors(ContractDefinition const& _contract)
+void TypeChecker::checkContractBaseConstructorArguments(ContractDefinition const& _contract)
 {
-	set<ContractDefinition const*> argumentsNeeded;
-	// check that we get arguments for all base constructors that need it.
-	// If not mark the contract as abstract (not fully implemented)
-
 	vector<ContractDefinition const*> const& bases = _contract.annotation().linearizedBaseContracts;
-	for (ContractDefinition const* contract: bases)
-		if (FunctionDefinition const* constructor = contract->constructor())
-			if (contract != &_contract && !constructor->parameters().empty())
-				argumentsNeeded.insert(contract);
 
+	// Determine the arguments that are used for the base constructors.
 	for (ContractDefinition const* contract: bases)
 	{
 		if (FunctionDefinition const* constructor = contract->constructor())
 			for (auto const& modifier: constructor->modifiers())
 			{
-				auto baseContract = dynamic_cast<ContractDefinition const*>(
-					&dereference(*modifier->name())
-				);
-				if (baseContract)
-					argumentsNeeded.erase(baseContract);
+				auto baseContract = dynamic_cast<ContractDefinition const*>(&dereference(*modifier->name()));
+				if (baseContract && baseContract->constructor() && !modifier->arguments().empty())
+					annotateBaseConstructorArguments(_contract, baseContract->constructor(), modifier.get());
 			}
 
-
 		for (ASTPointer<InheritanceSpecifier> const& base: contract->baseContracts())
 		{
 			auto baseContract = dynamic_cast<ContractDefinition const*>(&dereference(base->name()));
 			solAssert(baseContract, "");
-			if (base->arguments() && !base->arguments()->empty())
-				argumentsNeeded.erase(baseContract);
+
+			if (baseContract->constructor() && base->arguments() && !base->arguments()->empty())
+				annotateBaseConstructorArguments(_contract, baseContract->constructor(), base.get());
 		}
 	}
-	if (!argumentsNeeded.empty())
-		for (ContractDefinition const* contract: argumentsNeeded)
-			_contract.annotation().unimplementedFunctions.push_back(contract->constructor());
+
+	// check that we get arguments for all base constructors that need it.
+	// If not mark the contract as abstract (not fully implemented)
+	for (ContractDefinition const* contract: bases)
+		if (FunctionDefinition const* constructor = contract->constructor())
+			if (contract != &_contract && !constructor->parameters().empty())
+				if (!_contract.annotation().baseConstructorArguments.count(constructor))
+					_contract.annotation().unimplementedFunctions.push_back(constructor);
+}
+
+void TypeChecker::annotateBaseConstructorArguments(
+	ContractDefinition const& _currentContract,
+	FunctionDefinition const* _baseConstructor,
+	ASTNode const* _argumentNode
+)
+{
+	bool const v050 = _currentContract.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050);
+
+	solAssert(_baseConstructor, "");
+	solAssert(_argumentNode, "");
+
+	auto insertionResult = _currentContract.annotation().baseConstructorArguments.insert(
+		std::make_pair(_baseConstructor, _argumentNode)
+	);
+	if (!insertionResult.second)
+	{
+		ASTNode const* previousNode = insertionResult.first->second;
+
+		SecondarySourceLocation ssl;
+		ssl.append("Second constructor call is here:", _argumentNode->location());
+
+		if (v050)
+			m_errorReporter.declarationError(
+				previousNode->location(),
+				ssl,
+				"Base constructor arguments given twice."
+			);
+		else
+			m_errorReporter.warning(
+				previousNode->location(),
+				"Base constructor arguments given twice.",
+				ssl
+			);
+	}
+
 }
 
 void TypeChecker::checkContractIllegalOverrides(ContractDefinition const& _contract)
diff --git a/libsolidity/analysis/TypeChecker.h b/libsolidity/analysis/TypeChecker.h
index 2ba31232..2245abd6 100644
--- a/libsolidity/analysis/TypeChecker.h
+++ b/libsolidity/analysis/TypeChecker.h
@@ -73,7 +73,12 @@ private:
 	void checkFunctionOverride(FunctionDefinition const& function, FunctionDefinition const& super);
 	void overrideError(FunctionDefinition const& function, FunctionDefinition const& super, std::string message);
 	void checkContractAbstractFunctions(ContractDefinition const& _contract);
-	void checkContractAbstractConstructors(ContractDefinition const& _contract);
+	void checkContractBaseConstructorArguments(ContractDefinition const& _contract);
+	void annotateBaseConstructorArguments(
+		ContractDefinition const& _currentContract,
+		FunctionDefinition const* _baseConstructor,
+		ASTNode const* _argumentNode
+	);
 	/// Checks that different functions with external visibility end up having different
 	/// external argument types (i.e. different signature).
 	void checkContractExternalTypeClashes(ContractDefinition const& _contract);