diff --git a/Changelog.md b/Changelog.md index 1195fee1a..442139b52 100644 --- a/Changelog.md +++ b/Changelog.md @@ -29,6 +29,8 @@ Compiler Features: Bugfixes: * NatSpec: Constructors and functions have consistent userdoc output. + * Inheritance: Disallow public state variables overwriting ``pure`` functions. + * State Mutability: Constant public state variables are considered ``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..abff46452 100644 --- a/libsolidity/analysis/OverrideChecker.cpp +++ b/libsolidity/analysis/OverrideChecker.cpp @@ -289,7 +289,7 @@ StateMutability OverrideProxy::stateMutability() const return std::visit(GenericVisitor{ [&](FunctionDefinition const* _item) { return _item->stateMutability(); }, [&](ModifierDefinition const*) { solAssert(false, "Requested state mutability from modifier."); return StateMutability{}; }, - [&](VariableDeclaration const*) { return StateMutability::View; } + [&](VariableDeclaration const* _var) { return _var->isConstant() ? StateMutability::Pure : StateMutability::View; } }, m_item); } @@ -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/diamond_top_implemented_intermediate_empty_bottom_public_state_variable.sol b/test/libsolidity/syntaxTests/inheritance/override/diamond_top_implemented_intermediate_empty_bottom_public_state_variable.sol index 3af8fd6e1..61c159b08 100644 --- a/test/libsolidity/syntaxTests/inheritance/override/diamond_top_implemented_intermediate_empty_bottom_public_state_variable.sol +++ b/test/libsolidity/syntaxTests/inheritance/override/diamond_top_implemented_intermediate_empty_bottom_public_state_variable.sol @@ -1,5 +1,5 @@ contract I { - function f() external pure virtual returns (uint) { return 1; } + function f() external view virtual returns (uint) { return 1; } } contract A is I { @@ -11,3 +11,4 @@ contract C is A, B { uint public override f; } +// ---- diff --git a/test/libsolidity/syntaxTests/inheritance/override/diamond_top_implemented_intermediate_implemented_public_state_variable.sol b/test/libsolidity/syntaxTests/inheritance/override/diamond_top_implemented_intermediate_implemented_public_state_variable.sol index d59dade35..392b6640b 100644 --- a/test/libsolidity/syntaxTests/inheritance/override/diamond_top_implemented_intermediate_implemented_public_state_variable.sol +++ b/test/libsolidity/syntaxTests/inheritance/override/diamond_top_implemented_intermediate_implemented_public_state_variable.sol @@ -1,5 +1,5 @@ contract I { - function f() external pure virtual returns (uint) { return 1; } + function f() external view virtual returns (uint) { return 1; } } contract A is I { diff --git a/test/libsolidity/syntaxTests/inheritance/override/diamond_top_implemented_intermediate_public_state_variable.sol b/test/libsolidity/syntaxTests/inheritance/override/diamond_top_implemented_intermediate_public_state_variable.sol index 390e763e8..5e7052a62 100644 --- a/test/libsolidity/syntaxTests/inheritance/override/diamond_top_implemented_intermediate_public_state_variable.sol +++ b/test/libsolidity/syntaxTests/inheritance/override/diamond_top_implemented_intermediate_public_state_variable.sol @@ -1,5 +1,5 @@ contract I { - function f() external pure virtual returns (uint) { return 1; } + function f() external view virtual returns (uint) { return 1; } } contract A is I { diff --git a/test/libsolidity/syntaxTests/inheritance/override/public_constant_var_overrides_pure.sol b/test/libsolidity/syntaxTests/inheritance/override/public_constant_var_overrides_pure.sol new file mode 100644 index 000000000..92f83a01f --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/override/public_constant_var_overrides_pure.sol @@ -0,0 +1,7 @@ +abstract contract C { + function foo() external pure virtual returns (uint); +} +contract X is C { + uint public constant override foo = 7; +} +// ---- diff --git a/test/libsolidity/syntaxTests/inheritance/override/public_immutable_var_overrides_pure.sol b/test/libsolidity/syntaxTests/inheritance/override/public_immutable_var_overrides_pure.sol new file mode 100644 index 000000000..53f7170f4 --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/override/public_immutable_var_overrides_pure.sol @@ -0,0 +1,8 @@ +abstract contract C { + function foo() external pure virtual returns (uint); +} +contract X is C { + uint public immutable override foo = 7; +} +// ---- +// TypeError 6959: (100-138): Overriding public state variable changes state mutability from "pure" to "view". diff --git a/test/libsolidity/syntaxTests/inheritance/override/public_var_no_override_but_function.sol b/test/libsolidity/syntaxTests/inheritance/override/public_var_no_override_but_function.sol index 059450fe6..15b6e7b5d 100644 --- a/test/libsolidity/syntaxTests/inheritance/override/public_var_no_override_but_function.sol +++ b/test/libsolidity/syntaxTests/inheritance/override/public_var_no_override_but_function.sol @@ -1,5 +1,5 @@ contract A { - function foo() internal virtual pure returns(uint) { return 5; } + function foo() internal virtual view returns(uint) { return 5; } } contract X is A { uint public foo; 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". diff --git a/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple.sol b/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple.sol index f0a79beb2..0222db63b 100644 --- a/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple.sol +++ b/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple.sol @@ -1,8 +1,8 @@ contract A { - function foo() external virtual pure returns(uint) { return 5; } + function foo() external virtual view returns(uint) { return 5; } } contract B { - function foo() external virtual pure returns(uint) { return 5; } + function foo() external virtual view returns(uint) { return 5; } } contract X is A, B { uint public override foo; diff --git a/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple1.sol b/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple1.sol index 719c917c7..7cba6cf0a 100644 --- a/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple1.sol +++ b/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple1.sol @@ -2,7 +2,7 @@ contract A { uint public foo; } contract B { - function foo() external virtual pure returns(uint) { return 5; } + function foo() external virtual view returns(uint) { return 5; } } contract X is A, B { uint public override foo; diff --git a/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple2.sol b/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple2.sol index fa0c17880..42c0a97f1 100644 --- a/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple2.sol +++ b/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple2.sol @@ -1,7 +1,7 @@ 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) { } + function foo() external virtual view returns(uint) { return 4; } + function foo(uint ) external virtual view returns(uint) { return 4; } + function foo(uint , uint ) external view virtual returns(A) { } } contract X is A { uint public override foo; diff --git a/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple3.sol b/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple3.sol index 1ef6cf296..35e3e78ac 100644 --- a/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple3.sol +++ b/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple3.sol @@ -1,7 +1,7 @@ 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) { } + function foo() external virtual view returns(A) { } + function foo(uint ) external virtual view returns(uint) { return 4; } + function foo(uint , uint ) external view virtual returns(A) { } } contract X is A { uint public override foo; diff --git a/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple_diamond.sol b/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple_diamond.sol index ffac63dc8..67e95e129 100644 --- a/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple_diamond.sol +++ b/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple_diamond.sol @@ -1,11 +1,11 @@ contract A { - function foo() external virtual pure returns(uint) { return 5; } + function foo() external virtual view returns(uint) { return 5; } } contract B is A { - function foo() external virtual override pure returns(uint) { return 5; } + function foo() external virtual override view returns(uint) { return 5; } } contract C is A { - function foo() external virtual override pure returns(uint) { return 5; } + function foo() external virtual override view returns(uint) { return 5; } } contract X is B, C { uint public override foo; diff --git a/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple_diamond1.sol b/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple_diamond1.sol index b0b4d22fc..9830bfce7 100644 --- a/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple_diamond1.sol +++ b/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple_diamond1.sol @@ -1,11 +1,11 @@ contract A { - function foo() external virtual pure returns(uint) { return 5; } + function foo() external virtual view 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; } + function foo() external virtual override view returns(uint) { return 5; } } contract X is B, C { uint public override foo; diff --git a/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple_diamond2.sol b/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple_diamond2.sol index f303b82cd..0ce74dc6b 100644 --- a/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple_diamond2.sol +++ b/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple_diamond2.sol @@ -1,11 +1,11 @@ contract A { - function foo() external virtual pure returns(uint) { return 5; } + function foo() external virtual view 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; } + function foo() external virtual override view returns(uint) { return 5; } } contract X is B, C { uint public override(A, C) foo; diff --git a/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple_explicit_override.sol b/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple_explicit_override.sol index caf3f9ea8..b0579b307 100644 --- a/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple_explicit_override.sol +++ b/test/libsolidity/syntaxTests/inheritance/override/public_vars_multiple_explicit_override.sol @@ -1,8 +1,8 @@ contract A { - function foo() external virtual pure returns(uint) { return 5; } + function foo() external virtual view returns(uint) { return 5; } } contract B { - function foo() external virtual pure returns(uint) { return 5; } + function foo() external virtual view returns(uint) { return 5; } } contract X is A, B { uint public override(A, B) foo;