Skip to content

Fix #23239: give the nested tempinst a chance of instantiation#23346

Open
gulugulubing wants to merge 5 commits into
dlang:masterfrom
gulugulubing:issue23239
Open

Fix #23239: give the nested tempinst a chance of instantiation#23346
gulugulubing wants to merge 5 commits into
dlang:masterfrom
gulugulubing:issue23239

Conversation

@gulugulubing

Copy link
Copy Markdown
Contributor

Try to fix #23239:

The instance relationship was not immediately clear to me, so I used an LLM to help draw this graph:

module
│
├── template Tmpl                  ← original declaration, parent = module
│   └── template touch             ← original declaration, parent = Tmpl
│
├── Tmpl!()_primary                ← instance (1st, created inside static assert)
│   │   parent = module, inst = self, minst = root (after swap)
│   │
│   ├── int data                   ← copy declaration, parent = Tmpl!()
│   └── template touch             ← copy declaration, parent = Tmpl!()  ← key!
│
├── touch!int                      ← instance (1st, created inside static assert)
│   │   parent = module, inst = self, minst = null  ← BUG!
│   │   tempdecl ──────────────→ template touch (the copy inside Tmpl!())
│   │   tinst = null             (cleared by StaticAssert)
│   │
│   ├── static this()              ← module ctor, parent = touch!int
│   └── enum touch = true          ← eponymous member
│
├── Tmpl!()_secondary              ← instance (2nd, created in main())
│       inst → Tmpl!()_primary, minst = null (after swap)
│       (no members — secondary is just a record, not a copy)
│
└── main()                         ← function declaration

The root cause: touch!int is only instantiated once, inside static assert. StaticAssert::semantic2() sets sc.minst = null before evaluating the condition, so touch!int gets minst = null (speculative). Later when Tmpl!() is used at runtime and gets swapped to a root instance, the nested touch!int is left behind with minst = null, and needsCodegen() skips it entirely — including its static this().

The fix: after the swap makes Tmpl!() a root instance, scan the root module's members for any speculative TemplateInstances whose enclosing chain resolves to the newly-rooted instance. There is no direct backlink from TemplateDeclaration to TemplateInstance, so we filter candidates by checking nested.tempdecl.isInstantiated(). Once found, set nested.minst = root.

@gulugulubing gulugulubing marked this pull request as draft July 1, 2026 23:52
@gulugulubing

Copy link
Copy Markdown
Contributor Author

It looks break the phobo, marked as draft first.

@gulugulubing gulugulubing marked this pull request as ready for review July 2, 2026 02:15
@gulugulubing

Copy link
Copy Markdown
Contributor Author

It looks good now, by moving the fix logic to needsCodegen(). The previous approach modified minst during semantic analysis, which could interfere with module-level template instantiation order and break Phobos builds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

overeager dead code elimination of static assert of a template with static this

1 participant