tlib: order Tree containers with an explicit comparator

[jcelerier]

Problem

compiler/tlib/tree.hh specializes std::less<CTree*> to order trees by serial(), so that iterating an ordered container of trees is reproducible.

libc++ (20+) has a __make_transparent optimisation that rewrites std::less<T> to the transparent std::less<> when it recognises the comparator as the default one:

// libc++ __functional/operations.h
template <class _Tp> struct __make_transparent<_Tp, less<_Tp> > { using type = less<>; };

It applies on the container insert path (__tree::__find_equal), but not on the lookup path (__tree::__count_unique, which calls value_comp()). Every std::set<Tree> / std::map<Tree, T> in the compiler is therefore built in address order and queried in serial order.

The result is a container whose lookups miss entries that are present. In symlistVisit() the visited set stops blocking revisits, so the walk re-descends into signals it has already marked and recurses until the 16 MB compiler thread stack is exhausted:

#14 symlistVisit+0x1ab [compiler/signals/recursivness.cpp @ 163]
#15 symlistVisit+0x147 [compiler/signals/recursivness.cpp @ 158]
...  ~69,900 frames, 240 bytes each -> 16.7 MB

with the same five Tree pointers cycling forever. Instrumenting the set at the point of divergence:

FIRST-DIVERGENCE sig=0x03525AE0 count=0 find=1 size=34
ORDER-CHECK: violations-if-ordered-by-ADDRESS=0  violations-if-ordered-by-SERIAL=10
SERIALS: n=34 distinct=34 duplicated=0

Whether it crashes depends on the addresses malloc happens to return, so it is intermittent — process = (+ ~ _), (+ ~ _); fails about half of the time.

CTree::plist is std::map<Tree, Tree>, so the property/memoization layer is affected too, not just symlist.

Specializing std::less for a program-defined type is undefined behaviour in any case: [namespace.std] allows adding a specialization only if it "meets the standard library requirements for the original template", and std::less<T*> is required to yield the implementation-defined strict total order over pointers. libc++ is entitled to assume std::less<T> means <.

Fix

Replace the specialization with a named treeorder comparator, plus TreeSet / TreeMap<T> aliases, and use them wherever a container is keyed by Tree. A named comparator is not pattern-matched by __make_transparent, so both paths agree.

Results

examples/*.dsp compiled with the LLVM backend, 3 runs each, comparing the emitted IR across runs (clang 22 / libc++, MSYS2 CLANG64):

	before	after
compiled, identical IR on every run	33	258
segfault	223	1
non-deterministic IR	8	0
other errors	32	37

The one remaining segfault (physicalModeling/fds/2dKirchhoffThinPlate.dsp) reproduces identically before the change — a genuinely deep signal graph, unrelated.

"Other errors" are environmental in both columns (unable to open file instruments.lib, undefined symbol : process on library-style files); the count rises only because files that used to crash now get far enough to report a real error.

Determinism is preserved: the ordering is still by serial(), and no example produced differing IR across runs after the change.

Built and tested on MSYS2 CLANG64, clang 22.1.8, LLVM backend.

[sletz]

Thanks @jcelerier. The thing is that @orlarey is currently working on this https://github.com/grame-cncm/faust/tree/master-dev-speedup-new-signals branch where the tlib component has been deeply reworked and optimised (with IA assistance). So I'm not sure how your fix behaves in this new model. @orlarey may better answer here.

[jcelerier]

heya! the main thing is that on windows with clang / libc++22, one really has to use an explicit custom comparator for set / map rather than overriding std::less<...>. If there's going to be releases with a revamped tlib, maybe this patch could just be applied to current release branches that have the bug, and this design point taken into account for the future version ? (I could reproduce it as far back as 2.77 2.78 with clang-22 on windows)

[sletz]

Use of std::less was introduced in b92fe3f. You could machine check that your PR is somewhat similar to what we had before ?

[jcelerier]

yep, reverting it fixes the issue.

minimal repro that fails: process = (+ ~ _), (+ ~ _);