Move direct struct recursion check to detect recursion in global structs.

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;
@@ -79,10 +81,12 @@ bool DeclarationTypeChecker::visit(StructDefinition const& _struct)
 
 	m_currentStructsSeen.insert(&_struct);
 
-	for (auto const& _member: _struct.members())
+	for (auto const& member: _struct.members())
 	{
 		m_recursiveStructSeen = false;
-		_member->accept(*this);
+		member->accept(*this);
+		solAssert(member->annotation().type, "");
+		solAssert(member->annotation().type->canBeStored(), "Type cannot be used in struct.");
 		if (m_recursiveStructSeen)
 			hasRecursiveChild = true;
 	}
@@ -94,6 +98,29 @@ bool DeclarationTypeChecker::visit(StructDefinition const& _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 exhausting 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;
 }
 
@@ -342,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

@@ -65,6 +65,9 @@ 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;

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;

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

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/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/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.