Better error message for late/early lifetime param mismatch #140523
Conversation
rustbot
added
S-waiting-on-review
| @@ -17,11 +17,11 @@ pub trait Foo<'a, 't> { | |||
|
|
|||
| impl<'a, 't> Foo<'a, 't> for &'a isize { | |||
| fn no_bound<'b:'a>(self, b: Inv<'b>) { | |||
| //~^ ERROR lifetime parameters or bounds on method `no_bound` do not match | |||
| //~^ ERROR lifetime parameters do not match the trait definition, since they differ in late-boundedness | |||
There was a problem hiding this comment.
Hmm, brief thought as I skim this, but how often do we actually use the terminology "late-bound"/"early-bound" in error mesasges? In general, "late-boundedness" is kind of weird.
There was a problem hiding this comment.
no idea, but also we do need a precise term here. hiding this detail feels like we're obscuring the whole fact that this error arises.
There was a problem hiding this comment.
AFAIK (I've once grepped for this) in diagnostics we only ever mention boundedness when a user tries to bind a late-bound var early:
fn main() { fn f<'a>() {} f::<'static>; }
//~^ ERROR cannot specify lifetime arguments explicitly if late bound lifetime parameters are present [E0794]There was a problem hiding this comment.
The reference also uses the terms without any definition 💀 https://github.com/rust-lang/reference/blob/48ad733ce25b71385e917cab0383ed3407603490/src/types/function-item.md?plain=1#L10
There was a problem hiding this comment.
I kind of think that maybe just exclude the "since they differ in late-boundedness" part in the main error text.
We could also just kind of "lie" a bit and state that the reason there is an error is because of the mismatched bounds (which is true, but not precise).
There was a problem hiding this comment.
I'll move it down to a note, but I'd rather keep the early and late in the labels so that people can at least google it. Not really sure how to reword the main message, though, in that case.
There was a problem hiding this comment.
Yeah, having "late-bound" and "early-bound" in the note is fine because of googleability, but something about the "lateboundedness" term rubs me the wrong way - this is mostly just bikeshedding, because I don't immediately have a better answer.
There was a problem hiding this comment.
Reworded this to just say "lifetime parameters do not match the trait definition", and added a note saying "lifetime parameters differ in whether they are early- or late-bound".
compiler-errors
force-pushed
the
late-early-mismatch
branch
from
daec806 to
7ccc65b
Compare
|
Thanks for putting up with my bikeshedding - I think this is clearly an improvement. @bors r+ rollup |
bors
added
S-waiting-on-bors
|
Wait I didn't remove all instances of late-boundedness @bors r- |
bors
added
S-waiting-on-author
compiler-errors
force-pushed
the
late-early-mismatch
branch
from
7ccc65b to
d2b22cf
Compare
This comment has been minimized.
This comment has been minimized.
compiler-errors
force-pushed
the
late-early-mismatch
branch
from
d2b22cf to
f03d246
Compare
|
@bors r=jackh726 |
bors
added
S-waiting-on-bors
…h, r=jackh726 Better error message for late/early lifetime param mismatch Rework the way we report early-/late-bound lifetime param mismatches to equate the trait and impl signatures using region variables, so that we can detect when a late-bound param is present in the signature in place of an early-bound param, or vice versa. The diagnostic is a bit more technical, but it's more obviously clear to see what the problem is, even if it's not great at explaining how to fix it. I think this could be improved further, but I still think it's much better than what exists today. Note to reviewer(s): I'd appreciate if we didn't bikeshed *too* much about this verbiage, b/c I hope it's clear that the old message sucked a lot. I'm happy to file bugs for interested new contributors to improve the messaging further. Edit(fmease): Fixes rust-lang#33624.
Rollup of 15 pull requests Successful merges: - rust-lang#138736 (Sanitizers target modificators) - rust-lang#140260 (Only prefer param-env candidates if they remain non-global after norm) - rust-lang#140523 (Better error message for late/early lifetime param mismatch) - rust-lang#140579 (Remove estebank from automated review assignment) - rust-lang#140641 (detect additional uses of opaques after writeback) - rust-lang#140711 (Do not discard constraints on overflow if there was candidate ambiguity) - rust-lang#140716 (Improve `-Zremap-path-scope` tests with dependency) - rust-lang#140755 ([win][arm64] Disable various DebugInfo tests that don't work on Arm64 Windows) - rust-lang#140756 ([arm64] Pointer auth test should link with C static library statically) - rust-lang#140758 ([win][arm64] Disable MSVC Linker 'Arm Hazard' warning) - rust-lang#140759 ([win][arm64] Disable std::fs tests that require symlinks) - rust-lang#140762 (rustdoc-json: Remove newlines from attributes) - rust-lang#140764 (style: Never break within a nullary function call `func()` or a unit literal `()`) - rust-lang#140769 (Add `DefPathData::OpaqueLifetime` to avoid conflicts for remapped opaque lifetimes) - rust-lang#140773 (triagebot: Better message for changes to `tests/rustdoc-json`) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#140260 (Only prefer param-env candidates if they remain non-global after norm) - rust-lang#140523 (Better error message for late/early lifetime param mismatch) - rust-lang#140579 (Remove estebank from automated review assignment) - rust-lang#140641 (detect additional uses of opaques after writeback) - rust-lang#140711 (Do not discard constraints on overflow if there was candidate ambiguity) - rust-lang#140762 (rustdoc-json: Remove newlines from attributes) - rust-lang#140764 (style: Never break within a nullary function call `func()` or a unit literal `()`) - rust-lang#140769 (Add `DefPathData::OpaqueLifetime` to avoid conflicts for remapped opaque lifetimes) - rust-lang#140773 (triagebot: Better message for changes to `tests/rustdoc-json`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#140523 - compiler-errors:late-early-mismatch, r=jackh726 Better error message for late/early lifetime param mismatch Rework the way we report early-/late-bound lifetime param mismatches to equate the trait and impl signatures using region variables, so that we can detect when a late-bound param is present in the signature in place of an early-bound param, or vice versa. The diagnostic is a bit more technical, but it's more obviously clear to see what the problem is, even if it's not great at explaining how to fix it. I think this could be improved further, but I still think it's much better than what exists today. Note to reviewer(s): I'd appreciate if we didn't bikeshed *too* much about this verbiage, b/c I hope it's clear that the old message sucked a lot. I'm happy to file bugs for interested new contributors to improve the messaging further. Edit(fmease): Fixes rust-lang#33624.
Rework the way we report early-/late-bound lifetime param mismatches to equate the trait and impl signatures using region variables, so that we can detect when a late-bound param is present in the signature in place of an early-bound param, or vice versa.
The diagnostic is a bit more technical, but it's more obviously clear to see what the problem is, even if it's not great at explaining how to fix it. I think this could be improved further, but I still think it's much better than what exists today.
Note to reviewer(s): I'd appreciate if we didn't bikeshed too much about this verbiage, b/c I hope it's clear that the old message sucked a lot. I'm happy to file bugs for interested new contributors to improve the messaging further.