Merge pull request #1146 from LefterisJP/sol_fixBaseClassAccessors

Sol fix accessing public variable of base class

libsolidity/AST.cpp

@@ -209,6 +209,33 @@ vector<pair<FixedHash<4>, FunctionTypePointer>> const& ContractDefinition::getIn
 	return *m_interfaceFunctionList;
 }
 
+vector<Declaration const*> const& ContractDefinition::getInheritableMembers() const
+{
+	if (!m_inheritableMembers)
+	{
+		set<string> memberSeen;
+		m_inheritableMembers.reset(new vector<Declaration const*>());
+		auto addInheritableMember = [&](Declaration const* _decl)
+		{
+			if (memberSeen.count(_decl->getName()) == 0 && _decl->isVisibleInDerivedContracts())
+			{
+				memberSeen.insert(_decl->getName());
+				m_inheritableMembers->push_back(_decl);
+			}
+		};
+
+		for (ASTPointer<FunctionDefinition> const& f: getDefinedFunctions())
+			addInheritableMember(f.get());
+
+		for (ASTPointer<VariableDeclaration> const& v: getStateVariables())
+			addInheritableMember(v.get());
+
+		for (ASTPointer<StructDefinition> const& s: getDefinedStructs())
+			addInheritableMember(s.get());
+	}
+	return *m_inheritableMembers;
+}
+
 TypePointer EnumValue::getType(ContractDefinition const*) const
 {
 	EnumDefinition const* parentDef = dynamic_cast<EnumDefinition const*>(getScope());

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.
@@ -247,6 +247,9 @@ public:
 	/// as intended for use by the ABI.
 	std::map<FixedHash<4>, FunctionTypePointer> getInterfaceFunctions() const;
 
+	/// @returns a list of the inheritable members of this contract
+	std::vector<Declaration const*> 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<ContractDefinition const*> const& getLinearizedBaseContracts() const { return m_linearizedBaseContracts; }
@@ -273,6 +276,7 @@ private:
 	std::vector<ContractDefinition const*> m_linearizedBaseContracts;
 	mutable std::unique_ptr<std::vector<std::pair<FixedHash<4>, FunctionTypePointer>>> m_interfaceFunctionList;
 	mutable std::unique_ptr<std::vector<ASTPointer<EventDefinition>>> m_interfaceEvents;
+	mutable std::unique_ptr<std::vector<Declaration const*>> m_inheritableMembers;
 };
 
 class InheritanceSpecifier: public ASTNode
@@ -405,6 +409,11 @@ public:
 	ASTPointer<ParameterList> 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.

libsolidity/Types.cpp

@@ -643,8 +643,7 @@ MemberList const& ContractType::getMembers() const
 		{
 			for (ContractDefinition const* base: m_contract.getLinearizedBaseContracts())
 				for (ASTPointer<FunctionDefinition> const& function: base->getDefinedFunctions())
-					if (!function->isConstructor() && !function->getName().empty()&&
+					if (function->isVisibleInDerivedContracts())
-							function->isVisibleInDerivedContracts())
 						members.push_back(make_pair(function->getName(), make_shared<FunctionType>(*function, true)));
 		}
 		else
@@ -1039,10 +1038,9 @@ MemberList const& TypeType::getMembers() const
 			vector<ContractDefinition const*> 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<FunctionDefinition> const& f: contract.getDefinedFunctions())
+				for (Declaration const* decl: contract.getInheritableMembers())
-					if (!f->isConstructor() && !f->getName().empty() && f->isVisibleInDerivedContracts())
+					members.push_back(make_pair(decl->getName(), decl->getType()));
-						members.push_back(make_pair(f->getName(), make_shared<FunctionType>(*f)));
 		}
 		else if (m_actualType->getCategory() == Category::Enum)
 		{

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)
@@ -720,6 +720,58 @@ 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 is Parent{\n"
+					   "    function foo() returns (uint256) { return Parent.m_aMember; }\n"
+					   "}\n";
+	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(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"(