Merge pull request #9416 from ethereum/disallowOverridePure

Disallow public state variables overriding pure functions.

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)

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

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."
+			);
 	}
 }
 

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;
 }
+// ----

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
 {

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
 {

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;
+}
+// ----

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

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;

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

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;

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;

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;

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;

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;

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;

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;

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;