Implement and fix overriding unimplemented and implemented functions with unimplemented functions.

libsolidity/analysis/ContractLevelChecker.cpp

@@ -262,51 +262,33 @@ void ContractLevelChecker::checkIllegalOverrides(ContractDefinition const& _cont
 	}
 }
 
-bool ContractLevelChecker::checkFunctionOverride(FunctionDefinition const& _function, FunctionDefinition const& _super)
+void ContractLevelChecker::checkFunctionOverride(FunctionDefinition const& _function, FunctionDefinition const& _super)
 {
 	FunctionTypePointer functionType = FunctionType(_function).asCallableFunction(false);
 	FunctionTypePointer superType = FunctionType(_super).asCallableFunction(false);
 
-	bool success = true;
+	solAssert(functionType->hasEqualParameterTypes(*superType), "");
-
-	if (!functionType->hasEqualParameterTypes(*superType))
-		return true;
 
 	if (!_function.overrides())
-	{
 		overrideError(_function, _super, "Overriding function is missing 'override' specifier.");
-		success = false;
-	}
 
 	if (!_super.virtualSemantics())
-	{
 		overrideError( _super, _function, "Trying to override non-virtual function. Did you forget to add \"virtual\"?", "Overriding function is here:");
-		success = false;
-	}
 
 	if (!functionType->hasEqualReturnTypes(*superType))
-	{
 		overrideError(_function, _super, "Overriding function return types differ.");
-		success = false;
-	}
 
 	_function.annotation().baseFunctions.emplace(&_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.");
-			success = false;
-		}
-	}
 	if (_function.stateMutability() != _super.stateMutability())
-	{
 		overrideError(
 			_function,
 			_super,
@@ -316,10 +298,13 @@ bool ContractLevelChecker::checkFunctionOverride(FunctionDefinition const& _func
 			stateMutabilityToString(_function.stateMutability()) +
 			"\"."
 		);
-		success = false;
-	}
 
-	return success;
+	if (!_function.isImplemented() && _super.isImplemented())
+		overrideError(
+			_function,
+			_super,
+			"Overriding an implemented function with an unimplemented function is not allowed."
+		);
 }
 
 void ContractLevelChecker::overrideListError(FunctionDefinition const& function, set<ContractDefinition const*, LessFunction> _secondary, string const& _message1, string const& _message2)
@@ -372,11 +357,6 @@ void ContractLevelChecker::checkAbstractFunctions(ContractDefinition const& _con
 		});
 		if (it == overloads.end())
 			overloads.emplace_back(_type, _implemented);
-		else if (it->second)
-		{
-			if (!_implemented)
-				m_errorReporter.typeError(_declaration.location(), "Redeclaring an already implemented function as abstract");
-		}
 		else if (_implemented)
 			it->second = true;
 	};

libsolidity/analysis/ContractLevelChecker.h

@@ -70,10 +70,10 @@ private:
 	template <class T>
 	void findDuplicateDefinitions(std::map<std::string, std::vector<T>> const& _definitions, std::string _message);
 	void checkIllegalOverrides(ContractDefinition const& _contract);
-	/// Returns false and reports a type error with an appropriate
+	/// Performs various checks related to @a _function overriding @a _super like
-	/// message if overridden function signature differs.
+	/// different return type, invalid visibility change, etc.
-	/// Also stores the direct super function in the AST annotations.
+	/// Also stores @a _super as a base function of @a _function in its AST annotations.
-	bool checkFunctionOverride(FunctionDefinition const& _function, FunctionDefinition const& _super);
+	void checkFunctionOverride(FunctionDefinition const& _function, FunctionDefinition const& _super);
 	void overrideListError(FunctionDefinition const& function, std::set<ContractDefinition const*, LessFunction> _secondary, std::string const& _message1, std::string const& _message2);
 	void overrideError(CallableDeclaration const& function, CallableDeclaration const& super, std::string message, std::string secondaryMsg = "Overridden function is here:");
 	void checkAbstractFunctions(ContractDefinition const& _contract);

test/libsolidity/syntaxTests/inheritance/override/override_unimplemented_fail.sol

@@ -0,0 +1,15 @@
+abstract contract A {
+    function f() external virtual;
+}
+abstract contract B {
+    function f() external virtual {}
+}
+abstract contract C is A, B {
+    function f() external virtual override(A, B);
+}
+abstract contract D is B, A {
+    function f() external virtual override(A, B);
+}
+// ----
+// TypeError: (154-199): Overriding an implemented function with an unimplemented function is not allowed.
+// TypeError: (236-281): Overriding an implemented function with an unimplemented function is not allowed.

test/libsolidity/syntaxTests/inheritance/override/override_unimplemented_fine.sol

@@ -0,0 +1,9 @@
+interface A {
+    function f() external;
+}
+interface B {
+    function f() external;
+}
+abstract contract C is A, B {
+    function f() external virtual override(A, B);
+}

test/libsolidity/syntaxTests/nameAndTypeResolution/030_redeclare_implemented_abstract_function_as_abstract.sol

@@ -2,4 +2,4 @@ abstract contract base { function foo() public virtual; }
 contract derived is base { function foo() public virtual override {} }
 contract wrong is derived { function foo() public virtual override; }
 // ----
-// TypeError: (157-196): Redeclaring an already implemented function as abstract
+// TypeError: (157-196): Overriding an implemented function with an unimplemented function is not allowed.