FunctionDefinition.resolveVirtual(): Skip unimplemented functions when lookup happens via super

Author: cameel

Changelog.md

@@ -16,6 +16,7 @@ Compiler Features:
 
 Bugfixes:
  * AST: Do not output value of Yul literal if it is not a valid UTF-8 string.
+ * Code Generator: Fix internal error when super would have to skip an unimplemented function in the virtual resolution order.
  * SMTChecker: Fix internal error on struct constructor with fixed bytes member initialized with string literal.
  * Source Locations: Properly set source location of scoped blocks.
  * Standard JSON: Properly allow the ``inliner`` setting under ``settings.optimizer.details``.

libsolidity/ast/AST.cpp

@@ -397,6 +397,7 @@ FunctionDefinition const& FunctionDefinition::resolveVirtual(
 ) const
 {
 	solAssert(!isConstructor(), "");
+
 	// If we are not doing super-lookup and the function is not virtual, we can stop here.
 	if (_searchStart == nullptr && !virtualSemantics())
 		return *this;
@@ -407,19 +408,30 @@ FunctionDefinition const& FunctionDefinition::resolveVirtual(
 
 	FunctionType const* functionType = TypeProvider::function(*this)->asExternallyCallableFunction(false);
 
+	bool foundSearchStart = (_searchStart == nullptr);
 	for (ContractDefinition const* c: _mostDerivedContract.annotation().linearizedBaseContracts)
 	{
-		if (_searchStart != nullptr && c != _searchStart)
+		if (!foundSearchStart && c != _searchStart)
 			continue;
-		_searchStart = nullptr;
+		else
+			foundSearchStart = true;
+
 		for (FunctionDefinition const* function: c->definedFunctions())
 			if (
 				function->name() == name() &&
 				!function->isConstructor() &&
+				// Lookup via super should skip an unimplemented function because there must be
+				// another, implemented one behind it (otherwise we would fail at analysis stage).
+				// For ordinary virtual lookup, on the other hand, the only case in which we might
+				// encounter an unimplemented function is when there is no implementation at all
+				// and that only happens if the code passed analysis due to the call never actually
+				// happening (i.e. it's inside a function that gets overridden).
+				(function->isImplemented() || _searchStart == nullptr) &&
 				FunctionType(*function).asExternallyCallableFunction(false)->hasEqualParameterTypes(*functionType)
 			)
 				return *function;
 	}
+
 	solAssert(false, "Virtual function " + name() + " not found.");
 	return *this; // not reached
 }

test/libsolidity/semanticTests/functionCall/inheritance/call_unimplemented_base.sol

@@ -2,13 +2,17 @@ abstract contract I
 {
     function a() internal view virtual returns(uint256);
 }
-abstract contract V is I
+abstract contract J is I
+{
+    function a() internal view virtual override returns(uint256);
+}
+abstract contract V is J
 {
     function b() public view returns(uint256) { return a(); }
 }
 contract C is V
 {
-    function a() internal view override returns (uint256) { return 42;}
+    function a() internal view override returns (uint256) { return 42; }
 }
 // ====
 // compileToEwasm: also

test/libsolidity/semanticTests/functionCall/inheritance/super_skip_unimplemented_in_abstract_contract.sol

@@ -0,0 +1,22 @@
+contract A {
+    function f() public virtual returns (uint) {
+        return 42;
+    }
+}
+
+abstract contract I {
+    function f() external virtual returns (uint);
+}
+
+contract B is A, I {
+    function f() override(A, I) public returns (uint) {
+        // I.f() is before A.f() in the C3 linearized order
+        // but it has no implementation.
+        return super.f();
+    }
+}
+// ====
+// compileToEwasm: also
+// compileViaYul: also
+// ----
+// f() -> 42

test/libsolidity/semanticTests/functionCall/inheritance/super_skip_unimplemented_in_interface.sol

@@ -0,0 +1,22 @@
+contract A {
+    function f() public virtual returns (uint) {
+        return 42;
+    }
+}
+
+interface I {
+    function f() external returns (uint);
+}
+
+contract B is A, I {
+    function f() override(A, I) public returns (uint) {
+        // I.f() is before A.f() in the C3 linearized order
+        // but it has no implementation.
+        return super.f();
+    }
+}
+// ====
+// compileToEwasm: also
+// compileViaYul: also
+// ----
+// f() -> 42

test/libsolidity/syntaxTests/functionCalls/call_unimplemented_base_via_super.sol

@@ -0,0 +1,18 @@
+abstract contract I {
+    function a() internal view virtual returns(uint256);
+}
+
+abstract contract C is I {
+    function f() public view returns(uint256) {
+        return I.a();
+    }
+}
+
+abstract contract D is I {
+    function f() public view returns(uint256) {
+        return super.a();
+    }
+}
+// ----
+// TypeError 7501: (172-177): Cannot call unimplemented base function.
+// TypeError 9582: (278-285): Member "a" not found or not visible after argument-dependent lookup in type(contract super D).

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

@@ -0,0 +1,13 @@
+contract A {
+    function f() public virtual {}
+}
+abstract contract B {
+    function f() public virtual;
+}
+contract C is A, B {
+    function f() public virtual override(A, B) {
+        B.f(); // Should not skip over to A.f() just because B.f() has no implementation.
+    }
+}
+// ----
+// TypeError 7501: (185-190): Cannot call unimplemented base function.

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

@@ -0,0 +1,12 @@
+contract A {
+    function f() public virtual {}
+}
+abstract contract B {
+    function f() public virtual;
+}
+contract C is A, B {
+    function f() public override(A, B) {
+        super.f(); // super should skip the unimplemented B.f() and call A.f() instead.
+    }
+}
+// ----

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

@@ -0,0 +1,12 @@
+contract A {
+    function f() public virtual {}
+}
+abstract contract B {
+    function f() public virtual;
+}
+contract C is A, B {
+    function f() public override(A, B) {
+        // This is fine. The unimplemented B.f() is not used.
+    }
+}
+// ----

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

@@ -0,0 +1,13 @@
+contract A {
+    function f() public virtual {}
+}
+abstract contract B {
+    function f() public virtual;
+}
+abstract contract C is A, B {
+    function g() public {
+        f(); // Would call B.f() if we did not require an override in C.
+    }
+}
+// ----
+// TypeError 6480: (107-243): Derived contract must override function "f". Two or more base classes define function with same name and parameter types.