mirror of
https://github.com/ethereum/solidity
synced 2023-10-03 13:03:40 +00:00
Implement overridding of functions by public variables
This commit is contained in:
committed by
Mathias Baumann
parent
52467108ac
commit
b7d5de59c5
@@ -69,8 +69,8 @@ vector<ASTPointer<UserDefinedTypeName>> sortByContract(vector<ASTPointer<UserDef
|
||||
return sorted;
|
||||
}
|
||||
|
||||
template <class T>
|
||||
bool hasEqualNameAndParameters(T const& _a, T const& _b)
|
||||
template <class T, class B>
|
||||
bool hasEqualNameAndParameters(T const& _a, B const& _b)
|
||||
{
|
||||
return
|
||||
_a.name() == _b.name() &&
|
||||
@@ -243,6 +243,36 @@ void ContractLevelChecker::checkIllegalOverrides(ContractDefinition const& _cont
|
||||
|
||||
checkModifierOverrides(funcSet, modSet, _contract.functionModifiers());
|
||||
|
||||
for (auto const* stateVar: _contract.stateVariables())
|
||||
{
|
||||
if (!stateVar->isPublic())
|
||||
continue;
|
||||
|
||||
bool found = false;
|
||||
for (
|
||||
auto it = find_if(funcSet.begin(), funcSet.end(), MatchByName{stateVar->name()});
|
||||
it != funcSet.end();
|
||||
it = find_if(++it, funcSet.end(), MatchByName{stateVar->name()})
|
||||
)
|
||||
{
|
||||
if (!hasEqualNameAndParameters(*stateVar, **it))
|
||||
continue;
|
||||
|
||||
if ((*it)->visibility() != Declaration::Visibility::External)
|
||||
overrideError(*stateVar, **it, "Public state variables can only override functions with external visibility.");
|
||||
else
|
||||
checkFunctionOverride(*stateVar, **it);
|
||||
|
||||
found = true;
|
||||
}
|
||||
|
||||
if (!found && stateVar->overrides())
|
||||
m_errorReporter.typeError(
|
||||
stateVar->overrides()->location(),
|
||||
"Public state variable has override specified but does not override anything."
|
||||
);
|
||||
}
|
||||
|
||||
for (FunctionDefinition const* function: _contract.definedFunctions())
|
||||
{
|
||||
if (function->isConstructor())
|
||||
@@ -262,49 +292,64 @@ void ContractLevelChecker::checkIllegalOverrides(ContractDefinition const& _cont
|
||||
}
|
||||
}
|
||||
|
||||
void ContractLevelChecker::checkFunctionOverride(FunctionDefinition const& _function, FunctionDefinition const& _super)
|
||||
template<class T>
|
||||
void ContractLevelChecker::checkFunctionOverride(T const& _overriding, FunctionDefinition const& _super)
|
||||
{
|
||||
FunctionTypePointer functionType = FunctionType(_function).asCallableFunction(false);
|
||||
string overridingName;
|
||||
|
||||
if constexpr(std::is_same<FunctionDefinition, T>::value)
|
||||
overridingName = "function";
|
||||
else
|
||||
overridingName = "public state variable";
|
||||
|
||||
FunctionTypePointer functionType = FunctionType(_overriding).asCallableFunction(false);
|
||||
FunctionTypePointer superType = FunctionType(_super).asCallableFunction(false);
|
||||
|
||||
solAssert(functionType->hasEqualParameterTypes(*superType), "");
|
||||
solAssert(functionType->hasEqualParameterTypes(*superType), "Override doesn't have equal parameters!");
|
||||
|
||||
if (!_function.overrides())
|
||||
overrideError(_function, _super, "Overriding function is missing 'override' specifier.");
|
||||
if (!_overriding.overrides())
|
||||
overrideError(_overriding, _super, "Overriding " + overridingName + " is missing 'override' specifier.");
|
||||
|
||||
if (!_super.virtualSemantics())
|
||||
overrideError( _super, _function, "Trying to override non-virtual function. Did you forget to add \"virtual\"?", "Overriding function is here:");
|
||||
overrideError( _super, _overriding, "Trying to override non-virtual function. Did you forget to add \"virtual\"?", "Overriding " + overridingName + " is here:");
|
||||
|
||||
if (!functionType->hasEqualReturnTypes(*superType))
|
||||
overrideError(_function, _super, "Overriding function return types differ.");
|
||||
overrideError(_overriding, _super, "Overriding " + overridingName + " return types differ.");
|
||||
|
||||
_function.annotation().baseFunctions.emplace(&_super);
|
||||
if constexpr(std::is_same<T, FunctionDefinition>::value)
|
||||
_overriding.annotation().baseFunctions.emplace(&_super);
|
||||
|
||||
if (_function.visibility() != _super.visibility())
|
||||
if (_overriding.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
|
||||
_overriding.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()) +
|
||||
"\"."
|
||||
);
|
||||
overrideError(_overriding, _super, "Overriding " + overridingName + " visibility differs.");
|
||||
}
|
||||
|
||||
if (!_function.isImplemented() && _super.isImplemented())
|
||||
overrideError(
|
||||
_function,
|
||||
_super,
|
||||
"Overriding an implemented function with an unimplemented function is not allowed."
|
||||
);
|
||||
if constexpr(std::is_same<T, FunctionDefinition>::value)
|
||||
{
|
||||
if (_overriding.stateMutability() != _super.stateMutability())
|
||||
overrideError(
|
||||
_overriding,
|
||||
_super,
|
||||
"Overriding function changes state mutability from \"" +
|
||||
stateMutabilityToString(_super.stateMutability()) +
|
||||
"\" to \"" +
|
||||
stateMutabilityToString(_overriding.stateMutability()) +
|
||||
"\"."
|
||||
);
|
||||
|
||||
if (!_overriding.isImplemented() && _super.isImplemented())
|
||||
overrideError(
|
||||
_overriding,
|
||||
_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)
|
||||
@@ -332,12 +377,12 @@ void ContractLevelChecker::overrideListError(FunctionDefinition const& function,
|
||||
);
|
||||
}
|
||||
|
||||
void ContractLevelChecker::overrideError(CallableDeclaration const& function, CallableDeclaration const& super, string message, string secondaryMsg)
|
||||
void ContractLevelChecker::overrideError(Declaration const& _overriding, Declaration const& _super, string _message, string _secondaryMsg)
|
||||
{
|
||||
m_errorReporter.typeError(
|
||||
function.location(),
|
||||
SecondarySourceLocation().append(secondaryMsg, super.location()),
|
||||
message
|
||||
_overriding.location(),
|
||||
SecondarySourceLocation().append(_secondaryMsg, _super.location()),
|
||||
_message
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -73,9 +73,10 @@ private:
|
||||
/// 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);
|
||||
template<class T>
|
||||
void checkFunctionOverride(T const& _overriding, 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 overrideError(Declaration const& _overriding, Declaration const& _super, std::string _message, std::string _secondaryMsg = "Overridden function is here:");
|
||||
void checkAbstractFunctions(ContractDefinition const& _contract);
|
||||
/// Checks that the base constructor arguments are properly provided.
|
||||
/// Fills the list of unimplemented functions in _contract's annotations.
|
||||
|
||||
@@ -346,9 +346,21 @@ void NameAndTypeResolver::importInheritedScope(ContractDefinition const& _base)
|
||||
solAssert(conflictingDeclaration, "");
|
||||
|
||||
// Usual shadowing is not an error
|
||||
if (dynamic_cast<ModifierDefinition const*>(declaration) && dynamic_cast<ModifierDefinition const*>(conflictingDeclaration))
|
||||
if (
|
||||
dynamic_cast<ModifierDefinition const*>(declaration) &&
|
||||
dynamic_cast<ModifierDefinition const*>(conflictingDeclaration)
|
||||
)
|
||||
continue;
|
||||
|
||||
// Public state variable can override functions
|
||||
if (auto varDecl = dynamic_cast<VariableDeclaration const*>(conflictingDeclaration))
|
||||
if (
|
||||
dynamic_cast<FunctionDefinition const*>(declaration) &&
|
||||
varDecl->isStateVariable() &&
|
||||
varDecl->isPublic()
|
||||
)
|
||||
continue;
|
||||
|
||||
if (declaration->location().start < conflictingDeclaration->location().start)
|
||||
{
|
||||
firstDeclarationLocation = declaration->location();
|
||||
|
||||
Reference in New Issue
Block a user