-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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 ICE when asm_const
and const_refs_to_static
are combined
#129472
fix ICE when asm_const
and const_refs_to_static
are combined
#129472
Conversation
3805bdc
to
4f65da6
Compare
I think this should be fixing a buch of known other ICEs, too. Try running the |
no luck there
|
5e33c81
to
774fbc7
Compare
This comment has been minimized.
This comment has been minimized.
774fbc7
to
957502d
Compare
// FIXME(#129952): We probably want a more principled approach here. | ||
if let Err(terr) = ty.error_reported() { | ||
self.infcx.set_tainted_by_errors(terr); | ||
} |
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 think that maybe we want to do it for all signatures, i.e. assign this match to a variable and then check whether that variable has any error_reported
. I expect that to fix even more issues 😁
I am otherwise quite happy with this change
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.
no further crashes are fixed by that change. I did rename #129952 to reflect that the tainting now occurs in a different place.
957502d
to
7ec7724
Compare
7ec7724
to
49e3b9a
Compare
thanks for your patience and good work ❤️ @bors r+ rollup |
fixes #129462
fixes #126896
fixes #124164
I think this is a case that was missed in the fix for #125558, which inserts a type error in the case of an invalid (that is, non-integer) type being passed to an asm
const
operand.I'm not 100% sure that
span_mirbug_and_err
is the right macro here, but it is used earlier withbuiltin_deref
and seems to do the trick.r? @lcnr