-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Add second message for LiveDrop errors #73515
Conversation
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
r? @ecstatic-morse Cc @rust-lang/wg-const-eval |
@@ -588,7 +588,13 @@ impl Visitor<'tcx> for Validator<'mir, 'tcx> { | |||
}; | |||
|
|||
if needs_drop { | |||
self.check_op_spanned(ops::LiveDrop, err_span); | |||
let term_span = terminator.source_info.span; | |||
let op = if term_span.hi() - term_span.lo() > rustc_span::BytePos(1) { |
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 what you want is !term_span.is_dummy()
at which point you should be able to not do this branch at all, as the diagnostics system can handle dummy spans I think
Oh, this is checking the span length. Why are you doing 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.
The ui test diffs seem good to me and you can probably just bless them.
Can you also add a test showing your example in the PR post?
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 was just following @ecstatic-morse suggestion: #72907 (comment)
Also without it there are a lot of overlapping messages, let me get you an example
Update: Nevermind, apparently I was looking a the diffs instead of the actual errors 🤦
src/test/ui/consts/miri_unleashed/feature-gate-unleash_the_miri_inside_of_you.stderr
Show resolved
Hide resolved
📌 Commit 9355168 has been approved by |
…s, r=oli-obk Add second message for LiveDrop errors This is an attempt to fix rust-lang#72907 by adding a second message to the `LiveDrop` diagnostics. Changing from this ``` error[E0493]: destructors cannot be evaluated at compile-time --> src/lib.rs:7:9 | 7 | let mut always_returned = None; | ^^^^^^^^^^^^^^^^^^^ constants cannot evaluate destructors error: aborting due to previous error ``` to this ``` error[E0493]: destructors cannot be evaluated at compile-time --> foo.rs:6:9 | 6 | let mut always_returned = None; | ^^^^^^^^^^^^^^^^^^^ constants cannot evaluate destructors ... 10 | always_returned = never_returned; | --------------- value is dropped here error: aborting due to previous error ``` r? @RalfJung @ecstatic-morse
…arth Rollup of 9 pull requests Successful merges: - rust-lang#72271 (Improve compiler error message for wrong generic parameter order) - rust-lang#72493 ( move leak-check to during coherence, candidate eval) - rust-lang#73398 (A way forward for pointer equality in const eval) - rust-lang#73472 (Clean up E0689 explanation) - rust-lang#73496 (Account for multiple impl/dyn Trait in return type when suggesting `'_`) - rust-lang#73515 (Add second message for LiveDrop errors) - rust-lang#73567 (Clarify --extern documentation.) - rust-lang#73572 (Fix typos in doc comments) - rust-lang#73590 (bootstrap: no `config.toml` exists regression) Failed merges: r? @ghost
This is an attempt to fix #72907 by adding a second message to the
LiveDrop
diagnostics. Changing from thisto this
r? @RalfJung @ecstatic-morse