From 6d48abddd36fa28aa47e46f46bc725efd4077f2a Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Wed, 25 Feb 2015 16:32:04 +0100 Subject: [PATCH 1/7] Adding test for base class statevar accessors --- test/SolidityNameAndTypeResolution.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/SolidityNameAndTypeResolution.cpp b/test/SolidityNameAndTypeResolution.cpp index 4809aac1e..e4764c5ef 100644 --- a/test/SolidityNameAndTypeResolution.cpp +++ b/test/SolidityNameAndTypeResolution.cpp @@ -720,6 +720,18 @@ BOOST_AUTO_TEST_CASE(private_state_variable) BOOST_CHECK_MESSAGE(function == nullptr, "Accessor function of an internal variable should not exist"); } +BOOST_AUTO_TEST_CASE(base_class_state_variable_accessor) +{ + // test for issue #1126 https://github.com/ethereum/cpp-ethereum/issues/1126 + char const* text = "contract Parent {\n" + " uint256 public m_aMember;\n" + "}\n" + "contract Child {\n" + " function foo() returns (uint256) { return Parent.m_aMember(); }\n" + "}\n"; + BOOST_CHECK_NO_THROW(parseTextAndResolveNamesWithChecks(text)); +} + BOOST_AUTO_TEST_CASE(fallback_function) { char const* text = R"( From 345e84baeaccaa810f163fe7e62c33e8d0fe749c Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Wed, 25 Feb 2015 20:35:55 +0100 Subject: [PATCH 2/7] Adding inheritable members to a contract --- libsolidity/AST.cpp | 27 +++++++++++++++++++++++++++ libsolidity/AST.h | 4 ++++ libsolidity/Types.cpp | 5 ++--- 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index c37e8c374..b61eb011e 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -209,6 +209,33 @@ vector, FunctionTypePointer>> const& ContractDefinition::getIn return *m_interfaceFunctionList; } +vector> const& ContractDefinition::getInheritableMembers() const +{ + if (!m_inheritableMembers) + { + set memberSeen; + m_inheritableMembers.reset(new vector>()); + for (ContractDefinition const* contract: getLinearizedBaseContracts()) + { + for (ASTPointer const& f: contract->getDefinedFunctions()) + if (f->isPublic() && !f->isConstructor() && !f->getName().empty() + && memberSeen.count(f->getName()) == 0 && f->isVisibleInDerivedContracts()) + { + memberSeen.insert(f->getName()); + m_inheritableMembers->push_back(f); + } + + for (ASTPointer const& v: contract->getStateVariables()) + if (v->isPublic() && memberSeen.count(v->getName()) == 0) + { + memberSeen.insert(v->getName()); + m_inheritableMembers->push_back(v); + } + } + } + return *m_inheritableMembers; +} + TypePointer EnumValue::getType(ContractDefinition const*) const { EnumDefinition const* parentDef = dynamic_cast(getScope()); diff --git a/libsolidity/AST.h b/libsolidity/AST.h index dea0fba63..dd40e0239 100644 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -247,6 +247,9 @@ public: /// as intended for use by the ABI. std::map, FunctionTypePointer> getInterfaceFunctions() const; + /// @returns a list of the inheritable members of this contract + std::vector> const& getInheritableMembers() const; + /// List of all (direct and indirect) base contracts in order from derived to base, including /// the contract itself. Available after name resolution std::vector const& getLinearizedBaseContracts() const { return m_linearizedBaseContracts; } @@ -273,6 +276,7 @@ private: std::vector m_linearizedBaseContracts; mutable std::unique_ptr, FunctionTypePointer>>> m_interfaceFunctionList; mutable std::unique_ptr>> m_interfaceEvents; + mutable std::unique_ptr>> m_inheritableMembers; }; class InheritanceSpecifier: public ASTNode diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index adcd2e542..1347c9ea8 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -1025,9 +1025,8 @@ MemberList const& TypeType::getMembers() const if (find(currentBases.begin(), currentBases.end(), &contract) != currentBases.end()) // We are accessing the type of a base contract, so add all public and protected // functions. Note that this does not add inherited functions on purpose. - for (ASTPointer const& f: contract.getDefinedFunctions()) - if (!f->isConstructor() && !f->getName().empty() && f->isVisibleInDerivedContracts()) - members.push_back(make_pair(f->getName(), make_shared(*f))); + for (ASTPointer const& decl: contract.getInheritableMembers()) + members.push_back(make_pair(decl->getName(), decl->getType())); } else if (m_actualType->getCategory() == Category::Enum) { From f2fdeb3599d3f885d046bc94f4ebb74c4d1258bf Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Thu, 26 Feb 2015 12:11:54 +0100 Subject: [PATCH 3/7] Add structs to inheritable members --- libsolidity/AST.cpp | 7 +++++++ libsolidity/Types.cpp | 2 +- test/SolidityNameAndTypeResolution.cpp | 4 ++-- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index b61eb011e..8c1944f96 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -231,6 +231,13 @@ vector> const& ContractDefinition::getInheritableMembers memberSeen.insert(v->getName()); m_inheritableMembers->push_back(v); } + + for (ASTPointer const& s: contract->getDefinedStructs()) + if (s->isPublic() && memberSeen.count(s->getName()) == 0) + { + memberSeen.insert(s->getName()); + m_inheritableMembers->push_back(s); + } } } return *m_inheritableMembers; diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index 1347c9ea8..73367c85d 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -1024,7 +1024,7 @@ MemberList const& TypeType::getMembers() const vector currentBases = m_currentContract->getLinearizedBaseContracts(); if (find(currentBases.begin(), currentBases.end(), &contract) != currentBases.end()) // We are accessing the type of a base contract, so add all public and protected - // functions. Note that this does not add inherited functions on purpose. + // members. Note that this does not add inherited functions on purpose. for (ASTPointer const& decl: contract.getInheritableMembers()) members.push_back(make_pair(decl->getName(), decl->getType())); } diff --git a/test/SolidityNameAndTypeResolution.cpp b/test/SolidityNameAndTypeResolution.cpp index e4764c5ef..9db45b911 100644 --- a/test/SolidityNameAndTypeResolution.cpp +++ b/test/SolidityNameAndTypeResolution.cpp @@ -726,8 +726,8 @@ BOOST_AUTO_TEST_CASE(base_class_state_variable_accessor) char const* text = "contract Parent {\n" " uint256 public m_aMember;\n" "}\n" - "contract Child {\n" - " function foo() returns (uint256) { return Parent.m_aMember(); }\n" + "contract Child is Parent{\n" + " function foo() returns (uint256) { return Parent.m_aMember; }\n" "}\n"; BOOST_CHECK_NO_THROW(parseTextAndResolveNamesWithChecks(text)); } From b1dcc2a77ff7268c8a6a0b4f1eeadb509c6a499e Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Fri, 27 Feb 2015 10:08:14 +0100 Subject: [PATCH 4/7] VisibleInDerivedContracts() is now virtual() - Plus an extra test for internal visibility in a base class variable --- libsolidity/AST.cpp | 7 +++---- libsolidity/AST.h | 7 ++++++- libsolidity/Types.cpp | 3 +-- test/SolidityNameAndTypeResolution.cpp | 13 ++++++++++++- 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 8c1944f96..d18a296d5 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -218,22 +218,21 @@ vector> const& ContractDefinition::getInheritableMembers for (ContractDefinition const* contract: getLinearizedBaseContracts()) { for (ASTPointer const& f: contract->getDefinedFunctions()) - if (f->isPublic() && !f->isConstructor() && !f->getName().empty() - && memberSeen.count(f->getName()) == 0 && f->isVisibleInDerivedContracts()) + if (memberSeen.count(f->getName()) == 0 && f->isVisibleInDerivedContracts()) { memberSeen.insert(f->getName()); m_inheritableMembers->push_back(f); } for (ASTPointer const& v: contract->getStateVariables()) - if (v->isPublic() && memberSeen.count(v->getName()) == 0) + if (memberSeen.count(v->getName()) == 0 && v->isVisibleInDerivedContracts()) { memberSeen.insert(v->getName()); m_inheritableMembers->push_back(v); } for (ASTPointer const& s: contract->getDefinedStructs()) - if (s->isPublic() && memberSeen.count(s->getName()) == 0) + if (memberSeen.count(s->getName()) == 0 && s->isVisibleInDerivedContracts()) { memberSeen.insert(s->getName()); m_inheritableMembers->push_back(s); diff --git a/libsolidity/AST.h b/libsolidity/AST.h index dd40e0239..97e85927b 100644 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -144,7 +144,7 @@ public: Visibility getVisibility() const { return m_visibility == Visibility::Default ? getDefaultVisibility() : m_visibility; } bool isPublic() const { return getVisibility() >= Visibility::Public; } bool isVisibleInContract() const { return getVisibility() != Visibility::External; } - bool isVisibleInDerivedContracts() const { return isVisibleInContract() && getVisibility() >= Visibility::Internal; } + virtual bool isVisibleInDerivedContracts() const { return isVisibleInContract() && getVisibility() >= Visibility::Internal; } /// @returns the scope this declaration resides in. Can be nullptr if it is the global scope. /// Available only after name and type resolution step. @@ -409,6 +409,11 @@ public: ASTPointer const& getReturnParameterList() const { return m_returnParameters; } Block const& getBody() const { return *m_body; } + virtual bool isVisibleInDerivedContracts() const override + { + return !isConstructor() && !getName().empty() && isVisibleInContract() && + getVisibility() >= Visibility::Internal; + } virtual TypePointer getType(ContractDefinition const*) const override; /// Checks that all parameters have allowed types and calls checkTypeRequirements on the body. diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index 73367c85d..84e029269 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -628,8 +628,7 @@ MemberList const& ContractType::getMembers() const { for (ContractDefinition const* base: m_contract.getLinearizedBaseContracts()) for (ASTPointer const& function: base->getDefinedFunctions()) - if (!function->isConstructor() && !function->getName().empty()&& - function->isVisibleInDerivedContracts()) + if (function->isVisibleInDerivedContracts()) members.push_back(make_pair(function->getName(), make_shared(*function, true))); } else diff --git a/test/SolidityNameAndTypeResolution.cpp b/test/SolidityNameAndTypeResolution.cpp index 9db45b911..58cebaebb 100644 --- a/test/SolidityNameAndTypeResolution.cpp +++ b/test/SolidityNameAndTypeResolution.cpp @@ -436,7 +436,7 @@ BOOST_AUTO_TEST_CASE(inheritance_diamond_basic) function g() { f(); rootFunction(); } } )"; - BOOST_CHECK_NO_THROW(parseTextAndResolveNames(text)); + BOOST_CHECK_NO_THROW(parseTextAndResolveNamesWithChecks(text)); } BOOST_AUTO_TEST_CASE(cyclic_inheritance) @@ -732,6 +732,17 @@ BOOST_AUTO_TEST_CASE(base_class_state_variable_accessor) BOOST_CHECK_NO_THROW(parseTextAndResolveNamesWithChecks(text)); } +BOOST_AUTO_TEST_CASE(base_class_state_variable_internal_member) +{ + char const* text = "contract Parent {\n" + " uint256 internal m_aMember;\n" + "}\n" + "contract Child is Parent{\n" + " function foo() returns (uint256) { return Parent.m_aMember; }\n" + "}\n"; + BOOST_CHECK_NO_THROW(parseTextAndResolveNamesWithChecks(text)); +} + BOOST_AUTO_TEST_CASE(fallback_function) { char const* text = R"( From 58005fd2034c0b63cb170f7f9a5140dff1825027 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Fri, 27 Feb 2015 11:35:25 +0100 Subject: [PATCH 5/7] Use lambda to avoid code duplication in inheritableMembers --- libsolidity/AST.cpp | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index d18a296d5..24a94a798 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -217,26 +217,23 @@ vector> const& ContractDefinition::getInheritableMembers m_inheritableMembers.reset(new vector>()); for (ContractDefinition const* contract: getLinearizedBaseContracts()) { - for (ASTPointer const& f: contract->getDefinedFunctions()) - if (memberSeen.count(f->getName()) == 0 && f->isVisibleInDerivedContracts()) + auto addInheritableMember = [&](ASTPointer const& _decl) + { + if (memberSeen.count(_decl->getName()) == 0 && _decl->isVisibleInDerivedContracts()) { - memberSeen.insert(f->getName()); - m_inheritableMembers->push_back(f); + memberSeen.insert(_decl->getName()); + m_inheritableMembers->push_back(_decl); } + }; + + for (ASTPointer const& f: contract->getDefinedFunctions()) + addInheritableMember(f); for (ASTPointer const& v: contract->getStateVariables()) - if (memberSeen.count(v->getName()) == 0 && v->isVisibleInDerivedContracts()) - { - memberSeen.insert(v->getName()); - m_inheritableMembers->push_back(v); - } + addInheritableMember(v); for (ASTPointer const& s: contract->getDefinedStructs()) - if (memberSeen.count(s->getName()) == 0 && s->isVisibleInDerivedContracts()) - { - memberSeen.insert(s->getName()); - m_inheritableMembers->push_back(s); - } + addInheritableMember(s); } } return *m_inheritableMembers; From 1681fbd8858f8e8b67c7a1413b2d85d1cccb4779 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Sat, 28 Feb 2015 18:30:33 +0100 Subject: [PATCH 6/7] getInheritableMembers() does not look at BaseContracts - Also adding tests for improper accessing members of other contracts. --- libsolidity/AST.cpp | 27 +++++++++++------------- test/SolidityNameAndTypeResolution.cpp | 29 ++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 15 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 24a94a798..e5aef4cbb 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -215,26 +215,23 @@ vector> const& ContractDefinition::getInheritableMembers { set memberSeen; m_inheritableMembers.reset(new vector>()); - for (ContractDefinition const* contract: getLinearizedBaseContracts()) + auto addInheritableMember = [&](ASTPointer const& _decl) { - auto addInheritableMember = [&](ASTPointer const& _decl) + if (memberSeen.count(_decl->getName()) == 0 && _decl->isVisibleInDerivedContracts()) { - if (memberSeen.count(_decl->getName()) == 0 && _decl->isVisibleInDerivedContracts()) - { - memberSeen.insert(_decl->getName()); - m_inheritableMembers->push_back(_decl); - } - }; + memberSeen.insert(_decl->getName()); + m_inheritableMembers->push_back(_decl); + } + }; - for (ASTPointer const& f: contract->getDefinedFunctions()) - addInheritableMember(f); + for (ASTPointer const& f: getDefinedFunctions()) + addInheritableMember(f); - for (ASTPointer const& v: contract->getStateVariables()) - addInheritableMember(v); + for (ASTPointer const& v: getStateVariables()) + addInheritableMember(v); - for (ASTPointer const& s: contract->getDefinedStructs()) - addInheritableMember(s); - } + for (ASTPointer const& s: getDefinedStructs()) + addInheritableMember(s); } return *m_inheritableMembers; } diff --git a/test/SolidityNameAndTypeResolution.cpp b/test/SolidityNameAndTypeResolution.cpp index 58cebaebb..81d58d0dd 100644 --- a/test/SolidityNameAndTypeResolution.cpp +++ b/test/SolidityNameAndTypeResolution.cpp @@ -743,6 +743,35 @@ BOOST_AUTO_TEST_CASE(base_class_state_variable_internal_member) BOOST_CHECK_NO_THROW(parseTextAndResolveNamesWithChecks(text)); } +BOOST_AUTO_TEST_CASE(state_variable_member_of_wrong_class1) +{ + char const* text = "contract Parent1 {\n" + " uint256 internal m_aMember1;\n" + "}\n" + "contract Parent2 is Parent1{\n" + " uint256 internal m_aMember2;\n" + "}\n" + "contract Child is Parent2{\n" + " function foo() returns (uint256) { return Parent2.m_aMember1; }\n" + "}\n"; + BOOST_CHECK_THROW(parseTextAndResolveNames(text), TypeError); +} + +BOOST_AUTO_TEST_CASE(state_variable_member_of_wrong_class2) +{ + char const* text = "contract Parent1 {\n" + " uint256 internal m_aMember1;\n" + "}\n" + "contract Parent2 is Parent1{\n" + " uint256 internal m_aMember2;\n" + "}\n" + "contract Child is Parent2{\n" + " function foo() returns (uint256) { return Child.m_aMember2; }\n" + " uint256 public m_aMember3;\n" + "}\n"; + BOOST_CHECK_THROW(parseTextAndResolveNames(text), TypeError); +} + BOOST_AUTO_TEST_CASE(fallback_function) { char const* text = R"( From de6e9f4f54f669369a521e45cb2d6d9af3dbadd1 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Mon, 2 Mar 2015 12:08:32 +0100 Subject: [PATCH 7/7] Using normal pointer in getInheritableMembers() --- libsolidity/AST.cpp | 12 ++++++------ libsolidity/AST.h | 4 ++-- libsolidity/Types.cpp | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index e5aef4cbb..4c6db6a20 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -209,13 +209,13 @@ vector, FunctionTypePointer>> const& ContractDefinition::getIn return *m_interfaceFunctionList; } -vector> const& ContractDefinition::getInheritableMembers() const +vector const& ContractDefinition::getInheritableMembers() const { if (!m_inheritableMembers) { set memberSeen; - m_inheritableMembers.reset(new vector>()); - auto addInheritableMember = [&](ASTPointer const& _decl) + m_inheritableMembers.reset(new vector()); + auto addInheritableMember = [&](Declaration const* _decl) { if (memberSeen.count(_decl->getName()) == 0 && _decl->isVisibleInDerivedContracts()) { @@ -225,13 +225,13 @@ vector> const& ContractDefinition::getInheritableMembers }; for (ASTPointer const& f: getDefinedFunctions()) - addInheritableMember(f); + addInheritableMember(f.get()); for (ASTPointer const& v: getStateVariables()) - addInheritableMember(v); + addInheritableMember(v.get()); for (ASTPointer const& s: getDefinedStructs()) - addInheritableMember(s); + addInheritableMember(s.get()); } return *m_inheritableMembers; } diff --git a/libsolidity/AST.h b/libsolidity/AST.h index 97e85927b..d028e8a76 100644 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -248,7 +248,7 @@ public: std::map, FunctionTypePointer> getInterfaceFunctions() const; /// @returns a list of the inheritable members of this contract - std::vector> const& getInheritableMembers() const; + std::vector const& getInheritableMembers() const; /// List of all (direct and indirect) base contracts in order from derived to base, including /// the contract itself. Available after name resolution @@ -276,7 +276,7 @@ private: std::vector m_linearizedBaseContracts; mutable std::unique_ptr, FunctionTypePointer>>> m_interfaceFunctionList; mutable std::unique_ptr>> m_interfaceEvents; - mutable std::unique_ptr>> m_inheritableMembers; + mutable std::unique_ptr> m_inheritableMembers; }; class InheritanceSpecifier: public ASTNode diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index 84e029269..dc72dcc49 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -1024,7 +1024,7 @@ MemberList const& TypeType::getMembers() const if (find(currentBases.begin(), currentBases.end(), &contract) != currentBases.end()) // We are accessing the type of a base contract, so add all public and protected // members. Note that this does not add inherited functions on purpose. - for (ASTPointer const& decl: contract.getInheritableMembers()) + for (Declaration const* decl: contract.getInheritableMembers()) members.push_back(make_pair(decl->getName(), decl->getType())); } else if (m_actualType->getCategory() == Category::Enum)