Skip to content
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

Extend rustc_on_unimplemented to allow pointing at enclosing function/closure #61709

Closed
estebank opened this issue Jun 10, 2019 · 5 comments · Fixed by #66651
Closed

Extend rustc_on_unimplemented to allow pointing at enclosing function/closure #61709

estebank opened this issue Jun 10, 2019 · 5 comments · Fixed by #66651
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. F-on_unimplemented Error messages that can be tackled with `#[rustc_on_unimplemented]` P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@estebank
Copy link
Contributor

We should extend rustc_on_unimplemented to be able to point at the enclosing scope. With that capability, we could turn the following:

error[E0277]: the `?` operator can only be used in a function that returns `Result` or `Option` (or another type that implements `std::ops::Try`)
  --> src/lib.rs:28:31
   |
28 |                     let end = parse_range_u32(end)?;
   |                               ^^^^^^^^^^^^^^^^^^^^^ cannot use the `?` operator in a function that returns `std::ops::RangeInclusive<u32>`
   |
   = help: the trait `std::ops::Try` is not implemented for `std::ops::RangeInclusive<u32>`
   = note: required by `std::ops::Try::from_error`

into something along the lines of

error[E0277]: the `?` operator can only be used in a function that returns `Result` or `Option` (or another type that implements `std::ops::Try`)
  --> src/lib.rs:28:31
   |
24 |          .map(|range_spec| {
   |               -
   | ______________|
... |
28 ||                     let end = parse_range_u32(end)?;
   ||                               ^^^^^^^^^^^^^^^^^^^^^ cannot use the `?` operator in a function that returns `std::ops::RangeInclusive<u32>`
... |
48 ||         })
   ||         - this function should return `Result` or `Option` to accept `?`
   ||_________|
   |
   = help: the trait `std::ops::Try` is not implemented for `std::ops::RangeInclusive<u32>`
   = note: required by `std::ops::Try::from_error`

This has caused confusion in the wild.

@estebank estebank added A-diagnostics Area: Messages for errors, warnings, and lints P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. labels Jun 10, 2019
@estebank estebank added the F-on_unimplemented Error messages that can be tackled with `#[rustc_on_unimplemented]` label Aug 4, 2019
@sam09
Copy link
Contributor

sam09 commented Oct 2, 2019

@estebank Can I pick this up? I don't have much of an idea about this, but would like to get involved

@estebank
Copy link
Contributor Author

estebank commented Oct 2, 2019

@sam09 of course! I can take a look at #54946 and #47613 for some changes made to this code, the currently up to date documentation for the feature. You will probably have to take a look at fn report_selection_error and find a way to get the enclosing scope to the current item. This part will likely be hard given that this function only deals with obligations, doesn't have access to the originating item. The most likely successful course of action would be to extend the obligation itself to keep some information (most likely just a span) of the enclosing scope. The you could extend rustc_on_unimplemented to be something like

#[rustc_on_unimplemented(
   on(all(
       any(from_method="from_error", from_method="from_ok"),
       from_desugaring="QuestionMark"),
      message="the `?` operator can only be used in a \
               function that returns `Result` or `Option` \
               (or another type that implements `{Try}`)",
      label="cannot use the `?` operator in a function that returns `{Self}`",
      enclosing_scope="this should return `Result` or `Option`"), // <--- This is new
   on(all(from_method="into_result", from_desugaring="QuestionMark"),
      message="the `?` operator can only be applied to values \
               that implement `{Try}`",
      label="the `?` operator cannot be applied to type `{Self}`")
)]
#[doc(alias = "?")]
pub trait Try 

@basil-cow
Copy link
Contributor

@sam09 would you mind me picking this up?

@sam09
Copy link
Contributor

sam09 commented Nov 21, 2019

@Areredify @estebank
Apologies for not following up. I have been super busy with some things at work.
@Areredify Please go ahead. I haven't found any time to work on this 😄

@basil-cow
Copy link
Contributor

@estebank I implemented it, but I don't know how to compile it, since I changed both libstd(added an annotation to the Try trait) and the compiler.

Centril added a commit to Centril/rust that referenced this issue Dec 3, 2019
…=davidtwco

Add `enclosing scope` parameter to `rustc_on_unimplemented`

Adds a new parameter to `#[rustc_on_unimplemented]`, `enclosing scope`, which highlights the function or closure scope with a message.

The wip part refers to adding this annotation to `Try` trait to improve ergonomics (which I don't know how to do since I change both std and librustc)

Closes rust-lang#61709.
@bors bors closed this as completed in 8dacfc2 Dec 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. F-on_unimplemented Error messages that can be tackled with `#[rustc_on_unimplemented]` P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants