[TurkeyMan]

This is an alternative take on #23940, it creates an artificial slice SymbolSlice!T instead of a real proper one, that way the compiler doesn't need any extra tricks.
The disadvantage is that expressions like typeof(__traits(vtblSymbol, C)) and friends will not report (void*)[], but instead SymbolSlice!(void*), which means they may not interact completely naturally with some set of meta, but these are super-niche things, and really just to facilitate lib-defined ClassInfo; so I think we can live with that imperfection.
__traits(vtblSymbol, C)[] would cause it to present as a natural slice and work through any meta correctly.

[github-actions]

DMD perf check

Metric	Base	PR	Δ
compile hello.d (instr)	210.3 M	210.6 M	+0.138%
compile hello.d -O -release (instr)	227.9 M	228.2 M	+0.147%
compile Phobos (instr)	4,891.2 M	4,899.3 M	+0.166%
compile Phobos codegen (instr)	1,426.1 M	1,429.9 M	+0.266%
compile vibe.d (instr)	14,257.9 M	14,277.9 M	+0.140%
dmd binary size (stripped)	8.15 MB	8.14 MB	-0.14%

Breakdown — compile hello.d

Phase (wall, self time)	Base	PR	Δ
sema_other	12.4 ms	12.0 ms	-3.03%
parse	23.2 ms	23.6 ms	+1.58%
sema1	8.0 ms	7.8 ms	-1.63%
codegen	1.7 ms	1.6 ms	-1.85%
sema3	4.3 ms	4.3 ms	+0.05%

Breakdown — compile Phobos

+8.1 M instructions: frontend +4.3 M (+0.12%), codegen +3.8 M (+0.27%)

Phase (wall, self time)	Base	PR	Δ
codegen	329 ms	289 ms	-12.26%
sema3	361 ms	323 ms	-10.59%
sema1	153 ms	135 ms	-11.42%
parse	79.6 ms	70.3 ms	-11.71%
sema_other	164 ms	156 ms	-5.12%
sema2	1.2 ms	0.6 ms	-50.12%
inline	3.5 ms	3.9 ms	+13.09%
ctfe	6.9 ms	7.1 ms	+3.34%

All measurements

Metric	Base	PR	Δ
compile hello.d (instr)	210.3 M	210.6 M	+0.138%
compile hello.d -O -release (instr)	227.9 M	228.2 M	+0.147%
compile Phobos (instr)	4,891.2 M	4,899.3 M	+0.166%
compile Phobos codegen (instr)	1,426.1 M	1,429.9 M	+0.266%
compile vibe.d (instr)	14,257.9 M	14,277.9 M	+0.140%
dmd binary size (stripped)	8.15 MB	8.14 MB	-0.14%
hello binary size (stripped)	0.72 MB	0.72 MB	0.00%
peak RSS (compile hello.d)	44.32 MB	44.17 MB	-0.34%
peak RSS (compile Phobos)	604.2 MB	604.6 MB	+0.06%
peak RSS (compile vibe.d)	1858 MB	1858 MB	+0.02%
page faults (compile hello.d)	8,901	8,908	+0.08%
page faults (compile Phobos)	150,954	151,066	+0.07%
page faults (compile vibe.d)	465,135	465,753	+0.13%
compile dmd itself (wall)	8.6 s	8.6 s	+0.40%
compile hello.d (wall)	49.6 ms	49.5 ms	-0.34%
compile Phobos (wall)	1,098 ms	985 ms	-10.35%

_{624f1f1 vs merge-base 92ac966 · about these metrics}

[dkorpel]

As far as CTFE passing goes this looks mostly sound now, although breaking initSymbol and instantiating all these RTInfo!(SymbolSlice!(const(void))) templates does make it look less appealing in another way...

Is there an intention behind the large amount of dmd code additions (determineVtbl vtblBusy __gshared SymbolDeclaration[void*][3] declarations;) or is that just classic Claude being unaware of existing functionality and loving writing new things from scratch?

[dkorpel]

This function does not have a safe interface currently

[dkorpel]

void main() @safe
{
    SymbolSlice!(int)(20, malloc(1)).get())[7]; // Out of bounds access
}

[TurkeyMan]

[oops, accidentally deleted my comment rather than edit]

How not? It's the symbol length, which is quite certain and not variable.

[TurkeyMan]

Oh I see; yeah it can't be const because transitive const, and therefore it could be modified (or forged!). And so, not really @safe... which sucks, because this has no reason not to be safe. Not sure how to resolve that with this approach :/

[TurkeyMan]

I agree; I don't actually love this approach; but I was trying to find the least-invasive patch. This doesn't have to apply to the init symbol, but it feels lame to me that __traits(initSymbol, T).length is a compile error, where there's no good reason for that restriction.

I would certainly rather solve this with a native slice expression that worked properly... but from your last review, I figured the best approach might be to try and hoist the symbol reference out from the array.

determineVtbl was necessary to flush the vtable before capturing the length; analogous to determineFields. Without that step, your examples where .length would change from one function to the next isn't solved. vtblBusy resists recursion.

It could be Claude solving problems however he likes; I had my hands on the reigns pretty tight, but I don't know what I don't know... if he's invented a solution that already has an established pattern, I wouldn't know to flag that. We need code reviews by experts more than ever!

[TurkeyMan]

...instantiating all these RTInfo!(SymbolSlice!(const(void))) templates...

"all these"? There's just one instance; const void
If RTInfo is not for CTFE, then it can be reduced back to a const(void)[]

[TurkeyMan]

__gshared SymbolDeclaration[void*][3] declarations <- that was pretty weird; I interrogated that, the problem was that the compiler makes a new symbol node at every reference, and then 2 same-symbol references don't compare equal, because it just compares the node itself. It's solving for an existing assumption in DMD.

His weird solution was to de-duplicate the symbols with that cache, so there's only ever one instance and then comparisons naturally work everywhere. Theoretically, this uses less memory than the alternative (existing way) which is be to synthesise a new node at every symbol reference, and then also proliferate a comparison function to everywhere that checks symbols for equality.

The alternate solution actually isn't much more code, but it's theoretically slower; adds cost to every global symbol comparison, and also multiple symbol references duplicate the symbol node. Honestly not clear which approach is better, this cache is probably faster, maybe slightly more memory? Unless symbols are referenced multiple times, then it might be less memory...?

[dkorpel]

"all these"? There's just one instance; const void If RTInfo is not for CTFE, then it can be reduced back to a const(void)[]

Right, I thought it would instantiate for every initializer, but it's only 3 instantiations for the 3 traits

[dkorpel]

ClassDeclaration already stores a vtbl symbol, and determineVtbl is basically 'dsymbolSemantic'. Here's a version instructed to remove the duplicated code and fix unsoundness issues: #23993

[dkorpel]

This doesn't have to apply to the init symbol, but it feels lame to me that __traits(initSymbol, T).length is a compile error, where there's no good reason for that restriction.

initSymbol introduced the awkward concept of forward referencing a glue layer value in the frontend. Being conservative in CTFE keeps a can of worms closed. Open it at your own risk :p

[TurkeyMan]

This doesn't have to apply to the init symbol, but it feels lame to me that __traits(initSymbol, T).length is a compile error, where there's no good reason for that restriction.

initSymbol introduced the awkward concept of forward referencing a glue layer value in the frontend. Being conservative in CTFE keeps a can of worms closed. Open it at your own risk :p

It feels like it's solved though; opaque symbol references are already comprehensively ctfe compatible. Pinning a length beside one just doesn't seem like it should be any more trouble, although it turns out that's a COMPLETELY different thing, and so it is. The length is not from the glue layer, the length is easily known to the frontend directly, which is why we can interact with it from ctfe no problem.