diff --git a/libsolidity/analysis/ContractLevelChecker.cpp b/libsolidity/analysis/ContractLevelChecker.cpp index 1e7dc8aec..23a265bc9 100644 --- a/libsolidity/analysis/ContractLevelChecker.cpp +++ b/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; - - if (!functionType->hasEqualParameterTypes(*superType)) - return true; + solAssert(functionType->hasEqualParameterTypes(*superType), ""); 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 _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; }; diff --git a/libsolidity/analysis/ContractLevelChecker.h b/libsolidity/analysis/ContractLevelChecker.h index d2d1006fb..0ab2567e7 100644 --- a/libsolidity/analysis/ContractLevelChecker.h +++ b/libsolidity/analysis/ContractLevelChecker.h @@ -70,10 +70,10 @@ private: template void findDuplicateDefinitions(std::map> const& _definitions, std::string _message); void checkIllegalOverrides(ContractDefinition const& _contract); - /// Returns false and reports a type error with an appropriate - /// message if overridden function signature differs. - /// Also stores the direct super function in the AST annotations. - bool checkFunctionOverride(FunctionDefinition const& _function, FunctionDefinition const& _super); + /// Performs various checks related to @a _function overriding @a _super like + /// different return type, invalid visibility change, etc. + /// Also stores @a _super as a base function of @a _function in its AST annotations. + void checkFunctionOverride(FunctionDefinition const& _function, FunctionDefinition const& _super); void overrideListError(FunctionDefinition const& function, std::set _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); diff --git a/test/libsolidity/syntaxTests/inheritance/override/override_unimplemented_fail.sol b/test/libsolidity/syntaxTests/inheritance/override/override_unimplemented_fail.sol new file mode 100644 index 000000000..833743f75 --- /dev/null +++ b/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. diff --git a/test/libsolidity/syntaxTests/inheritance/override/override_unimplemented_fine.sol b/test/libsolidity/syntaxTests/inheritance/override/override_unimplemented_fine.sol new file mode 100644 index 000000000..50d293716 --- /dev/null +++ b/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); +} diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/030_redeclare_implemented_abstract_function_as_abstract.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/030_redeclare_implemented_abstract_function_as_abstract.sol index a29c2f24c..acad919c5 100644 --- a/test/libsolidity/syntaxTests/nameAndTypeResolution/030_redeclare_implemented_abstract_function_as_abstract.sol +++ 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.