dmd
dmd copied to clipboard
Fix 22540 - Copy parameter types / defaults of dependant template...
... value / alias parameters.
The introduced copies ensure that the initialized parameters of the instantiation do not share objects with the template declaration.
Previously, sharing could occur in the following scenarios:
- type parameter appears inside of value type, e.g.
foo(T, T* val)
Later semantic resolvesT
to the concrete type and hence changes the type ofval
in the template declaration. - alias parameter default value isn't copied and manifested according to the first template instance.
TODO:
- [ ] Investigate https://issues.dlang.org/show_bug.cgi?id=22526 which seems related
Thanks for your pull request and interest in making D better, @MoonlightSentinel! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. Please verify that your PR follows this checklist:
- My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
- My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
- I have provided a detailed rationale explaining my changes
- New or modified functions have Ddoc comments (with
Params:
andReturns:
)
Please see CONTRIBUTING.md for more information.
If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.
Bugzilla references
Auto-close | Bugzilla | Severity | Description |
---|---|---|---|
✓ | 22540 | critical | Instantiation modifies dependant type of value / alias template parameters |
Testing this PR locally
If you don't have a local development environment setup, you can use Digger to test this PR:
dub run digger -- build "stable + dmd#13352"
In that case it’s still worth making it obvious From: FlorianSent: 23 November 2021 23:08To: dlang/dmdCc: Max Haughton; CommentSubject: Re: [dlang/dmd] Fix 22540 - Copy parameter types / defaults of dependant template... (PR ***@***.*** commented on this pull request.In src/dmd/dtemplate.d:> @@ -8039,7 +8043,16 @@ MATCH matchArg(TemplateParameter tp, Scope* sc, RootObject oarg, size_t i, Templ } //printf("\tvalType: %s, ty = %d\n", tvp.valType.toChars(), tvp.valType.ty);- vt = tvp.valType.typeSemantic(tvp.loc, sc);++ // The type might rely on preceeding parameters, e.g.+ // void foo(T, T val)(...) {}+ // Make a copy to not modify the template declaration!but this smells of something really broken in the sema code somewhere so best make it super obvious.Not necessarily, the assumption for template instances is to work on an independent copy of the AST that can be modified by semantic. The code previously didn't make a copy at all.Making the copy conditional is mostly intended to avoid redundant copies.—You are receiving this because you commented.Reply to this email directly, view it on GitHub, or unsubscribe.Triage notifications on the go with GitHub Mobile for iOS or Android.
@MoonlightSentinel what should we do with this
Closing as draft appears to be abandoned.