Skip to content
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

Issue 24118 - ICE / regression from 2.103.1 - segfault on CTFE only code in 2.104.2 and 2.105.0 #15578

Merged
merged 1 commit into from
Sep 11, 2023

Conversation

RazvanN7
Copy link
Contributor

@RazvanN7 RazvanN7 commented Sep 5, 2023

The problem here is that when a lambda is passed to a template instance, the lambda call is inlined in the template instance. The context of the lambda is __ctfe so the compiler decides to not create a lowering for it, however, the template instance is still generated (without the lowering) thus causing the current regression.

A general solution would be to mark template instances as not needing code generation when instantiated from a speculative context, however, this is not easy to implement given the current caching strategy of template instances.

Until we fix that, we need to bail out of not generating the compiler hook call to avoid ices in e2ir.

@dlang-bot
Copy link
Contributor

dlang-bot commented Sep 5, 2023

Thanks for your pull request and interest in making D better, @RazvanN7! 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: and Returns:)

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
24118 regression ICE / regression from 2.103.1 - segfault on CTFE only code in 2.104.2 and 2.105.0

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#15578"

@RazvanN7
Copy link
Contributor Author

RazvanN7 commented Sep 5, 2023

cc @WalterBright

@maxhaton
Copy link
Member

maxhaton commented Sep 5, 2023

Is this issue not that we are generating code at all in __ctfe if statements?

@maxhaton
Copy link
Member

maxhaton commented Sep 5, 2023

Also commit message looks wrong

@RazvanN7
Copy link
Contributor Author

RazvanN7 commented Sep 5, 2023

Is this issue not that we are generating code at all in __ctfe if statements?

Nope, ctfe block is not generated, but template instances from it are.

@maxhaton
Copy link
Member

maxhaton commented Sep 5, 2023

Is this issue not that we are generating code at all in __ctfe if statements?

Nope, ctfe block is not generated, but template instances from it are.

I thought we fixed that oof

@maxhaton
Copy link
Member

maxhaton commented Sep 5, 2023

(This is another thing that a dependency graph would help with, more on that shortly)

@RazvanN7
Copy link
Contributor Author

RazvanN7 commented Sep 5, 2023

@maxhaton I tried to fix it in the past : #13359 but bumped into some issue in phobos that I did not know how to reduce. Will give it another shot these days.

@RazvanN7 RazvanN7 changed the title Issue 23874 - -profile=gc segfaults / ICE regression Issue 24118 - ICE / regression from 2.103.1 - segfault on CTFE only code in 2.104.2 and 2.105.0 Sep 6, 2023
@RazvanN7
Copy link
Contributor Author

RazvanN7 commented Sep 7, 2023

@maxhaton ok with this fix?

@maxhaton
Copy link
Member

maxhaton commented Sep 7, 2023

It looks ok although if possible having the tracegc && needsCodegen pattern as a variable would be good, if you can think of a clean way of doing it.

@RazvanN7
Copy link
Contributor Author

RazvanN7 commented Sep 8, 2023

@maxhaton Done.

@RazvanN7 RazvanN7 merged commit 6bc17b0 into dlang:stable Sep 11, 2023
43 checks passed
@ibuclaw ibuclaw mentioned this pull request Sep 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants