Move override checks.

libsolidity/analysis/ContractLevelChecker.cpp

@@ -35,6 +35,7 @@ bool ContractLevelChecker::check(ContractDefinition const& _contract)
 {
 	checkContractDuplicateFunctions(_contract);
 	checkContractDuplicateEvents(_contract);
+	checkContractIllegalOverrides(_contract);
 
 	return Error::containsOnlyWarnings(m_errorReporter.errors());
 }
@@ -120,3 +121,85 @@ void ContractLevelChecker::findDuplicateDefinitions(map<string, vector<T>> const
 		}
 	}
 }
+
+void ContractLevelChecker::checkContractIllegalOverrides(ContractDefinition const& _contract)
+{
+	// TODO unify this at a later point. for this we need to put the constness and the access specifier
+	// into the types
+	map<string, vector<FunctionDefinition const*>> functions;
+	map<string, ModifierDefinition const*> modifiers;
+
+	// We search from derived to base, so the stored item causes the error.
+	for (ContractDefinition const* contract: _contract.annotation().linearizedBaseContracts)
+	{
+		for (FunctionDefinition const* function: contract->definedFunctions())
+		{
+			if (function->isConstructor())
+				continue; // constructors can neither be overridden nor override anything
+			string const& name = function->name();
+			if (modifiers.count(name))
+				m_errorReporter.typeError(modifiers[name]->location(), "Override changes function to modifier.");
+
+			for (FunctionDefinition const* overriding: functions[name])
+				checkFunctionOverride(*overriding, *function);
+
+			functions[name].push_back(function);
+		}
+		for (ModifierDefinition const* modifier: contract->functionModifiers())
+		{
+			string const& name = modifier->name();
+			ModifierDefinition const*& override = modifiers[name];
+			if (!override)
+				override = modifier;
+			else if (ModifierType(*override) != ModifierType(*modifier))
+				m_errorReporter.typeError(override->location(), "Override changes modifier signature.");
+			if (!functions[name].empty())
+				m_errorReporter.typeError(override->location(), "Override changes modifier to function.");
+		}
+	}
+}
+
+void ContractLevelChecker::checkFunctionOverride(FunctionDefinition const& _function, FunctionDefinition const& _super)
+{
+	FunctionTypePointer functionType = FunctionType(_function).asCallableFunction(false);
+	FunctionTypePointer superType = FunctionType(_super).asCallableFunction(false);
+
+	if (!functionType->hasEqualParameterTypes(*superType))
+		return;
+	if (!functionType->hasEqualReturnTypes(*superType))
+		overrideError(_function, _super, "Overriding function return types differ.");
+
+	if (!_function.annotation().superFunction)
+		_function.annotation().superFunction = &_super;
+
+	if (_function.visibility() != _super.visibility())
+	{
+		// Visibility change from external to public is fine.
+		// Any other change is disallowed.
+		if (!(
+			_super.visibility() == FunctionDefinition::Visibility::External &&
+			_function.visibility() == FunctionDefinition::Visibility::Public
+		))
+			overrideError(_function, _super, "Overriding function visibility differs.");
+	}
+	if (_function.stateMutability() != _super.stateMutability())
+		overrideError(
+			_function,
+			_super,
+			"Overriding function changes state mutability from \"" +
+			stateMutabilityToString(_super.stateMutability()) +
+			"\" to \"" +
+			stateMutabilityToString(_function.stateMutability()) +
+			"\"."
+		);
+}
+
+void ContractLevelChecker::overrideError(FunctionDefinition const& function, FunctionDefinition const& super, string message)
+{
+	m_errorReporter.typeError(
+		function.location(),
+		SecondarySourceLocation().append("Overridden function is here:", super.location()),
+		message
+	);
+}
+

libsolidity/analysis/ContractLevelChecker.h

@@ -58,6 +58,11 @@ private:
 	void checkContractDuplicateEvents(ContractDefinition const& _contract);
 	template <class T>
 	void findDuplicateDefinitions(std::map<std::string, std::vector<T>> const& _definitions, std::string _message);
+	void checkContractIllegalOverrides(ContractDefinition const& _contract);
+	/// Reports a type error with an appropriate message if overridden function signature differs.
+	/// Also stores the direct super function in the AST annotations.
+	void checkFunctionOverride(FunctionDefinition const& function, FunctionDefinition const& super);
+	void overrideError(FunctionDefinition const& function, FunctionDefinition const& super, std::string message);
 
 	langutil::ErrorReporter& m_errorReporter;
 };

libsolidity/analysis/TypeChecker.cpp

@@ -96,7 +96,6 @@ bool TypeChecker::visit(ContractDefinition const& _contract)
 	ASTNode::listAccept(_contract.definedStructs(), *this);
 	ASTNode::listAccept(_contract.baseContracts(), *this);
 
-	checkContractIllegalOverrides(_contract);
 	checkContractAbstractFunctions(_contract);
 	checkContractBaseConstructorArguments(_contract);
 
@@ -288,87 +287,6 @@ void TypeChecker::annotateBaseConstructorArguments(
 
 }
 
-void TypeChecker::checkContractIllegalOverrides(ContractDefinition const& _contract)
-{
-	// TODO unify this at a later point. for this we need to put the constness and the access specifier
-	// into the types
-	map<string, vector<FunctionDefinition const*>> functions;
-	map<string, ModifierDefinition const*> modifiers;
-
-	// We search from derived to base, so the stored item causes the error.
-	for (ContractDefinition const* contract: _contract.annotation().linearizedBaseContracts)
-	{
-		for (FunctionDefinition const* function: contract->definedFunctions())
-		{
-			if (function->isConstructor())
-				continue; // constructors can neither be overridden nor override anything
-			string const& name = function->name();
-			if (modifiers.count(name))
-				m_errorReporter.typeError(modifiers[name]->location(), "Override changes function to modifier.");
-
-			for (FunctionDefinition const* overriding: functions[name])
-				checkFunctionOverride(*overriding, *function);
-
-			functions[name].push_back(function);
-		}
-		for (ModifierDefinition const* modifier: contract->functionModifiers())
-		{
-			string const& name = modifier->name();
-			ModifierDefinition const*& override = modifiers[name];
-			if (!override)
-				override = modifier;
-			else if (ModifierType(*override) != ModifierType(*modifier))
-				m_errorReporter.typeError(override->location(), "Override changes modifier signature.");
-			if (!functions[name].empty())
-				m_errorReporter.typeError(override->location(), "Override changes modifier to function.");
-		}
-	}
-}
-
-void TypeChecker::checkFunctionOverride(FunctionDefinition const& _function, FunctionDefinition const& _super)
-{
-	FunctionTypePointer functionType = FunctionType(_function).asCallableFunction(false);
-	FunctionTypePointer superType = FunctionType(_super).asCallableFunction(false);
-
-	if (!functionType->hasEqualParameterTypes(*superType))
-		return;
-	if (!functionType->hasEqualReturnTypes(*superType))
-		overrideError(_function, _super, "Overriding function return types differ.");
-
-	if (!_function.annotation().superFunction)
-		_function.annotation().superFunction = &_super;
-
-	if (_function.visibility() != _super.visibility())
-	{
-		// Visibility change from external to public is fine.
-		// Any other change is disallowed.
-		if (!(
-			_super.visibility() == FunctionDefinition::Visibility::External &&
-			_function.visibility() == FunctionDefinition::Visibility::Public
-		))
-			overrideError(_function, _super, "Overriding function visibility differs.");
-	}
-	if (_function.stateMutability() != _super.stateMutability())
-		overrideError(
-			_function,
-			_super,
-			"Overriding function changes state mutability from \"" +
-			stateMutabilityToString(_super.stateMutability()) +
-			"\" to \"" +
-			stateMutabilityToString(_function.stateMutability()) +
-			"\"."
-		);
-}
-
-void TypeChecker::overrideError(FunctionDefinition const& function, FunctionDefinition const& super, string message)
-{
-	m_errorReporter.typeError(
-		function.location(),
-		SecondarySourceLocation().append("Overridden function is here:", super.location()),
-		message
-	);
-}
-
 void TypeChecker::checkContractExternalTypeClashes(ContractDefinition const& _contract)
 {
 	map<string, vector<pair<Declaration const*, FunctionTypePointer>>> externalDeclarations;

libsolidity/analysis/TypeChecker.h

@@ -66,11 +66,6 @@ public:
 private:
 
 	bool visit(ContractDefinition const& _contract) override;
-	void checkContractIllegalOverrides(ContractDefinition const& _contract);
-	/// Reports a type error with an appropriate message if overridden function signature differs.
-	/// Also stores the direct super function in the AST annotations.
-	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 checkContractBaseConstructorArguments(ContractDefinition const& _contract);
 	void annotateBaseConstructorArguments(