From c47da51eab8e5c4ebff0f6dd2ca748179aa21ab4 Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Wed, 2 Feb 2022 17:18:38 +0100 Subject: [PATCH] Consider all grandparents in override analysis. --- Changelog.md | 1 + libsolidity/analysis/OverrideChecker.cpp | 7 +++--- .../correct_choice_for_base_function.sol | 17 ++++++++++++++ ...ce_for_base_function_abstract_contract.sol | 17 ++++++++++++++ .../interface_and_base_override_err.sol | 18 +++++++++++++++ .../interface_and_base_override_fine.sol | 16 ++++++++++++++ .../override/override_multi_layered_error.sol | 22 +++++++++++++++++++ .../override/override_multi_layered_fine.sol | 18 +++++++++++++++ .../override_multi_layered_fine_abstract.sol | 18 +++++++++++++++ 9 files changed, 131 insertions(+), 3 deletions(-) create mode 100644 test/libsolidity/syntaxTests/inheritance/override/correct_choice_for_base_function.sol create mode 100644 test/libsolidity/syntaxTests/inheritance/override/correct_choice_for_base_function_abstract_contract.sol create mode 100644 test/libsolidity/syntaxTests/inheritance/override/interface_and_base_override_err.sol create mode 100644 test/libsolidity/syntaxTests/inheritance/override/interface_and_base_override_fine.sol create mode 100644 test/libsolidity/syntaxTests/inheritance/override/override_multi_layered_error.sol create mode 100644 test/libsolidity/syntaxTests/inheritance/override/override_multi_layered_fine.sol create mode 100644 test/libsolidity/syntaxTests/inheritance/override/override_multi_layered_fine_abstract.sol diff --git a/Changelog.md b/Changelog.md index f688af733..364fe35d8 100644 --- a/Changelog.md +++ b/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. diff --git a/libsolidity/analysis/OverrideChecker.cpp b/libsolidity/analysis/OverrideChecker.cpp index 6f44f92ac..2f83748fb 100644 --- a/libsolidity/analysis/OverrideChecker.cpp +++ b/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; diff --git a/test/libsolidity/syntaxTests/inheritance/override/correct_choice_for_base_function.sol b/test/libsolidity/syntaxTests/inheritance/override/correct_choice_for_base_function.sol new file mode 100644 index 000000000..a7da5e4d1 --- /dev/null +++ b/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) {} +} diff --git a/test/libsolidity/syntaxTests/inheritance/override/correct_choice_for_base_function_abstract_contract.sol b/test/libsolidity/syntaxTests/inheritance/override/correct_choice_for_base_function_abstract_contract.sol new file mode 100644 index 000000000..881402620 --- /dev/null +++ b/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) {} +} diff --git a/test/libsolidity/syntaxTests/inheritance/override/interface_and_base_override_err.sol b/test/libsolidity/syntaxTests/inheritance/override/interface_and_base_override_err.sol new file mode 100644 index 000000000..e3f491770 --- /dev/null +++ b/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". diff --git a/test/libsolidity/syntaxTests/inheritance/override/interface_and_base_override_fine.sol b/test/libsolidity/syntaxTests/inheritance/override/interface_and_base_override_fine.sol new file mode 100644 index 000000000..e20bcd888 --- /dev/null +++ b/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) {} +} +// ---- diff --git a/test/libsolidity/syntaxTests/inheritance/override/override_multi_layered_error.sol b/test/libsolidity/syntaxTests/inheritance/override/override_multi_layered_error.sol new file mode 100644 index 000000000..309d905d1 --- /dev/null +++ b/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". diff --git a/test/libsolidity/syntaxTests/inheritance/override/override_multi_layered_fine.sol b/test/libsolidity/syntaxTests/inheritance/override/override_multi_layered_fine.sol new file mode 100644 index 000000000..3c12d3ba6 --- /dev/null +++ b/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) {} +} diff --git a/test/libsolidity/syntaxTests/inheritance/override/override_multi_layered_fine_abstract.sol b/test/libsolidity/syntaxTests/inheritance/override/override_multi_layered_fine_abstract.sol new file mode 100644 index 000000000..1671c9d0d --- /dev/null +++ b/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) {} +}