Term rewriting stops processing on long files #25304
Description
Nim Version
Any version from 0.9.4 forwards
Description
A recursion was added in this commit in hlo.nim for version 0.9.4, to make sure this recursion didn't cause the compiler to hang a recursion limit of 300 was also added. The term-rewriting already had a recursion though, which loops over the children of the node and applies hlo to each child. This means that for files and functions that are longer than 300 nodes this causes term-rewriting to just give up. I believe the best fix would be to either simply reset the recursion counter when looping through children, only increasing the count on the "true" recursion point, pass the recursion count as an argument to the hlo function instead of globally tracking it in a state object, or just remove the counter entirely.
The first solution means that if we end up in a flip-flop scenario where the first child somehow transforms to its parent then the recursion would run forever. Not sure if this is even possible however.
The second solution means that if we have more children that each contribute a small bit to the count it will at some point give up again. Even if it is not the recursion depth we're actually hitting.
The third solution means that each recursive call stack of hlo will have its own count, meaning that it actually tracks recursion depth and not just the number of times the hlo function has been called since it was last reset. This means that each child in the tree will receive the same count, and not the count of the previous child. We can still hit the limit just by going down the tree of nodes, but in my testing with the project where I discovered this I was nowhere near the limit this way. It is of course also possible to combine this and the second solution so that when looping through children they inherit the parent count instead of an increased version.
Initially I just tried removing the limitation completely. This fixed the problem and didn't cause the compiler to hang. I think this might've just been defensive coding, or taking into account something that was true 13 years ago which doesn't hold today. Maybe it can be removed in devel and if it turns out that the check is needed one of the above solutions can be implemented for the next version.
I'll create a PR of my preferred solution of 3+2, but if any of the others appeals more just let me know and I can change it.