Extend rustc_on_implemented to improve more ? error messages
#85596
Conversation
rust-highfive
added
the
S-waiting-on-review
scottmcm
changed the title
rustc_on_implemented to improve a ?-on-ControlFlow error messagerustc_on_implemented to improve more ? error messages
scottmcm
force-pushed
the
more-on-unimplemented
branch
from
ca45e7e to
b6d1cf1
Compare
scottmcm
force-pushed
the
more-on-unimplemented
branch
from
b6d1cf1 to
8be6799
Compare
| --> $DIR/bad-interconversion.rs:22:22 | ||
| | | ||
| LL | / fn result_to_option() -> Option<u16> { | ||
| LL | | Some(Err("hello")?) | ||
| | | ^ this `?` produces `Result<Infallible, &str>`, which is incompatible with `Option<u16>` | ||
| | | ^ use `.ok()?` if you want to discard the `Result<Infallible, &str>` error information |
There was a problem hiding this comment.
I wonder if it would be clearer if we mentioned something like if you want to discard the Result::Err(&str) or something along those lines 🤔
There was a problem hiding this comment.
Yeah, the residual being "coloured" with the Try type does make it harder to put in this error message nicely. I guess it would be less bad with just Result<!, &str>, but Infallible is long enough to be distracting. 🤷
There was a problem hiding this comment.
I'm more worried about confusing newcomers than anything else, anyone that already knows about Result, Option and .ok() wouldn't be confused regardless of wording, but someone who doesn't yet have a good grasp of what an enum even or that hasn't internalized Result::Err is the error information might be helped with things being more spelled out. Either way, I'm ok with the current wording.
There was a problem hiding this comment.
I have been looking at the quadratic growth of the on-unimplemented attribute here and been wondering if it should turn into custom code for it at some point. If at some point in the future you get inspired with a great idea for how these error should look, feel free to assign me an issue. (Or, of course, just do it yourself if that's easier that answering my inevitable noob questions 😅)
|
@bors r+ |
|
📌 Commit 8be6799 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
_Selfcould match the generic definition; this adds that functionality for matching the generic definition of type parameters too.Your advice welcome on the wording of all these messages, and which things belong in the message/label/note.
r? @estebank