-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Do not ICE in the face of invalid enum discriminant #69903
Conversation
I don't think this is the best way of doing it, but it is certainly a way to mitigate the issues. |
cc @oli-obk |
fields: vec![], | ||
base: None, | ||
}, | ||
_ if ty.references_error() => { | ||
// Handle degenerate input without ICE (#67377). | ||
ExprKind::Literal { literal: ty::Const::zero_sized(cx.tcx, ty), user_ty: None } |
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.
what if you use cx.tcx.types.err
instead of ty
here? That should avoid the TooGeneric
and instead give us an AlreadyReported
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.
Pushing that change as it is reasonable, but there's no change in the output. Could there be a better placeholder than ExprKind::Literal
or ty::Const::zero_sized
?
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.
What do you mean by "change in the output"? If you change the delay_span_bug
back to span_bug
, will it still get triggered? I don't actually expect any stderr changes.
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.
ping @estebank did you have a chance to try this?
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.
Made the requested change, reverting delay_span_bug
change and making ty
types.err
. The span_bug
triggers.
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.
Thanks for trying! I guess at this point in parser recovery we just have to give up.
r? @oli-obk |
This comment has been minimized.
This comment has been minimized.
@bors r+ |
📌 Commit 187f270 has been approved by |
Do not ICE in the face of invalid enum discriminant Fix rust-lang#67377. r? @pnkfelix
Failed in #71161 (comment), @bors r- |
@bors r=oli-obk rebased and reblessed |
📌 Commit f47c4ff has been approved by |
🌲 The tree is currently closed for pull requests below priority 1000, this pull request will be tested once the tree is reopened |
Do not ICE in the face of invalid enum discriminant Fix rust-lang#67377. r? @pnkfelix
Rollup of 6 pull requests Successful merges: - rust-lang#69903 (Do not ICE in the face of invalid enum discriminant) - rust-lang#70354 (Update RELEASES.md for 1.43.0) - rust-lang#70774 (End cleanup on rustdoc-js tools) - rust-lang#70990 (Improve rustdoc source code a bit) - rust-lang#71145 (Add illumos triple) - rust-lang#71166 (Clean up E0518 explanation) Failed merges: r? @ghost
_ if ty.references_error() => { | ||
// Handle degenerate input without ICE (#67377). | ||
ExprKind::Literal { | ||
literal: ty::Const::zero_sized(cx.tcx, cx.tcx.types.err), |
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.
Please use tcx.consts.err
, doing it this way makes it slip past #71049, had I not come across this PR for a completely unrelated reason.
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.
Wait how are we even getting here?! MIR isn't built if typeck_tables_of(def_id).is_tainted_by_errors
and that's where ty
comes from.
Err(ErrorHandled::TooGeneric) => { | ||
span_bug!(tcx.def_span(expr_did), "enum discriminant depends on generic arguments",) | ||
tcx.sess.delay_span_bug( | ||
tcx.def_span(expr_did), | ||
"enum discriminant depends on generic arguments", | ||
); | ||
None |
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.
This must be the actual change here, and it looks a lot like the "ty: use delay_span_bug
in ty::AdtDef::eval_explicit_discr
." commit from #70825.
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.
It is.
Fix #67377.
r? @pnkfelix