diff --git a/Changelog.md b/Changelog.md index 2f9d1b5f0..a1b502eee 100644 --- a/Changelog.md +++ b/Changelog.md @@ -22,6 +22,7 @@ Breaking changes: * Source mappings: Add "modifier depth" as a fifth field in the source mappings. * AST: Inline assembly is exported as structured JSON instead of plain string. * General: ``private`` cannot be used together with ``virtual``. + * Inheritance: State variable shadowing is now disallowed. Language Features: * Allow global enums and structs. diff --git a/docs/060-breaking-changes.rst b/docs/060-breaking-changes.rst index 43b29d6ea..7c1753cc0 100644 --- a/docs/060-breaking-changes.rst +++ b/docs/060-breaking-changes.rst @@ -38,6 +38,10 @@ This section lists purely syntactic changes that do not affect the behavior of e If the name contains a dot, its prefix up to the dot may not conflict with any declaration outside the inline assembly block. +* State variable shadowing is now disallowed. A derived contract can only + declare a state variable ``x``, if there is no visible state variable with + the same name in any of its bases. + Semantic Only Changes ===================== diff --git a/docs/contracts/inheritance.rst b/docs/contracts/inheritance.rst index cd2e3f8fc..6a29f83e0 100644 --- a/docs/contracts/inheritance.rst +++ b/docs/contracts/inheritance.rst @@ -19,6 +19,10 @@ is compiled into the created contract. This means that all internal calls to functions of base contracts also just use internal function calls (``super.f(..)`` will use JUMP and not a message call). +State variable shadowing is considered as an error. A derived contract can +only declare a state variable ``x``, if there is no visible state variable +with the same name in any of its bases. + The general inheritance system is very similar to `Python's `_, especially concerning multiple inheritance, but there are also diff --git a/libsolidity/analysis/NameAndTypeResolver.cpp b/libsolidity/analysis/NameAndTypeResolver.cpp index f85e20aaa..6bf4ef2c9 100644 --- a/libsolidity/analysis/NameAndTypeResolver.cpp +++ b/libsolidity/analysis/NameAndTypeResolver.cpp @@ -345,10 +345,6 @@ void NameAndTypeResolver::importInheritedScope(ContractDefinition const& _base) Declaration const* conflictingDeclaration = m_currentScope->conflictingDeclaration(*declaration); solAssert(conflictingDeclaration, ""); - // Usual shadowing is not an error - if (dynamic_cast(declaration) && dynamic_cast(conflictingDeclaration)) - continue; - // Usual shadowing is not an error if (dynamic_cast(declaration) && dynamic_cast(conflictingDeclaration)) continue; diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 8275536a0..77f342038 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -5971,38 +5971,6 @@ BOOST_AUTO_TEST_CASE(invalid_enum_as_external_arg) ABI_CHECK(callContractFunction("test()"), encodeArgs()); } - -BOOST_AUTO_TEST_CASE(proper_order_of_overwriting_of_attributes) -{ - // bug #1798 - char const* sourceCode = R"( - contract init { - function isOk() public virtual returns (bool) { return false; } - bool public ok = false; - } - contract fix { - function isOk() public virtual returns (bool) { return true; } - bool public ok = true; - } - - contract init_fix is init, fix { - function checkOk() public returns (bool) { return ok; } - function isOk() public override (init, fix) returns (bool) { return super.isOk(); } - } - contract fix_init is fix, init { - function checkOk() public returns (bool) { return ok; } - function isOk() public override (init, fix) returns (bool) { return super.isOk(); } - } - )"; - compileAndRun(sourceCode, 0, "init_fix"); - ABI_CHECK(callContractFunction("isOk()"), encodeArgs(true)); - ABI_CHECK(callContractFunction("ok()"), encodeArgs(true)); - - compileAndRun(sourceCode, 0, "fix_init"); - ABI_CHECK(callContractFunction("isOk()"), encodeArgs(false)); - ABI_CHECK(callContractFunction("ok()"), encodeArgs(false)); -} - BOOST_AUTO_TEST_CASE(struct_assign_reference_to_struct) { char const* sourceCode = R"( @@ -8051,42 +8019,6 @@ BOOST_AUTO_TEST_CASE(inherited_constant_state_var) ABI_CHECK(callContractFunction("f()"), encodeArgs(u256(7))); } -BOOST_AUTO_TEST_CASE(multiple_inherited_state_vars) -{ - char const* sourceCode = R"( - contract A { - uint x = 7; - } - contract B { - uint x = 9; - } - contract C is A, B { - function a() public returns (uint) { - return A.x; - } - function b() public returns (uint) { - return B.x; - } - function a_set(uint _x) public returns (uint) { - A.x = _x; - return 1; - } - function b_set(uint _x) public returns (uint) { - B.x = _x; - return 1; - } - } - )"; - - compileAndRun(sourceCode, 0, "C"); - ABI_CHECK(callContractFunction("a()"), encodeArgs(u256(7))); - ABI_CHECK(callContractFunction("b()"), encodeArgs(u256(9))); - ABI_CHECK(callContractFunction("a_set(uint256)", u256(1)), encodeArgs(u256(1))); - ABI_CHECK(callContractFunction("b_set(uint256)", u256(3)), encodeArgs(u256(1))); - ABI_CHECK(callContractFunction("a()"), encodeArgs(u256(1))); - ABI_CHECK(callContractFunction("b()"), encodeArgs(u256(3))); -} - BOOST_AUTO_TEST_CASE(constant_string_literal) { char const* sourceCode = R"( diff --git a/test/libsolidity/smtCheckerTests/inheritance/state_variables_2.sol b/test/libsolidity/smtCheckerTests/inheritance/state_variables_2.sol index 00d35923f..9e8a67eb8 100644 --- a/test/libsolidity/smtCheckerTests/inheritance/state_variables_2.sol +++ b/test/libsolidity/smtCheckerTests/inheritance/state_variables_2.sol @@ -2,12 +2,10 @@ pragma experimental SMTChecker; contract Base1 { uint x; - uint private t; } contract Base2 is Base1 { uint z; - uint private t; } contract C is Base2 { diff --git a/test/libsolidity/syntaxTests/inheritance/override/override.sol b/test/libsolidity/syntaxTests/inheritance/override/override.sol index bf79c702d..5925ba938 100644 --- a/test/libsolidity/syntaxTests/inheritance/override/override.sol +++ b/test/libsolidity/syntaxTests/inheritance/override/override.sol @@ -9,3 +9,4 @@ contract X is A { function test2() internal override(A) returns (uint256) {} } // ---- +// DeclarationError: (171-198): Identifier already declared. diff --git a/test/libsolidity/syntaxTests/inheritance/override/override_ambiguous.sol b/test/libsolidity/syntaxTests/inheritance/override/override_ambiguous.sol index 966c18f70..eec93cb6b 100644 --- a/test/libsolidity/syntaxTests/inheritance/override/override_ambiguous.sol +++ b/test/libsolidity/syntaxTests/inheritance/override/override_ambiguous.sol @@ -11,4 +11,5 @@ abstract contract X is A, B { function test() internal override returns (uint256) {} } // ---- +// DeclarationError: (257-284): Identifier already declared. // TypeError: (226-343): Derived contract must override function "foo". Function with the same name and parameter types defined in two or more base classes. diff --git a/test/libsolidity/syntaxTests/inheritance/override/override_multiple.sol b/test/libsolidity/syntaxTests/inheritance/override/override_multiple.sol index d8daaf406..8cb389e35 100644 --- a/test/libsolidity/syntaxTests/inheritance/override/override_multiple.sol +++ b/test/libsolidity/syntaxTests/inheritance/override/override_multiple.sol @@ -1,5 +1,4 @@ abstract contract A { - int public testvar; function foo() internal virtual returns (uint256); function test(uint8 _a) internal virtual returns (uint256) {} } @@ -18,4 +17,3 @@ contract X is A, B, C, D { function test() internal override returns (uint256) {} function foo() internal override(A, B, C, D) returns (uint256) {} } -// ---- diff --git a/test/libsolidity/syntaxTests/inheritance/override/override_multiple_duplicated.sol b/test/libsolidity/syntaxTests/inheritance/override/override_multiple_duplicated.sol index a2c05bbd0..dfdc30e41 100644 --- a/test/libsolidity/syntaxTests/inheritance/override/override_multiple_duplicated.sol +++ b/test/libsolidity/syntaxTests/inheritance/override/override_multiple_duplicated.sol @@ -20,6 +20,7 @@ abstract contract X is A, B, C, D { function foo() internal override(A, C, B, B, B, D ,D) virtual returns (uint256); } // ---- +// DeclarationError: (529-556): Identifier already declared. // TypeError: (599-600): Duplicate contract "D" found in override list of "test". // TypeError: (672-673): Duplicate contract "B" found in override list of "foo". // TypeError: (675-676): Duplicate contract "B" found in override list of "foo". diff --git a/test/libsolidity/syntaxTests/inheritance/override/override_multiple_missing.sol b/test/libsolidity/syntaxTests/inheritance/override/override_multiple_missing.sol index e7b493dfd..01ed990d0 100644 --- a/test/libsolidity/syntaxTests/inheritance/override/override_multiple_missing.sol +++ b/test/libsolidity/syntaxTests/inheritance/override/override_multiple_missing.sol @@ -1,5 +1,4 @@ abstract contract A { - int public testvar; function foo() internal virtual returns (uint256); function test(uint8 _a) virtual internal returns (uint256); } @@ -20,5 +19,5 @@ abstract contract X is A, B, C, D { function foo() internal override(A, C) virtual returns (uint256); } // ---- -// TypeError: (584-601): Invalid contract specified in override list: C. -// TypeError: (654-668): Function needs to specify overridden contracts B and D. +// TypeError: (563-580): Invalid contract specified in override list: C. +// TypeError: (633-647): Function needs to specify overridden contracts B and D. diff --git a/test/libsolidity/syntaxTests/inheritance/override/override_public_vars.sol b/test/libsolidity/syntaxTests/inheritance/override/override_public_vars.sol deleted file mode 100644 index 675a1cca0..000000000 --- a/test/libsolidity/syntaxTests/inheritance/override/override_public_vars.sol +++ /dev/null @@ -1,7 +0,0 @@ -abstract contract A { - int public testvar; -} -abstract contract X is A { - int public override testvar; -} -// ---- diff --git a/test/libsolidity/syntaxTests/inheritance/override/override_return_mismatch.sol b/test/libsolidity/syntaxTests/inheritance/override/override_return_mismatch.sol index e4aac5871..4a499d348 100644 --- a/test/libsolidity/syntaxTests/inheritance/override/override_return_mismatch.sol +++ b/test/libsolidity/syntaxTests/inheritance/override/override_return_mismatch.sol @@ -1,5 +1,4 @@ abstract contract A { - int public testvar; function foo() internal virtual returns (uint256); } abstract contract B { @@ -11,4 +10,4 @@ abstract contract X is A, B { function test() internal override virtual returns (uint256); } // ---- -// TypeError: (224-347): Derived contract must override function "foo". Function with the same name and parameter types defined in two or more base classes. +// TypeError: (203-326): Derived contract must override function "foo". Function with the same name and parameter types defined in two or more base classes. diff --git a/test/libsolidity/syntaxTests/inheritance/override/override_type_mismatch.sol b/test/libsolidity/syntaxTests/inheritance/override/override_type_mismatch.sol index ab9a4388b..5a79c098f 100644 --- a/test/libsolidity/syntaxTests/inheritance/override/override_type_mismatch.sol +++ b/test/libsolidity/syntaxTests/inheritance/override/override_type_mismatch.sol @@ -1,5 +1,4 @@ abstract contract A { - int public testvar; function foo() internal virtual returns (uint256); function test(uint8 _a) internal virtual returns (uint256); } @@ -22,5 +21,5 @@ abstract contract X is A, B, C, D { function foo() internal override(MyStruct, ENUM, A, B, C, D) virtual returns (uint256); } // ---- -// TypeError: (653-661): Expected contract but got struct X.MyStruct. -// TypeError: (663-667): Expected contract but got enum X.ENUM. +// TypeError: (632-640): Expected contract but got struct X.MyStruct. +// TypeError: (642-646): Expected contract but got enum X.ENUM. diff --git a/test/libsolidity/syntaxTests/inheritance/shadowing_base_state_vars.sol b/test/libsolidity/syntaxTests/inheritance/shadowing_base_state_vars.sol new file mode 100644 index 000000000..2e3a683b9 --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/shadowing_base_state_vars.sol @@ -0,0 +1,8 @@ +contract A { + uint i; +} +contract B is A { + uint i; +} +// ---- +// DeclarationError: (43-49): Identifier already declared. diff --git a/test/libsolidity/syntaxTests/inheritance/shadowing_private_base_state_vars.sol b/test/libsolidity/syntaxTests/inheritance/shadowing_private_base_state_vars.sol new file mode 100644 index 000000000..743aca86e --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/shadowing_private_base_state_vars.sol @@ -0,0 +1,6 @@ +contract A { + uint private i; +} +contract B is A { + uint i; +}