[libSyntax] Assert there are no reference-cycles in the SyntaxArenas

[ahoppen]

As discussed here.

[ahoppen]

@swift-ci Please smoke test

[rintaro]

Traversing a whole tree just for an assertion looks to be an overkill, but I think it's OK because the number of SyntaxArena must be fairly low.

[rintaro]

Since PreviousNodes is already copied, we don't need to make a new copy.

Suggested change

	if (PreviousNodes.count(this) > 0) {
	return true;
	}
	for (auto ChildArena : ChildArenas) {
	std::set<SyntaxArena *> NewPreviousNodes = PreviousNodes;
	NewPreviousNodes.insert(this);
	if (ChildArena->containsReferenceCycle(NewPreviousNodes)) {
	return true;
	}
	}
	return false;
	if (!PreviousNodes.insert(this).second)
	return true;
	return llvm::any_of(ChildArenas, [&](SyntaxArena *Child) {
	Child->containsReferenceCycle(PreviousNodes)
	});

[ahoppen]

Oh, yeah. For some reason I though I was adding a different node depending on which child node I was visiting. But of course I’m not. Also your implementation looks much cleaner. I changed it. Also renamed PreviousNodes to VisitedArenas.

[rintaro]

Suggested change

	assert(!containsReferenceCycle());
	assert(!Arena->containsReferenceCycle({this}));

[ahoppen]

Nice and small performance improvement. I like it 👍🏽

[rintaro]

const if possible.

[ahoppen]

Traversing a whole tree just for an assertion looks to be an overkill, but I think it's OK because the number of SyntaxArena must be fairly low.

I don’t expect there to ever be more than, say, 5 arenas, so the performance hit should be pretty marginal. Plus it’s only in assert builds, so I guess it should be fine.

[ahoppen]

@swift-ci Please smoke test