Merge pull request #11471 from ethereum/funcdef-unimplemented-modifier

Reports an error on unimplemented modifiers in a function definition.

Author: chriseth

Changelog.md

@@ -32,6 +32,7 @@ Bugfixes:
  * Type Checker: Fix internal compiler error when attempting to use an invalid external function type on pre-byzantium EVMs.
  * Type Checker: Make errors about (nested) mapping type in event or error parameter into fatal type errors.
  * Type Checker: Fix internal compiler error when overriding receive ether function with one having different parameters during inheritance.
+ * Type Checker: Fix internal compiler error when overriding an implemented modifier with an unimplemented one.
 
 
 AST Changes:

libsolidity/analysis/OverrideChecker.cpp

@@ -573,6 +573,19 @@ void OverrideChecker::checkOverride(OverrideProxy const& _overriding, OverridePr
 			);
 	}
 
+	if (_overriding.unimplemented() && !_super.unimplemented())
+	{
+		solAssert(!_overriding.isVariable() || !_overriding.unimplemented(), "");
+		overrideError(
+			_overriding,
+			_super,
+			4593_error,
+			"Overriding an implemented " + _super.astNodeName() +
+			" with an unimplemented " + _overriding.astNodeName() +
+			" is not allowed."
+		);
+	}
+
 	if (_super.isFunction())
 	{
 		FunctionType const* functionType = _overriding.functionType();
@@ -613,14 +626,6 @@ void OverrideChecker::checkOverride(OverrideProxy const& _overriding, OverridePr
 				stateMutabilityToString(_overriding.stateMutability()) +
 				"\"."
 			);
-
-		if (_overriding.unimplemented() && !_super.unimplemented())
-			overrideError(
-				_overriding,
-				_super,
-				4593_error,
-				"Overriding an implemented function with an unimplemented function is not allowed."
-			);
 	}
 }
 

test/libsolidity/syntaxTests/modifiers/modifier_abstract_override.sol

@@ -0,0 +1,11 @@
+contract A {
+    modifier m() virtual { _; }
+}
+abstract contract B is A {
+    modifier m() virtual override;
+}
+contract C is B {
+    function f() m public {}
+}
+// ----
+// TypeError 4593: (78-108): Overriding an implemented modifier with an unimplemented modifier is not allowed.

test/libsolidity/syntaxTests/modifiers/multiple_inheritance_unimplemented_override.sol

@@ -0,0 +1,11 @@
+contract A {
+    modifier m() virtual { _; }
+}
+abstract contract B {
+    modifier m() virtual;
+}
+contract C is A, B {
+    modifier m() override(A, B) { _; }
+    function f() m public {}
+}
+// ----

test/libsolidity/syntaxTests/modifiers/unimplemented_override_unimplemented.sol

@@ -0,0 +1,11 @@
+abstract contract A {
+    modifier m() virtual;
+}
+abstract contract B is A {
+    modifier m() virtual override;
+}
+abstract contract C is B {
+    modifier m() virtual override;
+    function f() m public {}
+}
+// ----

test/libsolidity/syntaxTests/modifiers/use_unimplemented_from_base.sol

@@ -0,0 +1,8 @@
+abstract contract A {
+    modifier m() virtual;
+    function f() m public {}
+}
+contract B is A {
+    modifier m() virtual override { _; }
+}
+// ----

test/libsolidity/syntaxTests/modifiers/use_unimplemented_on_overridden_func.sol

@@ -0,0 +1,8 @@
+abstract contract A {
+    modifier m() virtual;
+    function f() m public virtual {}
+}
+abstract contract B is A {
+    function f() public override {}
+}
+// ----