diff --git a/libsolidity/analysis/ContractLevelChecker.cpp b/libsolidity/analysis/ContractLevelChecker.cpp index a2e76edfc..bf132d63b 100644 --- a/libsolidity/analysis/ContractLevelChecker.cpp +++ b/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> 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> functions; + map 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 + ); +} + diff --git a/libsolidity/analysis/ContractLevelChecker.h b/libsolidity/analysis/ContractLevelChecker.h index b1632b787..400979c05 100644 --- a/libsolidity/analysis/ContractLevelChecker.h +++ b/libsolidity/analysis/ContractLevelChecker.h @@ -58,6 +58,11 @@ private: void checkContractDuplicateEvents(ContractDefinition const& _contract); template void findDuplicateDefinitions(std::map> 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; }; diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 3ffa81018..07d32563d 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/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> functions; - map 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>> externalDeclarations; diff --git a/libsolidity/analysis/TypeChecker.h b/libsolidity/analysis/TypeChecker.h index 69fbbb3ed..7d718ff90 100644 --- a/libsolidity/analysis/TypeChecker.h +++ b/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(