Check overrides and provide inherited public interface.

libsolidity/AST.cpp

@@ -43,6 +43,8 @@ TypeError ASTNode::createTypeError(string const& _description) const
 
 void ContractDefinition::checkTypeRequirements()
 {
+	checkIllegalOverrides();
+
 	FunctionDefinition const* constructor = getConstructor();
 	if (constructor && !constructor->getReturnParameters().empty())
 		BOOST_THROW_EXCEPTION(constructor->getReturnParameterList()->createTypeError(
@@ -52,7 +54,6 @@ void ContractDefinition::checkTypeRequirements()
 		function->checkTypeRequirements();
 
 	// check for hash collisions in function signatures
-	vector<pair<FixedHash<4>, FunctionDefinition const*>> exportedFunctionList = getInterfaceFunctionList();
 	set<FixedHash<4>> hashes;
 	for (auto const& hashAndFunction: getInterfaceFunctionList())
 	{
@@ -83,17 +84,43 @@ FunctionDefinition const* ContractDefinition::getConstructor() const
 	return nullptr;
 }
 
-vector<pair<FixedHash<4>, FunctionDefinition const*>> ContractDefinition::getInterfaceFunctionList() const
+void ContractDefinition::checkIllegalOverrides() const
 {
-	vector<pair<FixedHash<4>, FunctionDefinition const*>> exportedFunctions;
-	for (ASTPointer<FunctionDefinition> const& f: m_definedFunctions)
-		if (f->isPublic() && f->getName() != getName())
+	map<string, FunctionDefinition const*> functions;
+
+	// We search from derived to base, so the stored item causes the error.
+	for (ContractDefinition const* contract: getLinearizedBaseContracts())
+		for (ASTPointer<FunctionDefinition> const& function: contract->getDefinedFunctions())
 		{
-			FixedHash<4> hash(dev::sha3(f->getCanonicalSignature()));
-			exportedFunctions.push_back(make_pair(hash, f.get()));
+			if (function->getName() == contract->getName())
+				continue; // constructors can neither be overriden nor override anything
+			FunctionDefinition const*& override = functions[function->getName()];
+			if (!override)
+				override = function.get();
+			else if (override->isPublic() != function->isPublic() ||
+					 override->isDeclaredConst() != function->isDeclaredConst() ||
+					 FunctionType(*override) != FunctionType(*function))
+				BOOST_THROW_EXCEPTION(override->createTypeError("Override changes extended function signature."));
 		}
+}
 
-	return exportedFunctions;
+vector<pair<FixedHash<4>, FunctionDefinition const*>> const& ContractDefinition::getInterfaceFunctionList() const
+{
+	if (!m_interfaceFunctionList)
+	{
+		set<string> functionsSeen;
+		m_interfaceFunctionList.reset(new vector<pair<FixedHash<4>, FunctionDefinition const*>>());
+		for (ContractDefinition const* contract: getLinearizedBaseContracts())
+			for (ASTPointer<FunctionDefinition> const& f: contract->getDefinedFunctions())
+				if (f->isPublic() && f->getName() != contract->getName() &&
+						functionsSeen.count(f->getName()) == 0)
+				{
+					functionsSeen.insert(f->getName());
+					FixedHash<4> hash(dev::sha3(f->getCanonicalSignature()));
+					m_interfaceFunctionList->push_back(make_pair(hash, f.get()));
+				}
+	}
+	return *m_interfaceFunctionList;
 }
 
 void StructDefinition::checkMemberTypes() const

libsolidity/AST.h

@@ -178,8 +178,8 @@ public:
 	std::vector<ASTPointer<VariableDeclaration>> const& getStateVariables() const { return m_stateVariables; }
 	std::vector<ASTPointer<FunctionDefinition>> const& getDefinedFunctions() const { return m_definedFunctions; }
 
-	/// Checks that the constructor does not have a "returns" statement and calls
-	/// checkTypeRequirements on all its functions.
+	/// Checks that there are no illegal overrides, that the constructor does not have a "returns"
+	/// and calls checkTypeRequirements on all its functions.
 	void checkTypeRequirements();
 
 	/// @return A shared pointer of an ASTString.
@@ -199,7 +199,9 @@ public:
 	FunctionDefinition const* getConstructor() const;
 
 private:
-	std::vector<std::pair<FixedHash<4>, FunctionDefinition const*>> getInterfaceFunctionList() const;
+	void checkIllegalOverrides() const;
+
+	std::vector<std::pair<FixedHash<4>, FunctionDefinition const*>> const& getInterfaceFunctionList() const;
 
 	std::vector<ASTPointer<Identifier>> m_baseContracts;
 	std::vector<ASTPointer<StructDefinition>> m_definedStructs;
@@ -208,6 +210,7 @@ private:
 	ASTPointer<ASTString> m_documentation;
 
 	std::vector<ContractDefinition const*> m_linearizedBaseContracts;
+	mutable std::unique_ptr<std::vector<std::pair<FixedHash<4>, FunctionDefinition const*>>> m_interfaceFunctionList;
 };
 
 class StructDefinition: public Declaration

libsolidity/NameAndTypeResolver.cpp

@@ -102,7 +102,8 @@ void NameAndTypeResolver::importInheritedScope(ContractDefinition const& _base)
 	for (auto const& nameAndDeclaration: iterator->second.getDeclarations())
 	{
 		Declaration const* declaration = nameAndDeclaration.second;
-		if (declaration->getScope() == &_base)
+		// Import if it was declared in the base and is not the constructor
+		if (declaration->getScope() == &_base && declaration->getName() != _base.getName())
 			m_currentScope->registerDeclaration(*declaration);
 	}
 }

test/SolidityNameAndTypeResolution.cpp

@@ -402,6 +402,55 @@ BOOST_AUTO_TEST_CASE(cyclic_inheritance)
 	BOOST_CHECK_THROW(parseTextAndResolveNames(text), TypeError);
 }
 
+BOOST_AUTO_TEST_CASE(illegal_override_direct)
+{
+	char const* text = R"(
+		contract B { function f() {} }
+		contract C is B { function f(uint i) {} }
+	)";
+	BOOST_CHECK_THROW(parseTextAndResolveNames(text), TypeError);
+}
+
+BOOST_AUTO_TEST_CASE(illegal_override_indirect)
+{
+	char const* text = R"(
+		contract A { function f(uint a) {} }
+		contract B { function f() {} }
+		contract C is A, B { }
+	)";
+	BOOST_CHECK_THROW(parseTextAndResolveNames(text), TypeError);
+}
+
+BOOST_AUTO_TEST_CASE(complex_inheritance)
+{
+	char const* text = R"(
+		contract A { function f() { uint8 x = C(0).g(); } }
+		contract B { function f() {} function g() returns (uint8 r) {} }
+		contract C is A, B { }
+	)";
+	BOOST_CHECK_NO_THROW(parseTextAndResolveNames(text));
+}
+
+BOOST_AUTO_TEST_CASE(constructor_visibility)
+{
+	// The constructor of a base class should not be visible in the derived class
+	char const* text = R"(
+		contract A { function A() { } }
+		contract B is A { function f() { A x = A(0); } }
+	)";
+	BOOST_CHECK_NO_THROW(parseTextAndResolveNames(text));
+}
+
+BOOST_AUTO_TEST_CASE(overriding_constructor)
+{
+	// It is fine to "override" constructor of a base class since it is invisible
+	char const* text = R"(
+		contract A { function A() { } }
+		contract B is A { function A() returns (uint8 r) {} }
+	)";
+	BOOST_CHECK_NO_THROW(parseTextAndResolveNames(text));
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }