From d3647b13e4362a17e72fe6d36b53f152230d77d4 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 14 Jul 2020 20:11:01 +0200 Subject: [PATCH] Disallow public state variables overriding pure functions. --- Changelog.md | 1 + docs/contracts/inheritance.rst | 2 +- libsolidity/analysis/OverrideChecker.cpp | 52 +++++++++---------- .../override/public_var_overrides_pure.sol | 8 +++ 4 files changed, 36 insertions(+), 27 deletions(-) create mode 100644 test/libsolidity/syntaxTests/inheritance/override/public_var_overrides_pure.sol diff --git a/Changelog.md b/Changelog.md index 1195fee1a..c2aa865bc 100644 --- a/Changelog.md +++ b/Changelog.md @@ -29,6 +29,7 @@ Compiler Features: Bugfixes: * NatSpec: Constructors and functions have consistent userdoc output. + * Inheritance: Disallow public state variables overwriting ``pure`` functions. ### 0.6.12 (unreleased) diff --git a/docs/contracts/inheritance.rst b/docs/contracts/inheritance.rst index 343604a34..f628d49fb 100644 --- a/docs/contracts/inheritance.rst +++ b/docs/contracts/inheritance.rst @@ -310,7 +310,7 @@ of the variable: contract A { - function f() external pure virtual returns(uint) { return 5; } + function f() external view virtual returns(uint) { return 5; } } contract B is A diff --git a/libsolidity/analysis/OverrideChecker.cpp b/libsolidity/analysis/OverrideChecker.cpp index c58fd2c85..b5fe33ade 100644 --- a/libsolidity/analysis/OverrideChecker.cpp +++ b/libsolidity/analysis/OverrideChecker.cpp @@ -584,35 +584,35 @@ void OverrideChecker::checkOverride(OverrideProxy const& _overriding, OverridePr "Overridden " + _overriding.astNodeName() + " is here:" ); - // This is only relevant for a function overriding a function. - if (_overriding.isFunction()) - { - // Stricter mutability is always okay except when super is Payable - if (( + // Stricter mutability is always okay except when super is Payable + if ( + (_overriding.isFunction() || _overriding.isVariable()) && + ( _overriding.stateMutability() > _super.stateMutability() || _super.stateMutability() == StateMutability::Payable - ) && - _overriding.stateMutability() != _super.stateMutability() - ) - overrideError( - _overriding, - _super, - 6959_error, - "Overriding function changes state mutability from \"" + - stateMutabilityToString(_super.stateMutability()) + - "\" to \"" + - stateMutabilityToString(_overriding.stateMutability()) + - "\"." - ); + ) && + _overriding.stateMutability() != _super.stateMutability() + ) + overrideError( + _overriding, + _super, + 6959_error, + "Overriding " + + _overriding.astNodeName() + + " changes state mutability from \"" + + stateMutabilityToString(_super.stateMutability()) + + "\" to \"" + + stateMutabilityToString(_overriding.stateMutability()) + + "\"." + ); - if (_overriding.unimplemented() && !_super.unimplemented()) - overrideError( - _overriding, - _super, - 4593_error, - "Overriding an implemented function with an unimplemented function is not allowed." - ); - } + if (_overriding.unimplemented() && !_super.unimplemented()) + overrideError( + _overriding, + _super, + 4593_error, + "Overriding an implemented function with an unimplemented function is not allowed." + ); } } diff --git a/test/libsolidity/syntaxTests/inheritance/override/public_var_overrides_pure.sol b/test/libsolidity/syntaxTests/inheritance/override/public_var_overrides_pure.sol new file mode 100644 index 000000000..835351b3d --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/override/public_var_overrides_pure.sol @@ -0,0 +1,8 @@ +abstract contract C { + function foo() external pure virtual returns (uint); +} +contract X is C { + uint public override foo; +} +// ---- +// TypeError 6959: (100-124): Overriding public state variable changes state mutability from "pure" to "view".