-
-
Notifications
You must be signed in to change notification settings - Fork 608
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix Issue 20101 - BetterC: Template instantiation in CTFE only contex… #14789
Conversation
…t should skip codegen
|
Thanks for your pull request, @WalterBright! Bugzilla references
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#14789" |
|
This enables more use of the GC in CTFE for @nogc code. How much more, I'm not sure. GC checking is already turned off for most CTFE use. It's just that for this particular case, the nogc betterC check was in the code gen. |
| @@ -1676,7 +1676,7 @@ Expression semanticTraits(TraitsExp e, Scope* sc) | |||
| uint errors = global.startGagging(); | |||
| Scope* sc2 = sc.push(); | |||
| sc2.tinst = null; | |||
| sc2.minst = null; | |||
| sc2.minst = null; // this is why code for these are not emitted to object file | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not finding where Scope is declared via GH and I've forgotten where it's located, but wouldn't it be good to also document this behavior there as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why what is emitted and what isn't took me a while to find out. It's more complex than just minst. This is the only case I found that set minst to null. It could use a refactor, but I thought I'd leave a breadcrumb for the next guy wondering why traits compiles doesn't emit code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Essentially, too many people have worked on this aspect and the result is incoherence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed. Knowing what state the AST is in, is for sure an annoying beast!
|
That went better than I expected. |
|
Probably should target stable |
…t should skip codegen
The code that determines what is "compile only" or not is a mess. Took me a while to figure out what knob to turn.