Check for interface types of members and cache recursion check.

2 files changed, 48 insertions, 21 deletions

diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp
index c7843b90..83a5b465 100644
--- a/libsolidity/ast/Types.cpp
+++ b/libsolidity/ast/Types.cpp
@@ -1788,14 +1788,33 @@ MemberList::MemberMap StructType::nativeMembers(ContractDefinition const*) const
 
 TypePointer StructType::interfaceType(bool _inLibrary) const
 {
+	if (!canBeUsedExternally(_inLibrary))
+		return TypePointer();
+
+	// Has to fulfill canBeUsedExternally(_inLibrary) == !!interfaceType(_inLibrary)
 	if (_inLibrary && location() == DataLocation::Storage)
 		return shared_from_this();
-	else if (!recursive())
-		// TODO this might not be enough, we have to convert all members to
-		// their interfaceType
+	else
 		return copyForLocation(DataLocation::Memory, true);
+}
+
+bool StructType::canBeUsedExternally(bool _inLibrary) const
+{
+	if (_inLibrary && location() == DataLocation::Storage)
+		return true;
+	else if (recursive())
+		return false;
 	else
-		return TypePointer();
+	{
+		// Check that all members have interface types.
+		// We pass "false" to canBeUsedExternally (_inLibrary), because this struct will be
+		// passed by value and thus the encoding does not differ, but it will disallow
+		// mappings.
+		for (auto const& var: m_struct.members())
+			if (!var->annotation().type->canBeUsedExternally(false))
+				return false;
+	}
+	return true;
 }
 
 TypePointer StructType::copyForLocation(DataLocation _location, bool _isPointer) const
@@ -1887,25 +1906,29 @@ set<string> StructType::membersMissingInMemory() const
 
 bool StructType::recursive() const
 {
-	set<StructDefinition const*> structsSeen;
-	function<bool(StructType const*)> check = [&](StructType const* t) -> bool
+	if (!m_recursive.is_initialized())
 	{
-		StructDefinition const* str = &t->structDefinition();
-		if (structsSeen.count(str))
-			return true;
-		structsSeen.insert(str);
-		for (ASTPointer<VariableDeclaration> const& variable: str->members())
+		set<StructDefinition const*> structsSeen;
+		function<bool(StructType const*)> check = [&](StructType const* t) -> bool
 		{
-			Type const* memberType = variable->annotation().type.get();
-			while (dynamic_cast<ArrayType const*>(memberType))
-				memberType = dynamic_cast<ArrayType const*>(memberType)->baseType().get();
-			if (StructType const* innerStruct = dynamic_cast<StructType const*>(memberType))
-				if (check(innerStruct))
-					return true;
-		}
-		return false;
-	};
-	return check(this);
+			StructDefinition const* str = &t->structDefinition();
+			if (structsSeen.count(str))
+				return true;
+			structsSeen.insert(str);
+			for (ASTPointer<VariableDeclaration> const& variable: str->members())
+			{
+				Type const* memberType = variable->annotation().type.get();
+				while (dynamic_cast<ArrayType const*>(memberType))
+					memberType = dynamic_cast<ArrayType const*>(memberType)->baseType().get();
+				if (StructType const* innerStruct = dynamic_cast<StructType const*>(memberType))
+					if (check(innerStruct))
+						return true;
+			}
+			return false;
+		};
+		m_recursive = check(this);
+	}
+	return *m_recursive;
 }
 
 TypePointer EnumType::unaryOperatorResult(Token::Value _operator) const
diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h
index cb439693..8ba55521 100644
--- a/libsolidity/ast/Types.h
+++ b/libsolidity/ast/Types.h
@@ -32,6 +32,7 @@
 
 #include <boost/noncopyable.hpp>
 #include <boost/rational.hpp>
+#include <boost/optional.hpp>
 
 #include <memory>
 #include <string>
@@ -748,6 +749,7 @@ public:
 		return location() == DataLocation::Storage ? std::make_shared<IntegerType>(256) : shared_from_this();
 	}
 	virtual TypePointer interfaceType(bool _inLibrary) const override;
+	virtual bool canBeUsedExternally(bool _inLibrary) const override;
 
 	TypePointer copyForLocation(DataLocation _location, bool _isPointer) const override;
 
@@ -774,6 +776,8 @@ public:
 
 private:
 	StructDefinition const& m_struct;
+	/// Cache for the recursive() function.
+	mutable boost::optional<bool> m_recursive;
 };
 
 /**