-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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 another bug in circular type detection #41516
Conversation
src/builtins.c
Outdated
// If a supertype can reference the same type, then we may not be | ||
// able to compute the layout of the object before needing to | ||
// publish it, so we must assume it cannot be inlined, if that | ||
// check passes, then we also still need to check the object |
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.
// check passes, then we also still need to check the object | |
// check passes, then we also still need to check the fields too. |
Replaces #41359 ? |
(Please find a stacktrace in #41503 (comment).) |
Co-authored-by: Jeff Bezanson <jeff.bezanson@gmail.com>
@Sacha0 can you provide an example of a case that still fails with this PR? |
@JeffBezanson I will try to reduce the issue enough to pass something useful along tomorrow. Would an rr trace do the trick, absent an MRE? :) |
The issue in #41532 is not fixed by this patch for me (tested in nightly). Reproducer is adding and testing DFTK. |
Co-authored-by: Jeff Bezanson <jeff.bezanson@gmail.com> (cherry picked from commit 1c92e0d)
Co-authored-by: Jeff Bezanson <jeff.bezanson@gmail.com> (cherry picked from commit 1c92e0d)
An rr trace is as good as gold, as always :) |
When running under --bug-report=rr I got a segfault ending with
with no prompt to upload a trace. There's 420M worth of files in a folder in .julia/artifacts/, should I upload that somewhere? But really |
Apologies for not noting the following in this thread earlier: We shipped an rr trace to relevant folks out-of-band, from which they were able to produce an MRE, so there may be no need for more at the moment :). |
Similar to #35275, but through a supertype instead of through the parameters
Fixes #41503
Fixes #41349