Merge pull request #7839 from ethereum/state-var-override

Public State Variable Overriding
This commit is contained in:
chriseth 2019-12-05 14:48:25 +01:00 committed by GitHub
commit 1890d17744
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
31 changed files with 285 additions and 68 deletions

View File

@ -35,6 +35,7 @@ Language Features:
* Introduce ``virtual`` and ``override`` keywords.
* Modify ``push(element)`` for dynamic storage arrays such that it does not return the new length anymore.
* Yul: Introduce ``leave`` statement that exits the current function.
* Allow public variables to override external functions.
Compiler Features:
* Allow revert strings to be stripped from the binary using the ``--revert-strings`` option or the ``settings.debug.revertStrings`` setting.

View File

@ -259,6 +259,29 @@ overridden when used with multiple inheritance:
Functions without implementation have to be marked ``virtual``
outside of interfaces.
Public state variables can override external functions if the
parameter and return types of the function matches the getter function
of the variable:
::
pragma solidity >=0.5.0 <0.7.0;
contract A
{
function f() external virtual pure returns(uint) { return 5; }
}
contract B is A
{
uint public override f;
}
.. note::
While public state variables can override external functions, they themselves cannot
be overridden.
.. _modifier-overriding:
.. index:: ! overriding;modifier

View File

@ -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
);
}

View File

@ -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.

View File

@ -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();

View File

@ -0,0 +1,19 @@
contract A
{
function test() external virtual returns (uint256)
{
return 5;
}
}
contract X is A
{
uint256 public override test;
function set() public { test = 2; }
}
// ----
// test() -> 0
// set() ->
// test() -> 2

View File

@ -1,2 +1,2 @@
interface ERC20 { function x() external returns (uint); }
contract C is ERC20 { uint public x; }
contract C is ERC20 { uint public override x; }

View File

@ -1,6 +1,6 @@
interface X { function test() external returns (uint256); }
contract Y is X {
uint256 public test = 42;
uint256 public override test = 42;
}
contract T {
constructor() public { new Y(); }

View File

@ -1,9 +1,9 @@
abstract contract X { function test() internal virtual returns (uint256); }
contract Y is X {
uint256 public test = 42;
uint256 public override test = 42;
}
contract T {
constructor() public { new Y(); }
}
// ----
// DeclarationError: (98-122): Identifier already declared.
// TypeError: (98-131): Public state variables can only override functions with external visibility.

View File

@ -1,8 +1,10 @@
abstract contract X { function test() private returns (uint256) {} }
abstract contract X { function test() private virtual returns (uint256); }
contract Y is X {
uint256 public test = 42;
uint256 public override test = 42;
}
contract T {
constructor() public { new Y(); }
}
// ----
// TypeError: (97-130): Public state variables can only override functions with external visibility.
// TypeError: (22-72): "virtual" and "private" cannot be used together.

View File

@ -1,9 +1,8 @@
abstract contract X { function test() public virtual returns (uint256); }
abstract contract X { function test() external virtual returns (uint256); }
contract Y is X {
uint256 public test = 42;
uint256 public override test = 42;
}
contract T {
constructor() public { new Y(); }
}
// ----
// DeclarationError: (96-120): Identifier already declared.

View File

@ -1,12 +1,9 @@
abstract contract A {
int public testvar;
function test() internal virtual returns (uint256);
function test2() internal virtual returns (uint256);
}
contract X is A {
int public override testvar;
function test() internal override returns (uint256) {}
function test2() internal override(A) returns (uint256) {}
}
// ----
// DeclarationError: (171-198): Identifier already declared.

View File

@ -1,5 +1,4 @@
abstract contract A {
int public testvar;
function foo() internal virtual returns (uint256);
}
abstract contract B {
@ -7,9 +6,7 @@ abstract contract B {
function test() internal virtual returns (uint256);
}
abstract contract X is A, B {
int public override testvar;
function test() internal override returns (uint256) {}
}
// ----
// DeclarationError: (257-284): Identifier already declared.
// TypeError: (226-343): Derived contract must override function "foo". Function with the same name and parameter types defined in two or more base classes.
// TypeError: (205-292): Derived contract must override function "foo". Function with the same name and parameter types defined in two or more base classes.

View File

@ -13,7 +13,6 @@ abstract contract D {
function foo() internal virtual returns (uint256);
}
contract X is A, B, C, D {
int public override testvar;
function test() internal override returns (uint256) {}
function foo() internal override(A, B, C, D) returns (uint256) {}
}

View File

@ -1,5 +1,4 @@
abstract contract A {
int public testvar;
function foo() internal virtual returns (uint256);
function test(uint8 _a) internal virtual returns (uint256);
}
@ -15,13 +14,11 @@ abstract contract D {
function test() internal virtual returns (uint256);
}
abstract contract X is A, B, C, D {
int public override testvar;
function test() internal override(B, D, D) virtual returns (uint256);
function foo() internal override(A, C, B, B, B, D ,D) virtual returns (uint256);
}
// ----
// DeclarationError: (529-556): Identifier already declared.
// TypeError: (599-600): Duplicate contract "D" found in override list of "test".
// TypeError: (672-673): Duplicate contract "B" found in override list of "foo".
// TypeError: (675-676): Duplicate contract "B" found in override list of "foo".
// TypeError: (681-682): Duplicate contract "D" found in override list of "foo".
// TypeError: (548-549): Duplicate contract "D" found in override list of "test".
// TypeError: (621-622): Duplicate contract "B" found in override list of "foo".
// TypeError: (624-625): Duplicate contract "B" found in override list of "foo".
// TypeError: (630-631): Duplicate contract "D" found in override list of "foo".

View File

@ -14,10 +14,9 @@ abstract contract D {
function test() internal virtual returns (uint256);
}
abstract contract X is A, B, C, D {
int public override testvar;
function test() internal override(B, D, C) virtual returns (uint256);
function foo() internal override(A, C) virtual returns (uint256);
}
// ----
// TypeError: (563-580): Invalid contract specified in override list: C.
// TypeError: (633-647): Function needs to specify overridden contracts B and D.
// TypeError: (533-550): Invalid contract specified in override list: C.
// TypeError: (603-617): Function needs to specify overridden contracts B and D.

View File

@ -0,0 +1,9 @@
abstract contract A {
int public testvar;
}
abstract contract X is A {
int public override testvar;
}
// ----
// DeclarationError: (73-100): Identifier already declared.
// TypeError: (84-92): Public state variable has override specified but does not override anything.

View File

@ -6,8 +6,7 @@ abstract contract B {
function test() internal virtual returns (uint256);
}
abstract contract X is A, B {
int public override testvar;
function test() internal override virtual returns (uint256);
}
// ----
// TypeError: (203-326): Derived contract must override function "foo". Function with the same name and parameter types defined in two or more base classes.
// TypeError: (203-296): Derived contract must override function "foo". Function with the same name and parameter types defined in two or more base classes.

View File

@ -16,10 +16,9 @@ abstract contract X is A, B, C, D {
struct MyStruct { int abc; }
enum ENUM { F,G,H }
int public override testvar;
function test() internal override virtual returns (uint256);
function foo() internal override(MyStruct, ENUM, A, B, C, D) virtual returns (uint256);
}
// ----
// TypeError: (632-640): Expected contract but got struct X.MyStruct.
// TypeError: (642-646): Expected contract but got enum X.ENUM.
// TypeError: (602-610): Expected contract but got struct X.MyStruct.
// TypeError: (612-616): Expected contract but got enum X.ENUM.

View File

@ -0,0 +1,8 @@
contract A {
function foo() internal virtual pure returns(uint) { return 5; }
}
contract X is A {
uint public foo;
}
// ----
// TypeError: (100-115): Public state variables can only override functions with external visibility.

View File

@ -0,0 +1,7 @@
contract A {
function foo(uint) internal virtual pure returns(uint) { return 5; }
}
contract X is A {
uint public foo;
}
// ----

View File

@ -0,0 +1,11 @@
contract A {
function foo() external virtual pure returns(uint) { return 5; }
}
contract B {
function foo() external virtual pure returns(uint) { return 5; }
}
contract X is A, B {
uint public override foo;
}
// ----
// TypeError: (162-211): Derived contract must override function "foo". Function with the same name and parameter types defined in two or more base classes.

View File

@ -0,0 +1,11 @@
contract A {
uint public foo;
}
contract B {
function foo() external virtual pure returns(uint) { return 5; }
}
contract X is A, B {
uint public override foo;
}
// ----
// DeclarationError: (136-160): Identifier already declared.

View File

@ -0,0 +1,9 @@
contract A {
function foo() external virtual pure returns(uint) { return 4; }
function foo(uint ) external virtual pure returns(uint) { return 4; }
function foo(uint , uint ) external pure virtual returns(A) { }
}
contract X is A {
uint public override foo;
}
// ----

View File

@ -0,0 +1,10 @@
contract A {
function foo() external virtual pure returns(A) { }
function foo(uint ) external virtual pure returns(uint) { return 4; }
function foo(uint , uint ) external pure virtual returns(A) { }
}
contract X is A {
uint public override foo;
}
// ----
// TypeError: (225-249): Overriding public state variable return types differ.

View File

@ -0,0 +1,8 @@
contract ERC20 {
function balanceOf(address) external virtual view returns (uint) {}
function balanceOf(uint) external virtual view returns (uint) {}
function balanceOf() external virtual view returns (uint) {}
}
contract C is ERC20 {
mapping(address => uint) public override balanceOf;
}

View File

@ -0,0 +1,10 @@
contract ERC20 {
function balanceOf(address, uint) external virtual view returns (uint) {}
function balanceOf(uint) external virtual view returns (uint) {}
function balanceOf() external virtual view returns (uint) {}
}
contract C is ERC20 {
mapping(address => uint) public override balanceOf;
}
// ----
// TypeError: (281-289): Public state variable has override specified but does not override anything.

View File

@ -0,0 +1,14 @@
contract A {
function foo() external virtual pure returns(uint) { return 5; }
}
contract B is A {
function foo() external virtual override pure returns(uint) { return 5; }
}
contract C is A {
function foo() external virtual override pure returns(uint) { return 5; }
}
contract X is B, C {
uint public override foo;
}
// ----
// TypeError: (271-320): Derived contract must override function "foo". Function with the same name and parameter types defined in two or more base classes.

View File

@ -0,0 +1,15 @@
contract A {
function foo() external virtual pure returns(uint) { return 5; }
}
contract B is A {
uint public override foo;
}
contract C is A {
function foo() external virtual override pure returns(uint) { return 5; }
}
contract X is B, C {
uint public override foo;
}
// ----
// DeclarationError: (245-269): Identifier already declared.
// TypeError: (223-272): Derived contract must override function "foo". Function with the same name and parameter types defined in two or more base classes.

View File

@ -0,0 +1,11 @@
contract A {
function foo() external virtual pure returns(uint) { return 5; }
}
contract B {
function foo() external virtual pure returns(uint) { return 5; }
}
contract X is A, B {
uint public override(A, B) foo;
}
// ----
// TypeError: (162-217): Derived contract must override function "foo". Function with the same name and parameter types defined in two or more base classes.

View File

@ -0,0 +1,5 @@
contract X {
uint public override foo;
}
// ----
// TypeError: (26-34): Public state variable has override specified but does not override anything.