Merge pull request #12616 from ethereum/missingOverrideRequirement

Fix missing override errors when indirectly inheriting interfaces.

Changelog.md

@@ -19,6 +19,7 @@ Bugfixes:
  * Control Flow Graph: Perform proper virtual lookup for modifiers for uninitialized variable and unreachable code analysis.
  * General: ``string.concat`` now properly takes strings as arguments and returns ``string memory``. It was accidentally introduced as a copy of ``bytes.concat`` before.
  * Immutables: Fix wrong error when the constructor of a base contract uses ``return`` and the derived contract contains immutable variables.
+ * Inheritance: Consider functions in all ancestors during override analysis.
  * IR Generator: Add missing cleanup during the conversion of fixed bytes types to smaller fixed bytes types.
  * IR Generator: Add missing cleanup for indexed event arguments of value type.
  * IR Generator: Fix internal error when copying reference types in calldata and storage to struct or array members in memory.

libsolidity/analysis/OverrideChecker.cpp

@@ -897,10 +897,11 @@ OverrideChecker::OverrideProxyBySignatureMultiSet const& OverrideChecker::inheri
 				if (var->isPublic())
 					functionsInBase.emplace(OverrideProxy{var});
 
-			for (OverrideProxy const& func: inheritedFunctions(*base))
-				functionsInBase.insert(func);
-
 			result += functionsInBase;
+
+			for (OverrideProxy const& func: inheritedFunctions(*base))
+				if (!functionsInBase.count(func))
+					result.insert(func);
 		}
 
 		m_inheritedFunctions[&_contract] = result;

test/libsolidity/syntaxTests/inheritance/override/correct_choice_for_base_function.sol

@@ -0,0 +1,17 @@
+interface IBase {
+    function foo() external view;
+}
+
+contract Base is IBase {
+    function foo() public virtual view {}
+}
+
+interface IExt is IBase {}
+
+contract Ext is IExt, Base {}
+
+contract T { function foo() public virtual view {} }
+
+contract Impl is Ext, T {
+    function foo() public view override(IBase, Base, T) {}
+}

test/libsolidity/syntaxTests/inheritance/override/correct_choice_for_base_function_abstract_contract.sol

@@ -0,0 +1,17 @@
+abstract contract IBase {
+    function foo() external view virtual;
+}
+
+contract Base is IBase {
+    function foo() public virtual override view {}
+}
+
+abstract contract IExt is IBase {}
+
+contract Ext is IExt, Base {}
+
+contract T { function foo() public virtual view {} }
+
+contract Impl is Ext, T {
+    function foo() public view override(IBase, Base, T) {}
+}

test/libsolidity/syntaxTests/inheritance/override/interface_and_base_override_err.sol

@@ -0,0 +1,18 @@
+interface IBase {
+    function foo() external view;
+}
+
+contract Base is IBase {
+    function foo() public virtual view {}
+}
+
+interface IExt is IBase {}
+
+contract Ext is IExt, Base {}
+
+contract Impl is Ext {
+    function foo() public view {}
+}
+// ----
+// TypeError 9456: (211-240): Overriding function is missing "override" specifier.
+// TypeError 4327: (211-240): Function needs to specify overridden contracts "Base" and "IBase".

test/libsolidity/syntaxTests/inheritance/override/interface_and_base_override_fine.sol

@@ -0,0 +1,16 @@
+interface IBase {
+    function foo() external view;
+}
+
+contract Base is IBase {
+    function foo() public virtual view {}
+}
+
+interface IExt is IBase {}
+
+contract Ext is IExt, Base {}
+
+contract Impl is Ext {
+    function foo() public view override (IBase, Base) {}
+}
+// ----

test/libsolidity/syntaxTests/inheritance/override/override_multi_layered_error.sol

@@ -0,0 +1,22 @@
+interface IBase {
+    function foo() external view;
+}
+
+contract Base1 is IBase { function foo() public virtual view {} }
+contract Base2 is IBase { function foo() public virtual view {} }
+
+interface IExt1a is IBase {}
+interface IExt1b is IBase {}
+interface IExt2a is IBase {}
+interface IExt2b is IBase {}
+
+contract Ext1 is IExt1a, IExt1b, Base1 {}
+contract Ext2 is IExt2a, IExt2b, Base2 {}
+
+contract Impl is Ext1, Ext2 {
+    function foo() public view {}
+}
+// ----
+// TypeError 9456: (424-453): Overriding function is missing "override" specifier.
+// TypeError 9456: (424-453): Overriding function is missing "override" specifier.
+// TypeError 4327: (424-453): Function needs to specify overridden contracts "Base1", "Base2" and "IBase".

test/libsolidity/syntaxTests/inheritance/override/override_multi_layered_fine.sol

@@ -0,0 +1,18 @@
+interface IBase {
+    function foo() external view;
+}
+
+contract Base1 is IBase { function foo() public virtual view {} }
+contract Base2 is IBase { function foo() public virtual view {} }
+
+interface IExt1a is IBase {}
+interface IExt1b is IBase {}
+interface IExt2a is IBase {}
+interface IExt2b is IBase {}
+
+contract Ext1 is IExt1a, IExt1b, Base1 {}
+contract Ext2 is IExt2a, IExt2b, Base2 {}
+
+contract Impl is Ext1, Ext2 {
+    function foo() public view override (IBase, Base1, Base2) {}
+}

test/libsolidity/syntaxTests/inheritance/override/override_multi_layered_fine_abstract.sol

@@ -0,0 +1,18 @@
+interface IBase {
+    function foo() external view;
+}
+
+contract Base1 is IBase { function foo() public virtual view {} }
+contract Base2 is IBase { function foo() public virtual view {} }
+
+interface IExt1a is IBase {}
+abstract contract IExt1b is IBase {}
+abstract contract IExt2a is IBase {}
+interface IExt2b is IBase {}
+
+contract Ext1 is IExt1a, IExt1b, Base1 {}
+contract Ext2 is IExt2a, IExt2b, Base2 {}
+
+contract Impl is Ext1, Ext2 {
+    function foo() public view override (IBase, Base1, Base2) {}
+}