fix: add stack frame accounting to manifest builtins

[Flo354]

builtinManifestJSONEx, builtinManifestYamlDoc, and builtinManifestTomlEx in builtins.go serialize values using native Go recursion (aux closures or tomlRenderValue). These recursive functions don't call i.newCall(), so the interpreter's MaxStack counter never increments.

A self-referential object like {a: $} passed to std.manifestJsonEx causes unbounded recursion and memory allocation (~900MB/s) until OOM, even when MaxStack is set. The default manifestation path correctly catches this via manifestJSON in interpreter.go which uses i.newCall().

The fix adds i.newCall(environment{}, false) and a deferred i.stack.popIfExists(stackSize) at the top of each recursive function, matching the pattern used by manifestJSON.

[johnbartholomew]

Could you investigate the test failures? (go test . should show them, though the output is rather garbled; I assume there is some output race going on as well as whatever the underlying problem is)

[Flo354]

I've rewrote the fix. Instead of using i.newCall() at every node (which ate up the global call stack on normal objects), I switched to a simple depth counter passed as parameter:

• Normal manifesting works fine
• Cyclic objects get a clean RUNTIME ERROR when they hit maxManifestDepth (500)
• Added regression tests for all three builtins with cyclic inputs
• All existing tests pass.

[johnbartholomew]

Thanks for the contribution!

This is good, but I don't like having the max depth fixed like this, as occasionally people have very strange use-cases. Although object nesting depth limit and call stack limit aren't quite the same thing they're close enough that I suggest applying the existing MaxStack limit as the depth cap here.

If you make depth count down and check against 0 instead of counting up and checking against a maxManifestDepth value, then you don't need to pass any extra limit argument around and the code stays mostly the same, you can just make the initial call with a depth of MaxStack, which you can get from the interpreter object (interpreter.stack.limit), though you might want to add an internal helper method to get that value cleanly.

Reusing the existing configurable limit will have the same default, but if someone is doing something strange they can set a much larger MaxStack (--max-stack for CLI users).

[Flo354]

I think you are right, reusing MaxStack makes more sense.
Depth now counts down from i.stack.limit and checks against 0. I also removed the fixed constant, users with custom --max-stack get the same limit applied to manifest recursion.

[johnbartholomew]

Looks good! Merged :-)