Fix circular dependency in __traits(getAttributes, __traits(getMember, ...))#22541
Fix circular dependency in __traits(getAttributes, __traits(getMember, ...))#22541CyberShadow wants to merge 1 commit intodlang:masterfrom
__traits(getAttributes, __traits(getMember, ...))#22541Conversation
…, ...)) Add a fast path for the nested `__traits(getAttributes, __traits(getMember, T, "name"))` pattern that resolves the member via lightweight `sym.search()` instead of full `expressionSemantic`. This avoids triggering `functionSemantic` → eager `functionSemantic3` (body compilation) on method members, which can cause "circular dependency: functions cannot be interpreted while being compiled" errors in introspection-heavy code. The issue occurs when a struct method iterates `__traits(allMembers)` and reads UDAs via `getAttributes(getMember(...))`. On HEAD, `getMember` on a method with `auto` return type triggers `functionSemantic3` for return type inference, which compiles that method's body. If the body references the introspecting function at compile time (e.g. `enum x = columnNames()`), a circular dependency results because the introspecting function is already being compiled. The fast path falls back to the existing full-semantic path (returns null) whenever the pattern doesn't match or the lightweight resolution fails, so there are no behavioral regressions for other uses of `getAttributes`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Thanks for your pull request, @CyberShadow! Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + dmd#22541" |
dkorpel
left a comment
There was a problem hiding this comment.
This is unmaintainable AI slop, a proper solution should be more general than a dedicated new code path just for this exact test case.
|
It was my idea 🙈 A more general path would mean that __traits getMember now returns something other than a fully resolved symbol. Any suggestions? |
|
One obvious alternative is new dedicated |
That would at least make it explicit that something special is happening, instead of giving special semantics to a magic combination which breaks at the slightest refactor, for example: alias mem = __traits(getMember, Table, name);
static foreach (attr; __traits(getAttributes, mem)) { ...And once a proper solution is implemented, |
|
OK, let's do that. I will resubmit.
(FWIW, this was clearly stated in the "Trade-offs" section.) |
I'm contemplating to what extend this kind of code should be supported. Introspecting all members of a type while inside that type (and potentially still defining that type) is always a minefield of possible contradictions, race conditions, or other edge cases. I recommend putting that kind of code outside the struct instead of mixing it in. |
(I'll be honest, I don't like reading long-winded AI texts) |
I don't think this is going to help. The problem is that we're still recursing into it while in the middle of semantic analysis, so moving the code is not going to break the chain. The only alternative I'm aware of is to give up on the approach and design the application differently, i.e. give up on features that rely on D compile-time introspection such as strong typing.
Understandable, but to be honest it probably would not have been very different if I had written it out by hand. The key point is that we are not OK with pattern-matching expression trees in order to elide problems such as recursive analysis errors. This was not something I was aware of, AI or not. |
From an ideological point of view, I agree in principle. The disconnect is that declaring the body of a method of a type should not count as part of declaring the type. Herein lies the problem at hand: one of the main tools we have to inspect a type is the |
|
The key is to make the methods that are mixed in not part of the introspection of the mixin. I could make your code snippet compilable by skipping - static foreach (name; __traits(allMembers, Table))
+ static foreach (name; __traits(allMembers, Table)) static if (name != "save") |
|
Hmm, that's no good, in practice the two are going to be quite far away, and there might be legitimate cases to get the currently analyzed function's UDAs from outside due to a legitimate dependency chain anyway. |
|
Could we make |
|
That could work. Need to see how the test suite reacts to that, there's a chance it will break certain constructs. |
Generated by Claude Opus 4.6, usual disclaimers apply.
Problem
D's compile-time introspection makes it natural to build ORM-style schemas, serialization frameworks, and other metaprogramming patterns where a type's methods introspect its own members via UDAs. A typical pattern is a template struct (or mixin template) that iterates
__traits(allMembers)to discover@Column-annotated fields, while also containing methods that reference the introspection results at compile time:This fails with:
The same issue occurs with mixin templates that introspect their host type's members — a pattern common in D ORM libraries.
Analysis
The root cause is that we sometimes want to read UDAs on symbols before those symbols are fully semantically resolved. The chain that produces the error is:
static assertevaluatescolumnNames()via CTFE, which starts compiling its body (semantic3)allMembers, which includes"save". For that member,__traits(getMember, Table, "save")runs fullexpressionSemanticon aDotIdExpfunctionSemanticonsave()save()has anautoreturn type, sofunctionSemanticeagerly callsfunctionSemantic3(body compilation) to infer the return typesave()'s body containsenum cols = columnNames(), which tries to CTFEcolumnNames()columnNames()is already being compiled — circular dependencyThe fundamental issue is that
getMemberpulls in far more semantic work than whatgetAttributesactually needs. The fullexpressionSemantic→functionSemantic→functionSemantic3cascade is necessary if you intend to call the member or inspect its type, butgetAttributesonly wants to read UDA metadata.Observation
__traits(getAttributes)only needs theDsymbolto read its.userAttribDecl— a field that is populated during parsing, long before any semantic analysis runs. It never needs a fully-resolved expression, a function's return type, or its compiled body. The expensive semantic work triggered bygetMemberis entirely wasted when the result is immediately passed togetAttributes.Proposal
Add a short-circuit in the
getAttributeshandler that detects when its argument is an unevaluated__traits(getMember, T, "name")and resolves the member via lightweightsym.search()(the same mechanism__traits(hasMember)uses) instead of fullexpressionSemantic. This reads UDAs directly from the symbol table without triggering any function body compilation.Implementation
A private helper
getAttributesOfMemberFastPathis added tocompiler/src/dmd/traits.d. It is called at the top of thegetAttributeshandler, before the existingTemplateInstance_semanticTiargscall:TraitsExpwithident == Id.getMemberand exactly 2 argumentsTemplateInstance_semanticTiargson the innergetMember's arguments to resolve the type and string — this is lightweight (no body compilation)IdentifiergetDsymbol()on the first argument — trivial forTypeStruct/TypeClass/TypeEnum(returns.sym)sym.search(loc, id)— pure symbol table lookup, no semantic analysissm.userAttribDecl.getAttributes()— runsarrayExpressionSemanticonly on the UDA values themselvesThe helper returns
nullat any step that fails, falling back to the existing full-semantic path. This ensures no behavioral regressions — if the fast path can't handle a case, the old code takes over transparently.Trade-offs
This does introduce an inconsistency between the nested and two-step forms:
The UDA results are identical in both cases. The difference is only in side effects: the nested form no longer triggers eager body compilation of method members. Code that relied on
getAttributes(getMember(...))to force early body compilation (an undocumented side effect) would need to use the two-step form. In practice this is unlikely — the nested form is the idiomatic way to read UDAs, and triggering body compilation was never its intent.