Merge pull request #8665 from ethereum/recursiveStructRefactoring

Recursive struct refactoring and a bunch of ICE fixes.

libsolidity/analysis/DeclarationTypeChecker.cpp

@@ -23,6 +23,8 @@
 
 #include <liblangutil/ErrorReporter.h>
 
+#include <libsolutil/Algorithms.h>
+
 #include <boost/range/adaptor/transformed.hpp>
 
 using namespace std;
@@ -31,8 +33,9 @@ using namespace solidity::frontend;
 
 bool DeclarationTypeChecker::visit(ElementaryTypeName const& _typeName)
 {
-	if (!_typeName.annotation().type)
-	{
+	if (_typeName.annotation().type)
+		return false;
+
 	_typeName.annotation().type = TypeProvider::fromElementaryTypeName(_typeName.typeName());
 	if (_typeName.stateMutability().has_value())
 	{
@@ -54,17 +57,87 @@ bool DeclarationTypeChecker::visit(ElementaryTypeName const& _typeName)
 				break;
 		}
 	}
-	}
 	return true;
 }
 
+bool DeclarationTypeChecker::visit(StructDefinition const& _struct)
+{
+	if (_struct.annotation().recursive.has_value())
+	{
+		if (!m_currentStructsSeen.empty() && *_struct.annotation().recursive)
+			m_recursiveStructSeen = true;
+		return false;
+	}
+
+	if (m_currentStructsSeen.count(&_struct))
+	{
+		_struct.annotation().recursive = true;
+		m_recursiveStructSeen = true;
+		return false;
+	}
+
+	bool previousRecursiveStructSeen = m_recursiveStructSeen;
+	bool hasRecursiveChild = false;
+
+	m_currentStructsSeen.insert(&_struct);
+
+	for (auto const& member: _struct.members())
+	{
+		m_recursiveStructSeen = false;
+		member->accept(*this);
+		solAssert(member->annotation().type, "");
+		solAssert(member->annotation().type->canBeStored(), "Type cannot be used in struct.");
+		if (m_recursiveStructSeen)
+			hasRecursiveChild = true;
+	}
+
+	if (!_struct.annotation().recursive.has_value())
+		_struct.annotation().recursive = hasRecursiveChild;
+	m_recursiveStructSeen = previousRecursiveStructSeen || *_struct.annotation().recursive;
+	m_currentStructsSeen.erase(&_struct);
+	if (m_currentStructsSeen.empty())
+		m_recursiveStructSeen = false;
+
+	// Check direct recursion, fatal error if detected.
+	auto visitor = [&](StructDefinition const& _struct, auto& _cycleDetector, size_t _depth)
+	{
+		if (_depth >= 256)
+			fatalDeclarationError(_struct.location(), "Struct definition exhausts cyclic dependency validator.");
+
+		for (ASTPointer<VariableDeclaration> const& member: _struct.members())
+		{
+			Type const* memberType = member->annotation().type;
+			while (auto arrayType = dynamic_cast<ArrayType const*>(memberType))
+			{
+				if (arrayType->isDynamicallySized())
+					break;
+				memberType = arrayType->baseType();
+			}
+			if (auto structType = dynamic_cast<StructType const*>(memberType))
+				if (_cycleDetector.run(structType->structDefinition()))
+					return;
+		}
+	};
+	if (util::CycleDetector<StructDefinition>(visitor).run(_struct) != nullptr)
+		fatalTypeError(_struct.location(), "Recursive struct definition.");
+
+	return false;
+}
+
 void DeclarationTypeChecker::endVisit(UserDefinedTypeName const& _typeName)
 {
+	if (_typeName.annotation().type)
+		return;
+
 	Declaration const* declaration = _typeName.annotation().referencedDeclaration;
 	solAssert(declaration, "");
 
 	if (StructDefinition const* structDef = dynamic_cast<StructDefinition const*>(declaration))
+	{
+		if (!m_insideFunctionType && !m_currentStructsSeen.empty())
+			structDef->accept(*this);
 		_typeName.annotation().type = TypeProvider::structType(*structDef, DataLocation::Storage);
+	}
 	else if (EnumDefinition const* enumDef = dynamic_cast<EnumDefinition const*>(declaration))
 		_typeName.annotation().type = TypeProvider::enumType(*enumDef);
 	else if (ContractDefinition const* contract = dynamic_cast<ContractDefinition const*>(declaration))
@@ -75,8 +148,17 @@ void DeclarationTypeChecker::endVisit(UserDefinedTypeName const& _typeName)
 		fatalTypeError(_typeName.location(), "Name has to refer to a struct, enum or contract.");
 	}
 }
-void DeclarationTypeChecker::endVisit(FunctionTypeName const& _typeName)
+bool DeclarationTypeChecker::visit(FunctionTypeName const& _typeName)
 {
+	if (_typeName.annotation().type)
+		return false;
+
+	bool previousInsideFunctionType = m_insideFunctionType;
+	m_insideFunctionType = true;
+	_typeName.parameterTypeList()->accept(*this);
+	_typeName.returnParameterTypeList()->accept(*this);
+	m_insideFunctionType = previousInsideFunctionType;
+
 	switch (_typeName.visibility())
 	{
 		case Visibility::Internal:
@@ -84,30 +166,22 @@ void DeclarationTypeChecker::endVisit(FunctionTypeName const& _typeName)
 			break;
 		default:
 			fatalTypeError(_typeName.location(), "Invalid visibility, can only be \"external\" or \"internal\".");
-			return;
+			return false;
 	}
 
 	if (_typeName.isPayable() && _typeName.visibility() != Visibility::External)
 	{
 		fatalTypeError(_typeName.location(), "Only external function types can be payable.");
-		return;
+		return false;
 	}
-
-	if (_typeName.visibility() == Visibility::External)
-		for (auto const& t: _typeName.parameterTypes() + _typeName.returnParameterTypes())
-		{
-			solAssert(t->annotation().type, "Type not set for parameter.");
-			if (!t->annotation().type->interfaceType(false).get())
-			{
-				fatalTypeError(t->location(), "Internal type cannot be used for external function type.");
-				return;
-			}
-		}
-
 	_typeName.annotation().type = TypeProvider::function(_typeName);
+	return false;
 }
 void DeclarationTypeChecker::endVisit(Mapping const& _mapping)
 {
+	if (_mapping.annotation().type)
+		return;
+
 	if (auto const* typeName = dynamic_cast<UserDefinedTypeName const*>(&_mapping.keyType()))
 	{
 		if (auto const* contractType = dynamic_cast<ContractType const*>(typeName->annotation().type))
@@ -140,6 +214,9 @@ void DeclarationTypeChecker::endVisit(Mapping const& _mapping)
 
 void DeclarationTypeChecker::endVisit(ArrayTypeName const& _typeName)
 {
+	if (_typeName.annotation().type)
+		return;
+
 	TypePointer baseType = _typeName.baseType().annotation().type;
 	if (!baseType)
 	{
@@ -292,6 +369,12 @@ void DeclarationTypeChecker::fatalTypeError(SourceLocation const& _location, str
 	m_errorReporter.fatalTypeError(_location, _description);
 }
 
+void DeclarationTypeChecker::fatalDeclarationError(SourceLocation const& _location, string const& _description)
+{
+	m_errorOccurred = true;
+	m_errorReporter.fatalDeclarationError(_location, _description);
+}
+
 bool DeclarationTypeChecker::check(ASTNode const& _node)
 {
 	_node.accept(*this);

libsolidity/analysis/DeclarationTypeChecker.h

@@ -53,10 +53,11 @@ private:
 
 	bool visit(ElementaryTypeName const& _typeName) override;
 	void endVisit(UserDefinedTypeName const& _typeName) override;
-	void endVisit(FunctionTypeName const& _typeName) override;
+	bool visit(FunctionTypeName const& _typeName) override;
 	void endVisit(Mapping const& _mapping) override;
 	void endVisit(ArrayTypeName const& _typeName) override;
 	void endVisit(VariableDeclaration const& _variable) override;
+	bool visit(StructDefinition const& _struct) override;
 
 	/// Adds a new error to the list of errors.
 	void typeError(langutil::SourceLocation const& _location, std::string const& _description);
@@ -64,9 +65,15 @@ private:
 	/// Adds a new error to the list of errors and throws to abort reference resolving.
 	void fatalTypeError(langutil::SourceLocation const& _location, std::string const& _description);
 
+	/// Adds a new error to the list of errors and throws to abort reference resolving.
+	void fatalDeclarationError(langutil::SourceLocation const& _location, std::string const& _description);
+
 	langutil::ErrorReporter& m_errorReporter;
 	bool m_errorOccurred = false;
 	langutil::EVMVersion m_evmVersion;
+	bool m_insideFunctionType = false;
+	bool m_recursiveStructSeen = false;
+	std::set<StructDefinition const*> m_currentStructsSeen;
 };
 
 }

libsolidity/analysis/TypeChecker.cpp

@@ -289,39 +289,6 @@ void TypeChecker::endVisit(UsingForDirective const& _usingFor)
 		m_errorReporter.fatalTypeError(_usingFor.libraryName().location(), "Library name expected.");
 }
 
-bool TypeChecker::visit(StructDefinition const& _struct)
-{
-	for (ASTPointer<VariableDeclaration> const& member: _struct.members())
-		solAssert(type(*member)->canBeStored(), "Type cannot be used in struct.");
-
-	// Check recursion, fatal error if detected.
-	auto visitor = [&](StructDefinition const& _struct, CycleDetector<StructDefinition>& _cycleDetector, size_t _depth)
-	{
-		if (_depth >= 256)
-			m_errorReporter.fatalDeclarationError(_struct.location(), "Struct definition exhausting cyclic dependency validator.");
-
-		for (ASTPointer<VariableDeclaration> const& member: _struct.members())
-		{
-			Type const* memberType = type(*member);
-			while (auto arrayType = dynamic_cast<ArrayType const*>(memberType))
-			{
-				if (arrayType->isDynamicallySized())
-					break;
-				memberType = arrayType->baseType();
-			}
-			if (auto structType = dynamic_cast<StructType const*>(memberType))
-				if (_cycleDetector.run(structType->structDefinition()))
-					return;
-		}
-	};
-	if (CycleDetector<StructDefinition>(visitor).run(_struct) != nullptr)
-		m_errorReporter.fatalTypeError(_struct.location(), "Recursive struct definition.");
-
-	ASTNode::listAccept(_struct.members(), *this);
-
-	return false;
-}
-
 bool TypeChecker::visit(FunctionDefinition const& _function)
 {
 	bool isLibraryFunction = _function.inContractKind() == ContractKind::Library;
@@ -518,19 +485,16 @@ bool TypeChecker::visit(VariableDeclaration const& _variable)
 			m_errorReporter.typeError(_variable.location(), "Internal or recursive type is not allowed for public state variables.");
 	}
 
-	switch (varType->category())
+	if (auto referenceType = dynamic_cast<ReferenceType const*>(varType))
 	{
-	case Type::Category::Array:
-		if (auto arrayType = dynamic_cast<ArrayType const*>(varType))
-			if (
-				((arrayType->location() == DataLocation::Memory) ||
-				(arrayType->location() == DataLocation::CallData)) &&
-				!arrayType->validForCalldata()
-			)
-				m_errorReporter.typeError(_variable.location(), "Array is too large to be encoded.");
-		break;
-	default:
-		break;
+		auto result = referenceType->validForLocation(referenceType->location());
+		if (result && _variable.isPublicCallableParameter())
+			result = referenceType->validForLocation(DataLocation::CallData);
+		if (!result)
+		{
+			solAssert(!result.message().empty(), "Expected detailed error message");
+			m_errorReporter.typeError(_variable.location(), result.message());
+		}
 	}
 
 	return false;
@@ -631,8 +595,16 @@ void TypeChecker::endVisit(FunctionTypeName const& _funType)
 {
 	FunctionType const& fun = dynamic_cast<FunctionType const&>(*_funType.annotation().type);
 	if (fun.kind() == FunctionType::Kind::External)
+	{
+		for (auto const& t: _funType.parameterTypes() + _funType.returnParameterTypes())
+		{
+			solAssert(t->annotation().type, "Type not set for parameter.");
+			if (!t->annotation().type->interfaceType(false).get())
+				m_errorReporter.typeError(t->location(), "Internal type cannot be used for external function type.");
+		}
 		solAssert(fun.interfaceType(false), "External function type uses internal types.");
 	}
+}
 
 bool TypeChecker::visit(InlineAssembly const& _inlineAssembly)
 {

libsolidity/analysis/TypeChecker.h

@@ -112,7 +112,6 @@ private:
 
 	void endVisit(InheritanceSpecifier const& _inheritance) override;
 	void endVisit(UsingForDirective const& _usingFor) override;
-	bool visit(StructDefinition const& _struct) override;
 	bool visit(FunctionDefinition const& _function) override;
 	bool visit(VariableDeclaration const& _variable) override;
 	/// We need to do this manually because we want to pass the bases of the current contract in

libsolidity/ast/AST.cpp

@@ -257,12 +257,13 @@ TypeNameAnnotation& TypeName::annotation() const
 
 TypePointer StructDefinition::type() const
 {
+	solAssert(annotation().recursive.has_value(), "Requested struct type before DeclarationTypeChecker.");
 	return TypeProvider::typeType(TypeProvider::structType(*this, DataLocation::Storage));
 }
 
-TypeDeclarationAnnotation& StructDefinition::annotation() const
+StructDeclarationAnnotation& StructDefinition::annotation() const
 {
-	return initAnnotation<TypeDeclarationAnnotation>();
+	return initAnnotation<StructDeclarationAnnotation>();
 }
 
 TypePointer EnumValue::type() const
@@ -558,6 +559,18 @@ bool VariableDeclaration::isExternalCallableParameter() const
 	return false;
 }
 
+bool VariableDeclaration::isPublicCallableParameter() const
+{
+	if (!isCallableOrCatchParameter())
+		return false;
+
+	if (auto const* callable = dynamic_cast<CallableDeclaration const*>(scope()))
+		if (callable->visibility() == Visibility::Public)
+			return !isReturnParameter();
+
+	return false;
+}
+
 bool VariableDeclaration::isInternalCallableParameter() const
 {
 	if (!isCallableOrCatchParameter())
@@ -616,13 +629,21 @@ set<VariableDeclaration::Location> VariableDeclaration::allowedDataLocations() c
 	else if (isLocalVariable())
 	{
 		solAssert(typeName(), "");
-		solAssert(typeName()->annotation().type, "Can only be called after reference resolution");
-		if (typeName()->annotation().type->category() == Type::Category::Mapping)
+		auto dataLocations = [](TypePointer _type, auto&& _recursion) -> set<Location> {
+			solAssert(_type, "Can only be called after reference resolution");
+			switch (_type->category())
+			{
+				case Type::Category::Array:
+					return _recursion(dynamic_cast<ArrayType const*>(_type)->baseType(), _recursion);
+				case Type::Category::Mapping:
 					return set<Location>{ Location::Storage };
-		else
+				default:
 					//  TODO: add Location::Calldata once implemented for local variables.
 					return set<Location>{ Location::Memory, Location::Storage };
 			}
+		};
+		return dataLocations(typeName()->annotation().type, dataLocations);
+	}
 	else
 		// Struct members etc.
 		return set<Location>{ Location::Unspecified };

libsolidity/ast/AST.h

@@ -607,7 +607,7 @@ public:
 	bool isVisibleInDerivedContracts() const override { return true; }
 	bool isVisibleViaContractTypeAccess() const override { return true; }
 
-	TypeDeclarationAnnotation& annotation() const override;
+	StructDeclarationAnnotation& annotation() const override;
 
 private:
 	std::vector<ASTPointer<VariableDeclaration>> m_members;
@@ -914,6 +914,8 @@ public:
 	/// @returns true if this variable is a parameter (not return parameter) of an external function.
 	/// This excludes parameters of external function type names.
 	bool isExternalCallableParameter() const;
+	/// @returns true if this variable is a parameter (not return parameter) of a public function.
+	bool isPublicCallableParameter() const;
 	/// @returns true if this variable is a parameter or return parameter of an internal function
 	/// or a function type of internal visibility.
 	bool isInternalCallableParameter() const;

libsolidity/ast/ASTAnnotations.h

@@ -128,6 +128,16 @@ struct TypeDeclarationAnnotation: DeclarationAnnotation
 	std::string canonicalName;
 };
 
+struct StructDeclarationAnnotation: TypeDeclarationAnnotation
+{
+	/// Whether the struct is recursive, i.e. if the struct (recursively) contains a member that involves a struct of the same
+	/// type, either in a dynamic array, as member of another struct or inside a mapping.
+	/// Only cases in which the recursive occurrence is within a dynamic array or a mapping are valid, while direct
+	/// recursion immediately raises an error.
+	/// Will be filled in by the DeclarationTypeChecker.
+	std::optional<bool> recursive;
+};
+
 struct ContractDefinitionAnnotation: TypeDeclarationAnnotation, StructurallyDocumentedAnnotation
 {
 	/// List of functions without a body. Can also contain functions from base classes.

libsolidity/ast/Types.cpp

@@ -1649,12 +1649,50 @@ bool ArrayType::operator==(Type const& _other) const
 	return isDynamicallySized() || length() == other.length();
 }
 
-bool ArrayType::validForCalldata() const
+BoolResult ArrayType::validForLocation(DataLocation _loc) const
 {
 	if (auto arrayBaseType = dynamic_cast<ArrayType const*>(baseType()))
-		if (!arrayBaseType->validForCalldata())
-			return false;
-	return isDynamicallySized() || unlimitedStaticCalldataSize(true) <= numeric_limits<unsigned>::max();
+	{
+		BoolResult result = arrayBaseType->validForLocation(_loc);
+		if (!result)
+			return result;
+	}
+	if (isDynamicallySized())
+		return true;
+	switch (_loc)
+	{
+		case DataLocation::Memory:
+		{
+			bigint size = bigint(length());
+			auto type = m_baseType;
+			while (auto arrayType = dynamic_cast<ArrayType const*>(type))
+			{
+				if (arrayType->isDynamicallySized())
+					break;
+				else
+				{
+					size *= arrayType->length();
+					type = arrayType->baseType();
+				}
+			}
+			if (type->isDynamicallySized())
+				size *= type->memoryHeadSize();
+			else
+				size *= type->memoryDataSize();
+			if (size >= numeric_limits<unsigned>::max())
+				return BoolResult::err("Type too large for memory.");
+			break;
+		}
+		case DataLocation::CallData:
+		{
+			if (unlimitedStaticCalldataSize(true) >= numeric_limits<unsigned>::max())
+				return BoolResult::err("Type too large for calldata.");
+			break;
+		}
+		case DataLocation::Storage:
+			break;
+	}
+	return true;
 }
 
 bigint ArrayType::unlimitedStaticCalldataSize(bool _padded) const
@@ -2175,32 +2213,57 @@ MemberList::MemberMap StructType::nativeMembers(ContractDefinition const*) const
 
 TypeResult StructType::interfaceType(bool _inLibrary) const
 {
-	if (_inLibrary && m_interfaceType_library.has_value())
-		return *m_interfaceType_library;
-
-	if (!_inLibrary && m_interfaceType.has_value())
+	if (!_inLibrary)
+	{
+		if (!m_interfaceType.has_value())
+		{
+			if (recursive())
+				m_interfaceType = TypeResult::err("Recursive type not allowed for public or external contract functions.");
+			else
+			{
+				TypeResult result{TypePointer{}};
+				for (ASTPointer<VariableDeclaration> const& member: m_struct.members())
+				{
+					if (!member->annotation().type)
+					{
+						result = TypeResult::err("Invalid type!");
+						break;
+					}
+					auto interfaceType = member->annotation().type->interfaceType(false);
+					if (!interfaceType.get())
+					{
+						solAssert(!interfaceType.message().empty(), "Expected detailed error message!");
+						result = interfaceType;
+						break;
+					}
+				}
+				if (result.message().empty())
+					m_interfaceType = TypeProvider::withLocation(this, DataLocation::Memory, true);
+				else
+					m_interfaceType = result;
+			}
+		}
 		return *m_interfaceType;
+	}
+	else if (m_interfaceType_library.has_value())
+		return *m_interfaceType_library;
 
 	TypeResult result{TypePointer{}};
 
-	m_recursive = false;
-
-	auto visitor = [&](
-		StructDefinition const& _struct,
-		util::CycleDetector<StructDefinition>& _cycleDetector,
-		size_t /*_depth*/
-	)
-	{
+	util::BreadthFirstSearch<StructDefinition const*> breadthFirstSearch{{&m_struct}};
+	breadthFirstSearch.run(
+		[&](StructDefinition const* _struct, auto&& _addChild) {
 				// Check that all members have interface types.
 				// Return an error if at least one struct member does not have a type.
 				// This might happen, for example, if the type of the member does not exist.
-		for (ASTPointer<VariableDeclaration> const& variable: _struct.members())
+				for (ASTPointer<VariableDeclaration> const& variable: _struct->members())
 				{
 					// If the struct member does not have a type return false.
 					// A TypeError is expected in this case.
 					if (!variable->annotation().type)
 					{
 						result = TypeResult::err("Invalid type!");
+						breadthFirstSearch.abort();
 						return;
 					}
 
@@ -2210,58 +2273,59 @@ TypeResult StructType::interfaceType(bool _inLibrary) const
 						memberType = dynamic_cast<ArrayType const*>(memberType)->baseType();
 
 					if (StructType const* innerStruct = dynamic_cast<StructType const*>(memberType))
-				if (
-					innerStruct->m_recursive == true ||
-					_cycleDetector.run(innerStruct->structDefinition())
-				)
 					{
-					m_recursive = true;
-					if (_inLibrary && location() == DataLocation::Storage)
-						continue;
-					else
+						if (innerStruct->recursive() && !(_inLibrary && location() == DataLocation::Storage))
 						{
-						result = TypeResult::err("Recursive structs can only be passed as storage pointers to libraries, not as memory objects to contract functions.");
+							result = TypeResult::err(
+								"Recursive structs can only be passed as storage pointers to libraries, not as memory objects to contract functions."
+							);
+							breadthFirstSearch.abort();
 							return;
 						}
+						else
+							_addChild(&innerStruct->structDefinition());
 					}
-
+					else
+					{
 						auto iType = memberType->interfaceType(_inLibrary);
 						if (!iType.get())
 						{
 							solAssert(!iType.message().empty(), "Expected detailed error message!");
 							result = iType;
+							breadthFirstSearch.abort();
 							return;
 						}
 					}
-	};
+			}
+		}
+	);
 
-	m_recursive = m_recursive.value() || (util::CycleDetector<StructDefinition>(visitor).run(structDefinition()) != nullptr);
-
-	std::string const recursiveErrMsg = "Recursive type not allowed for public or external contract functions.";
-
-	if (_inLibrary)
-	{
 	if (!result.message().empty())
-			m_interfaceType_library = result;
-		else if (location() == DataLocation::Storage)
+		return result;
+
+	if (location() == DataLocation::Storage)
 		m_interfaceType_library = this;
 	else
 		m_interfaceType_library = TypeProvider::withLocation(this, DataLocation::Memory, true);
-
-		if (m_recursive.value())
-			m_interfaceType = TypeResult::err(recursiveErrMsg);
-
 	return *m_interfaceType_library;
 }
 
-	if (m_recursive.value())
-		m_interfaceType = TypeResult::err(recursiveErrMsg);
-	else if (!result.message().empty())
-		m_interfaceType = result;
-	else
-		m_interfaceType = TypeProvider::withLocation(this, DataLocation::Memory, true);
+BoolResult StructType::validForLocation(DataLocation _loc) const
+{
+	for (auto const& member: m_struct.members())
+		if (auto referenceType = dynamic_cast<ReferenceType const*>(member->annotation().type))
+		{
+			BoolResult result = referenceType->validForLocation(_loc);
+			if (!result)
+				return result;
+		}
+	return true;
+}
 
-	return *m_interfaceType;
+bool StructType::recursive() const
+{
+	solAssert(m_struct.annotation().recursive.has_value(), "Called StructType::recursive() before DeclarationTypeChecker.");
+	return *m_struct.annotation().recursive;
 }
 
 std::unique_ptr<ReferenceType> StructType::copyForLocation(DataLocation _location, bool _isPointer) const
@@ -2644,21 +2708,11 @@ FunctionType::FunctionType(FunctionTypeName const& _typeName):
 	for (auto const& t: _typeName.parameterTypes())
 	{
 		solAssert(t->annotation().type, "Type not set for parameter.");
-		if (m_kind == Kind::External)
-			solAssert(
-				t->annotation().type->interfaceType(false).get(),
-				"Internal type used as parameter for external function."
-			);
 		m_parameterTypes.push_back(t->annotation().type);
 	}
 	for (auto const& t: _typeName.returnParameterTypes())
 	{
 		solAssert(t->annotation().type, "Type not set for return parameter.");
-		if (m_kind == Kind::External)
-			solAssert(
-				t->annotation().type->interfaceType(false).get(),
-				"Internal type used as return parameter for external function."
-			);
 		m_returnParameterTypes.push_back(t->annotation().type);
 	}
 

libsolidity/ast/Types.h

@@ -706,6 +706,9 @@ public:
 	/// never change the contents of the original value.
 	bool isPointer() const;
 
+	/// @returns true if this is valid to be stored in data location _loc
+	virtual BoolResult validForLocation(DataLocation _loc) const = 0;
+
 	bool operator==(ReferenceType const& _other) const
 	{
 		return location() == _other.location() && isPointer() == _other.isPointer();
@@ -772,8 +775,7 @@ public:
 	TypePointer decodingType() const override;
 	TypeResult interfaceType(bool _inLibrary) const override;
 
-	/// @returns true if this is valid to be stored in calldata
-	bool validForCalldata() const;
+	BoolResult validForLocation(DataLocation _loc) const override;
 
 	/// @returns true if this is a byte array or a string
 	bool isByteArray() const { return m_arrayKind != ArrayKind::Ordinary; }
@@ -827,8 +829,7 @@ public:
 	bool canLiveOutsideStorage() const override { return m_arrayType.canLiveOutsideStorage(); }
 	std::string toString(bool _short) const override;
 
-	/// @returns true if this is valid to be stored in calldata
-	bool validForCalldata() const { return m_arrayType.validForCalldata(); }
+	BoolResult validForLocation(DataLocation _loc) const override { return m_arrayType.validForLocation(_loc); }
 
 	ArrayType const& arrayType() const { return m_arrayType; }
 	u256 memoryDataSize() const override { solAssert(false, ""); }
@@ -934,15 +935,9 @@ public:
 	Type const* encodingType() const override;
 	TypeResult interfaceType(bool _inLibrary) const override;
 
-	bool recursive() const
-	{
-		if (m_recursive.has_value())
-			return m_recursive.value();
+	BoolResult validForLocation(DataLocation _loc) const override;
 
-		interfaceType(false);
-
-		return m_recursive.value();
-	}
+	bool recursive() const;
 
 	std::unique_ptr<ReferenceType> copyForLocation(DataLocation _location, bool _isPointer) const override;
 
@@ -971,7 +966,6 @@ private:
 	// Caches for interfaceType(bool)
 	mutable std::optional<TypeResult> m_interfaceType;
 	mutable std::optional<TypeResult> m_interfaceType_library;
-	mutable std::optional<bool> m_recursive;
 };
 
 /**

libsolutil/Algorithms.h

@@ -114,6 +114,10 @@ struct BreadthFirstSearch
 		}
 		return *this;
 	}
+	void abort()
+	{
+		verticesToTraverse.clear();
+	}
 
 	std::set<V> verticesToTraverse;
 	std::set<V> visited{};

libsolutil/Result.h

@@ -36,7 +36,7 @@ namespace solidity::util
 ///
 
 template <class ResultType>
-class Result
+class [[nodiscard]] Result
 {
 public:
 	/// Constructs a result with _value and an empty message.

test/libsolidity/SolidityTypes.cpp

@@ -184,6 +184,7 @@ BOOST_AUTO_TEST_CASE(type_identifiers)
 	BOOST_CHECK_EQUAL(ContractType(c, true).identifier(), "t_super$_MyContract$$$_$2");
 
 	StructDefinition s(++id, {}, make_shared<string>("Struct"), {});
+	s.annotation().recursive = false;
 	BOOST_CHECK_EQUAL(s.type()->identifier(), "t_type$_t_struct$_Struct_$3_storage_ptr_$");
 
 	EnumDefinition e(++id, {}, make_shared<string>("Enum"), {});

test/libsolidity/syntaxTests/array/length/local_memory_too_large.sol

@@ -0,0 +1,14 @@
+contract C {
+  function f() public pure
+  {
+    bytes32[1263941234127518272][500] memory x;
+    uint[2**30][] memory y;
+    uint[2**30][2**30][] memory z;
+    uint[2**16][2**16][] memory w;
+  }
+}
+// ----
+// TypeError: (48-90): Type too large for memory.
+// TypeError: (96-118): Type too large for memory.
+// TypeError: (124-153): Type too large for memory.
+// TypeError: (159-188): Type too large for memory.

test/libsolidity/syntaxTests/array/length/parameter_too_large.sol

@@ -2,4 +2,4 @@ contract C {
   function f(bytes32[1263941234127518272] memory) public pure {}
 }
 // ----
-// TypeError: (26-61): Array is too large to be encoded.
+// TypeError: (26-61): Type too large for memory.

test/libsolidity/syntaxTests/array/length/parameter_too_large_multidim.sol

@@ -5,7 +5,7 @@ contract C {
   function f(uint[2**16][2**16][] memory) public pure {}
 }
 // ----
-// TypeError: (26-66): Array is too large to be encoded.
-// TypeError: (96-116): Array is too large to be encoded.
-// TypeError: (146-173): Array is too large to be encoded.
-// TypeError: (203-230): Array is too large to be encoded.
+// TypeError: (26-66): Type too large for memory.
+// TypeError: (96-116): Type too large for memory.
+// TypeError: (146-173): Type too large for memory.
+// TypeError: (203-230): Type too large for memory.

test/libsolidity/syntaxTests/array/length/parameter_too_large_multidim_ABIv2.sol

@@ -5,5 +5,5 @@ contract C {
   function f(uint[2**30][2**30][][] memory) public pure {}
 }
 // ----
-// TypeError: (61-101): Array is too large to be encoded.
-// TypeError: (131-160): Array is too large to be encoded.
+// TypeError: (61-101): Type too large for memory.
+// TypeError: (131-160): Type too large for memory.

test/libsolidity/syntaxTests/iceRegressionTests/identifier_collision_return_declare.sol

@@ -0,0 +1,5 @@
+contract C {
+        function ( uint ) external returns ( a [ ] calldata ) public a = ( 1 / 2 ) ;
+}
+// ----
+// TypeError: (58-59): Name has to refer to a struct, enum or contract.

test/libsolidity/syntaxTests/iceRegressionTests/large_array_in_memory_struct.sol

@@ -0,0 +1,17 @@
+contract C {
+        struct X { bytes31 [ 3 ] x1 ;
+                uint x2 ;
+        }
+        struct S { uint256 [ ] [ 0.425781 ether ] s1 ;
+                uint [ 2 ** 0xFF ] [ 2 ** 0x42 ] s2 ;
+                X s3 ;
+                mapping ( uint => address payable ) c ;
+                uint [ 9 hours ** 16 ] d ;
+                string s ;
+        }
+        function f ( ) public { function ( function ( bytes9 , uint ) external pure returns ( uint ) , uint ) external pure returns ( uint ) [ 3 ] memory s2 ;
+                S memory s ;
+        }
+}
+// ----
+// TypeError: (530-540): Type too large for memory.

test/libsolidity/syntaxTests/iceRegressionTests/large_array_in_memory_struct_2.sol

@@ -0,0 +1,12 @@
+contract C {
+    struct R { uint[10][10] y; }
+    struct S { uint a; uint b; R d; uint[20][20][2999999999999999999999999990] c; }
+    function f() public pure {
+        C.S memory y;
+        C.S[10] memory z;
+        y.a < 2;
+        z; y;
+    }
+}
+// ----
+// TypeError: (169-181): Type too large for memory.

test/libsolidity/syntaxTests/iceRegressionTests/large_struct_array.sol

@@ -7,3 +7,4 @@ contract C {
     }
 }
 // ----
+// TypeError: (226-234): Type too large for memory.

test/libsolidity/syntaxTests/iceRegressionTests/memory_mapping_array.sol

@@ -0,0 +1,7 @@
+ contract C {
+        function h ( bool flag ) public returns ( bool c ) {
+                mapping ( string => uint24 ) [ 1 ] memory val ;
+        }
+}
+// ----
+// TypeError: (91-136): Data location must be "storage" for variable, but "memory" was given.

test/libsolidity/syntaxTests/iceRegressionTests/recursive_struct_memory.sol

@@ -0,0 +1,13 @@
+contract Test {
+        struct RecursiveStruct {
+                address payable d ;
+                mapping ( uint => address payable ) c ;
+                mapping ( uint => address payable [ ] ) d ;
+        }
+        function func ( ) private pure {
+                RecursiveStruct [ 1 ] memory val ;
+                val ;
+        }
+}
+// ----
+// DeclarationError: (157-198): Identifier already declared.

test/libsolidity/syntaxTests/nameAndTypeResolution/207_no_mappings_in_memory_array.sol

@@ -4,4 +4,4 @@ contract C {
     }
 }
 // ----
-// TypeError: (47-77): Type mapping(uint256 => uint256)[] memory is only valid in storage.
+// TypeError: (47-77): Data location must be "storage" for variable, but "memory" was given.

test/libsolidity/syntaxTests/nameAndTypeResolution/476_too_large_arrays_for_calldata_external.sol

@@ -3,4 +3,4 @@ contract C {
     }
 }
 // ----
-// TypeError: (28-56): Array is too large to be encoded.
+// TypeError: (28-56): Type too large for calldata.

test/libsolidity/syntaxTests/nameAndTypeResolution/477_too_large_arrays_for_calldata_internal.sol

@@ -3,4 +3,4 @@ contract C {
     }
 }
 // ----
-// TypeError: (28-54): Array is too large to be encoded.
+// TypeError: (28-54): Type too large for memory.

test/libsolidity/syntaxTests/nameAndTypeResolution/478_too_large_arrays_for_calldata_public.sol

@@ -3,4 +3,4 @@ contract C {
     }
 }
 // ----
-// TypeError: (28-54): Array is too large to be encoded.
+// TypeError: (28-54): Type too large for memory.

test/libsolidity/syntaxTests/structs/recursion/recursive_struct_forward_reference.sol

@@ -4,7 +4,7 @@ contract C {
     function f(Data.S memory a) public {}
 }
 contract Data {
-    struct S { S x; }
+    struct S { S[] x; }
 }
 // ----
 // TypeError: (63-78): Recursive type not allowed for public or external contract functions.

test/libsolidity/syntaxTests/structs/recursion/recursive_struct_function_pointer.sol

@@ -0,0 +1,10 @@
+pragma experimental ABIEncoderV2;
+contract C {
+    struct S {
+        uint a;
+        function() external returns (S memory) sub;
+    }
+    function f() public pure returns (S memory) {
+    }
+}
+// ----

test/libsolidity/syntaxTests/structs/struct_var_member.sol

@@ -0,0 +1,7 @@
+contract C {
+	struct S {
+		var x;
+	}
+}
+// ----
+// ParserError: (27-30): Expected explicit type name.

test/libsolidity/syntaxTests/types/cyclic_dependency_check_on_struct_exhausted.sol

@@ -257,4 +257,4 @@ contract Main {
     struct JW { int i; }
 }
 // ----
-// DeclarationError: (6091-6111): Struct definition exhausting cyclic dependency validator.
+// DeclarationError: (6091-6111): Struct definition exhausts cyclic dependency validator.

test/libsolidity/syntaxTests/types/global_struct_recursive.sol

@@ -0,0 +1,8 @@
+struct s1 { s2 x; }
+struct s2 { s1 y; }
+
+contract C {
+    // whatever
+}
+// ----
+// TypeError: (0-19): Recursive struct definition.

test/libsolidity/syntaxTests/types/mapping/function_type_argument_external.sol

@@ -4,4 +4,3 @@ contract C {
 }
 // ----
 // TypeError: (37-64): Data location must be "memory" for parameter in function, but "storage" was given.
-// TypeError: (37-64): Internal type cannot be used for external function type.

test/libsolidity/syntaxTests/types/mapping/function_type_return_external.sol

@@ -4,4 +4,3 @@ contract C {
 }
 // ----
 // TypeError: (57-84): Data location must be "memory" for return parameter in function, but "storage" was given.
-// TypeError: (57-84): Internal type cannot be used for external function type.