diff --git a/Changelog.md b/Changelog.md index d7c3eec00..26c958076 100644 --- a/Changelog.md +++ b/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. * Control Flow Graph: Take internal calls to functions that always revert into account for reporting unused or unassigned variables. * SMTChecker: Fix internal error on struct constructor with fixed bytes member initialized with string literal. * SMTChecker: Fix internal error on external calls from the constructor. diff --git a/libsolidity/ast/AST.cpp b/libsolidity/ast/AST.cpp index 537846007..e217fecfa 100644 --- a/libsolidity/ast/AST.cpp +++ b/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,26 @@ 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() && + // With super lookup analysis guarantees that there is an implemented function in the chain. + // With virtual lookup there are valid cases where returning an unimplemented one is fine. + (function->isImplemented() || _searchStart == nullptr) && FunctionType(*function).asExternallyCallableFunction(false)->hasEqualParameterTypes(*functionType) ) return *function; } + solAssert(false, "Virtual function " + name() + " not found."); return *this; // not reached } diff --git a/libsolidity/codegen/CompilerContext.cpp b/libsolidity/codegen/CompilerContext.cpp index 398e362f5..6de51298d 100644 --- a/libsolidity/codegen/CompilerContext.cpp +++ b/libsolidity/codegen/CompilerContext.cpp @@ -286,7 +286,11 @@ FunctionDefinition const& CompilerContext::superFunction(FunctionDefinition cons solAssert(m_mostDerivedContract, "No most derived contract set."); ContractDefinition const* super = _base.superContract(mostDerivedContract()); solAssert(super, "Super contract not available."); - return _function.resolveVirtual(mostDerivedContract(), super); + + FunctionDefinition const& resolvedFunction = _function.resolveVirtual(mostDerivedContract(), super); + solAssert(resolvedFunction.isImplemented(), ""); + + return resolvedFunction; } ContractDefinition const& CompilerContext::mostDerivedContract() const diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 5afa4d26f..fca076b65 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -604,6 +604,8 @@ bool ContractCompiler::visit(VariableDeclaration const& _variableDeclaration) bool ContractCompiler::visit(FunctionDefinition const& _function) { + solAssert(_function.isImplemented(), ""); + CompilerContext::LocationSetter locationSetter(m_context, _function); m_context.startFunction(_function); diff --git a/test/libsolidity/semanticTests/functionCall/inheritance/call_unimplemented_base.sol b/test/libsolidity/semanticTests/functionCall/inheritance/call_unimplemented_base.sol index 4edfa5cd9..4d84dcd7a 100644 --- a/test/libsolidity/semanticTests/functionCall/inheritance/call_unimplemented_base.sol +++ b/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 diff --git a/test/libsolidity/semanticTests/functionCall/inheritance/super_skip_unimplemented_in_abstract_contract.sol b/test/libsolidity/semanticTests/functionCall/inheritance/super_skip_unimplemented_in_abstract_contract.sol new file mode 100644 index 000000000..82dc5d513 --- /dev/null +++ b/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 diff --git a/test/libsolidity/semanticTests/functionCall/inheritance/super_skip_unimplemented_in_interface.sol b/test/libsolidity/semanticTests/functionCall/inheritance/super_skip_unimplemented_in_interface.sol new file mode 100644 index 000000000..8f7914013 --- /dev/null +++ b/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 diff --git a/test/libsolidity/syntaxTests/functionCalls/call_unimplemented_base_via_super.sol b/test/libsolidity/syntaxTests/functionCalls/call_unimplemented_base_via_super.sol new file mode 100644 index 000000000..4b7d45624 --- /dev/null +++ b/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). diff --git a/test/libsolidity/syntaxTests/inheritance/override/override_implemented_and_unimplemented_with_implemented_call_via_contract.sol b/test/libsolidity/syntaxTests/inheritance/override/override_implemented_and_unimplemented_with_implemented_call_via_contract.sol new file mode 100644 index 000000000..72e0eaa1b --- /dev/null +++ b/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. diff --git a/test/libsolidity/syntaxTests/inheritance/override/override_implemented_and_unimplemented_with_implemented_call_via_super.sol b/test/libsolidity/syntaxTests/inheritance/override/override_implemented_and_unimplemented_with_implemented_call_via_super.sol new file mode 100644 index 000000000..61d121f03 --- /dev/null +++ b/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. + } +} +// ---- diff --git a/test/libsolidity/syntaxTests/inheritance/override/override_implemented_and_unimplemented_with_implemented_no_call.sol b/test/libsolidity/syntaxTests/inheritance/override/override_implemented_and_unimplemented_with_implemented_no_call.sol new file mode 100644 index 000000000..3a9d94ea1 --- /dev/null +++ b/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. + } +} +// ---- diff --git a/test/libsolidity/syntaxTests/inheritance/override/override_implemented_and_unimplemented_with_implemented_virtual_call_into_base_contract.sol b/test/libsolidity/syntaxTests/inheritance/override/override_implemented_and_unimplemented_with_implemented_virtual_call_into_base_contract.sol new file mode 100644 index 000000000..5321bbdb0 --- /dev/null +++ b/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. diff --git a/test/libsolidity/syntaxTests/inheritance/override/override_implemented_with_unimplemented_then_implemented.sol b/test/libsolidity/syntaxTests/inheritance/override/override_implemented_with_unimplemented_then_implemented.sol new file mode 100644 index 000000000..b1721e164 --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/override/override_implemented_with_unimplemented_then_implemented.sol @@ -0,0 +1,11 @@ +contract A { + function f() public virtual {} +} +abstract contract B is A { + function f() public virtual override; +} +contract C is B { + function f() public virtual override {} +} +// ---- +// TypeError 4593: (81-118): Overriding an implemented function with an unimplemented function is not allowed. diff --git a/test/libsolidity/syntaxTests/inheritance/override/override_unimplemented_fail.sol b/test/libsolidity/syntaxTests/inheritance/override/override_unimplemented_and_implemented_with_unimplemented.sol similarity index 100% rename from test/libsolidity/syntaxTests/inheritance/override/override_unimplemented_fail.sol rename to test/libsolidity/syntaxTests/inheritance/override/override_unimplemented_and_implemented_with_unimplemented.sol diff --git a/test/libsolidity/syntaxTests/inheritance/override/override_unimplemented_fine.sol b/test/libsolidity/syntaxTests/inheritance/override/override_unimplemented_and_unimplemented_with_unimplemented.sol similarity index 100% rename from test/libsolidity/syntaxTests/inheritance/override/override_unimplemented_fine.sol rename to test/libsolidity/syntaxTests/inheritance/override/override_unimplemented_and_unimplemented_with_unimplemented.sol diff --git a/test/libsolidity/syntaxTests/modifiers/cross_contract_super.sol b/test/libsolidity/syntaxTests/modifiers/cross_contract_super.sol new file mode 100644 index 000000000..b70ba3092 --- /dev/null +++ b/test/libsolidity/syntaxTests/modifiers/cross_contract_super.sol @@ -0,0 +1,9 @@ +contract C { + modifier m() { _; } +} +contract D is C { + function f() super.m public { + } +} +// ---- +// DeclarationError 7920: (74-81): Identifier not found or not unique.