Check overrides and provide inherited public interface.

3 files changed, 43 insertions, 12 deletions

diff --git a/AST.cpp b/AST.cpp
index 7b633564..a529d6df 100644
--- a/AST.cpp
+++ b/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
diff --git a/AST.h b/AST.h
index b3fd840a..e8bc7f6a 100755
--- a/AST.h
+++ b/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
diff --git a/NameAndTypeResolver.cpp b/NameAndTypeResolver.cpp
index 2a73f21b..0b6afdd5 100644
--- a/NameAndTypeResolver.cpp
+++ b/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);
 	}
 }