Refuse a returned tree that reaches a node twice

Merged
QU
1d ago
+330-4

A returned binary tree that links back to an earlier node used to stall every runner: the tree serializers walk breadth first with no record of visited nodes, so JavaScript and Python timed out with "check for an infinite loop" in code that had already returned, Java was killed with SIGKILL, and C overran Compiler Explorer's output limit, with no case reporting anything. treeToArray, _tree_to_array, C's json_tree, C++'s toJson(TreeNode*) and Java's json(TreeNode) now refuse a node they reach a second time with "Returned tree must not reach a node twice.", and the cases without that defect still report their own results. C++ was missing from the issue's table but has the same serializer, so it is included.

The issue proposed "Returned tree must not contain a cycle.". A visited set cannot tell a cycle from two parents sharing one subtree, and neither is a tree, so the check refuses both and the message names what it saw rather than claiming a cycle. That settles follow-up 4 of the issue. Rejecting only cycles would need the ancestor path in all five serializers. C has no set, so json_tree scans the nodes it has already dequeued, and it reports the refusal as the case's error the way json_list does since #405. The scan is quadratic but bounded by the 4096-entry queue: a 2047-node chain, the largest the queue holds, takes about 13 ms at -O0.

The tests run the issue's cyclic flatten, the last Morris thread left in place, and expect the message as the error of the three cases with more than one node and the normal result on the empty and single-node cases. Two more flattens give every case with a root two children holding the root's value. One hangs the same node on both sides and must be refused, which is the shared child a cycle-only check would let through. The other uses two distinct nodes and must print as [v,v,v], which keeps the check keyed on the node rather than on val. tests/browser/compiler-explorer.test.js compiles and runs all three locally with cc, g++ and javac and checks the case count. scripts/browser-check.cjs runs them on the page in JavaScript and Python and checks the summary count.

Validation on macOS 26.5 over 3198a99:

  • node --test --test-name-pattern='returned tree' tests/browser/compiler-explorer.test.js: 9 pass. With the serializers from main, the three cycle tests fail (C aborts with SIGABRT, C++ and Java are killed by the 10 s timeout) and so do the three shared-child tests. Refusing only a link back to the root, which lets a shared child through as an ancestor-path check would, still passes the cycle tests and fails the three shared-child tests. Comparing val instead of the node fails the three distinct-children tests. Printing C's refusal as the case's value instead of its error fails both C refusal tests.
  • BROWSER_CHECK_AGENT=offline scripts/browser-check.sh passes. Reverting the JavaScript check fails it at scripts/browser-check.cjs:1658 and reverting only the Python check at scripts/browser-check.cjs:1702, the two cycle runs. Rethrowing the refusal from the per-case catch in web/runners.js still shows the message but fails the summary count at scripts/browser-check.cjs:1663. Refusing only a link back to the root fails the shared-child runs called at lines 1664 (JavaScript) and 1708 (Python), and comparing val fails the distinct-children runs called at lines 1673 and 1718.
  • ./scripts/test.sh passes. Not checked by this run: commentflow, ruff, shfmt, shellcheck, cargo-audit and actionlint are not installed, and the pinned Clang lane runs only on Linux x86_64.

Left out: the bound on json_tree's queue, tracked in #408 and fixed in #422. A returned tree with 2048 or more nodes still writes queue[4096] here, cycle or not, before the walk dequeues a repeated node. The next-pointer and quad tree serializers have no visited check either, tracked in #428.

Closes #386

Summary by cubic

Tree serializers in all five runners now refuse a returned tree that reaches a node twice, instead of walking it forever and leaving every case unreported or the run killed.

The serializers walked breadth first with no record of visited nodes. A visited set cannot tell a cycle from two parents sharing one subtree, and neither is a tree, so the check refuses both and the message names the repeated visit rather than claiming a cycle.

Notes

  • Cases without the defect still report their own results.
  • C has no exceptions, so its harness prints the message as the case's value.
  • C++ is covered too, although it was missing from the issue's table.

Closes #386.