Suggest unwrapping `???<T>` if a method cannot be found on it but is present on `T`. #102288

mejrs · 2022-09-25T23:10:32Z

This suggests various ways to get inside wrapper types if the method cannot be found on the wrapper type, but is present on the wrappee.

For this PR, those wrapper types include Localkey, MaybeUninit, RefCell, RwLock and Mutex.

rust-highfive · 2022-09-25T23:10:34Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @compiler-errors (or someone else) soon.

Please see the contribution instructions for more information.

rustbot · 2022-09-25T23:10:35Z

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

Stabilizing library features
Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
Changing public documentation in ways that create new stability guarantees
Changing observable runtime behavior of library APIs

compiler/rustc_typeck/src/check/method/suggest.rs

jackh726 · 2022-09-26T17:03:18Z

I wonder if a cooler approach would be to make a diagnostic attribute that can be added to any struct that signals "hey, this is really just a wrapper for an inner type"

(Though, its probably just easily to take the current approach)

mejrs · 2022-09-27T11:58:53Z

I wonder if a cooler approach would be to make a diagnostic attribute that can be added to any struct that signals "hey, this is really just a wrapper for an inner type"

That's a cool idea - worth considering I think. It could end up being rather wordy though, like how rustc_unimplemented is really wordy at times.

bors · 2022-09-27T14:04:42Z

☔ The latest upstream changes (presumably #102306) made this pull request unmergeable. Please resolve the merge conflicts.

rustbot · 2022-09-27T19:42:46Z

Some changes occurred in HTML/CSS themes.

cc @GuillaumeGomez

rustc_error_messages was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @estebank, @TaKO8Ki

rustc_macros::diagnostics was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @estebank, @TaKO8Ki

rustdoc-json-types is a public (although nightly-only) API. If possible, consider changing src/librustdoc/json/conversions.rs; otherwise, make sure you bump the FORMAT_VERSION constant.

cc @CraftSpider, @aDotInTheVoid, @Enselic, @obi1kenobi

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @Folyd, @jsha

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

A change occurred in the Ayu theme.

cc @Cldfire

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @celinval, @vakaras

Some changes occurred in src/librustdoc/clean/types.rs

cc @camelid

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

compiler-errors · 2022-09-27T19:45:08Z

Hm, looks like a rebase was messed up here..

Noratrieb · 2022-09-27T20:07:03Z

@rustbot label -A-bootstrap -A-query-system -A-rustdoc-json -A-translation -T-libs

compiler-errors · 2022-09-27T20:12:37Z

compiler/rustc_hir_analysis/src/check/method/suggest.rs

+                        Some(Mutability::Not) => {
+                            err.span_suggestion_verbose(
+                                expr.span.shrink_to_hi(),
+                                format!(
+                                    "use `.borrow()` to borrow the `{ty}`, \
+                                    panicking if any outstanding mutable borrows exist."
+                                ),
+                                ".borrow()",
+                                Applicability::MaybeIncorrect,
+                            );
+                        }
+                        Some(Mutability::Mut) => {
+                            err.span_suggestion_verbose(
+                                expr.span.shrink_to_hi(),
+                                format!(
+                                    "use `.borrow_mut()` to mutably borrow the `{ty}`, \
+                                    panicking if any outstanding borrows exist."
+                                ),
+                                ".borrow_mut()",
+                                Applicability::MaybeIncorrect,
+                            );
+                        }


Perhaps deduplicate a bit?

Suggested change

Some(Mutability::Not) => {

err.span_suggestion_verbose(

expr.span.shrink_to_hi(),

format!(

"use `.borrow()` to borrow the `{ty}`, \

panicking if any outstanding mutable borrows exist."

),

".borrow()",

Applicability::MaybeIncorrect,

);

}

Some(Mutability::Mut) => {

err.span_suggestion_verbose(

expr.span.shrink_to_hi(),

format!(

"use `.borrow_mut()` to mutably borrow the `{ty}`, \

panicking if any outstanding borrows exist."

),

".borrow_mut()",

Applicability::MaybeIncorrect,

);

}

Some(mutbl) => {

let (suggestion, mutbly, mutbl) = match mutbl {

Mutability::Not => (".borrow()", "", "mutable "),

Mutability::Mut => (".borrow()", "mutably ", ""),

};

err.span_suggestion_verbose(

expr.span.shrink_to_hi(),

format!(

"use `.borrow()` to {mutbly}borrow the `{ty}`, \

panicking if any outstanding {mutbl}borrows exist."

),

suggestion,

Applicability::MaybeIncorrect,

);

}

Hmm I'm not sure. I prefer being too verbose rather than too clever, and this looks very clever 😄

This is a very common pattern to deduplicate essentially identical diagnostic messages. I'd actually prefer if you deduplicated it even more, but this one is the easiest to apply.

compiler-errors · 2022-09-27T21:06:38Z

Actually, since you're adding a bunch of new custom error messages here, do you mind just converting them to derived structs?

https://rustc-dev-guide.rust-lang.org/diagnostics/diagnostic-structs.html

mejrs · 2022-09-28T01:25:51Z

@rustbot ready

I've deduplicated some of the logic but I don't think I can do anything more clever without making a big mess.

Actually, since you're adding a bunch of new custom error messages here, do you mind just converting them to derived structs?

I could be misunderstanding but I can't emit a subdiagnostic struct on a diagnosticbuilder. So it looks to me like that would entail refactoring basically all of suggest.rs and maybe more, because nothing in it uses error structs. I'm not going to do that.

compiler-errors · 2022-09-28T03:08:15Z

I can't emit a subdiagnostic struct on a diagnosticbuilder

https://doc.rust-lang.org/beta/nightly-rustc/rustc_errors/struct.DiagnosticBuilder.html#method.subdiagnostic

compiler-errors · 2022-09-28T03:11:38Z

Whatever though, this is fine for now.

@bors r+ rollup

bors · 2022-09-28T03:11:39Z

📌 Commit 4ff83ce has been approved by compiler-errors

It is now in the queue for this repository.

Rollup of 8 pull requests Successful merges: - rust-lang#100747 (Add long description and test for E0311) - rust-lang#102232 (Stabilize bench_black_box) - rust-lang#102288 (Suggest unwrapping `???<T>` if a method cannot be found on it but is present on `T`.) - rust-lang#102338 (Deny associated type bindings within associated type bindings) - rust-lang#102347 (Unescaping cleanups) - rust-lang#102348 (Tweak `FulfillProcessor`.) - rust-lang#102378 (Use already resolved `self_ty` in `confirm_fn_pointer_candidate`) - rust-lang#102380 (rustdoc: remove redundant mobile `.source > .sidebar` CSS) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

Suggest unwrapping `???<T>` if a method cannot be found on it but is present on `T`. This suggests various ways to get inside wrapper types if the method cannot be found on the wrapper type, but is present on the wrappee. For this PR, those wrapper types include `Localkey`, `MaybeUninit`, `RefCell`, `RwLock` and `Mutex`.

Rollup of 8 pull requests Successful merges: - rust-lang#100747 (Add long description and test for E0311) - rust-lang#102232 (Stabilize bench_black_box) - rust-lang#102288 (Suggest unwrapping `???<T>` if a method cannot be found on it but is present on `T`.) - rust-lang#102338 (Deny associated type bindings within associated type bindings) - rust-lang#102347 (Unescaping cleanups) - rust-lang#102348 (Tweak `FulfillProcessor`.) - rust-lang#102378 (Use already resolved `self_ty` in `confirm_fn_pointer_candidate`) - rust-lang#102380 (rustdoc: remove redundant mobile `.source > .sidebar` CSS) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rust-highfive assigned compiler-errors Sep 25, 2022

rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 25, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 25, 2022

compiler-errors reviewed Sep 25, 2022

View reviewed changes

compiler-errors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 25, 2022

This comment has been minimized.

Sign in to view

mejrs added 2 commits September 27, 2022 21:42

Wrapper suggestions

c4c9415

Address feedback

f3ac328

compiler-errors reviewed Sep 27, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

mejrs added 2 commits September 27, 2022 23:12

Add newline

e9224b3

Deduplicate some logic

4ff83ce

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 28, 2022

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 28, 2022

JohnTitor mentioned this pull request Sep 28, 2022

Rollup of 8 pull requests #102388

Merged

bors merged commit 9436ffc into rust-lang:master Sep 28, 2022

rustbot added this to the 1.66.0 milestone Sep 28, 2022

mejrs mentioned this pull request Oct 4, 2022

Let library authors hook into rustc's suggestions rust-lang/lang-team#177

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Suggest unwrapping `???<T>` if a method cannot be found on it but is present on `T`. #102288

Suggest unwrapping `???<T>` if a method cannot be found on it but is present on `T`. #102288

mejrs commented Sep 25, 2022 •

edited

Loading

rust-highfive commented Sep 25, 2022

rustbot commented Sep 25, 2022

This comment has been minimized.

jackh726 commented Sep 26, 2022

mejrs commented Sep 27, 2022

bors commented Sep 27, 2022

rustbot commented Sep 27, 2022

compiler-errors commented Sep 27, 2022

Noratrieb commented Sep 27, 2022

compiler-errors Sep 27, 2022 •

edited

Loading

mejrs Sep 27, 2022

compiler-errors Sep 27, 2022

This comment has been minimized.

compiler-errors commented Sep 27, 2022

mejrs commented Sep 28, 2022

compiler-errors commented Sep 28, 2022

compiler-errors commented Sep 28, 2022

bors commented Sep 28, 2022

Suggest unwrapping ???<T> if a method cannot be found on it but is present on T. #102288

Suggest unwrapping ???<T> if a method cannot be found on it but is present on T. #102288

Conversation

mejrs commented Sep 25, 2022 • edited Loading

rust-highfive commented Sep 25, 2022

rustbot commented Sep 25, 2022

This comment has been minimized.

jackh726 commented Sep 26, 2022

mejrs commented Sep 27, 2022

bors commented Sep 27, 2022

rustbot commented Sep 27, 2022

compiler-errors commented Sep 27, 2022

Noratrieb commented Sep 27, 2022

compiler-errors Sep 27, 2022 • edited Loading

Choose a reason for hiding this comment

mejrs Sep 27, 2022

Choose a reason for hiding this comment

compiler-errors Sep 27, 2022

Choose a reason for hiding this comment

This comment has been minimized.

compiler-errors commented Sep 27, 2022

mejrs commented Sep 28, 2022

compiler-errors commented Sep 28, 2022

compiler-errors commented Sep 28, 2022

bors commented Sep 28, 2022

Suggest unwrapping `???<T>` if a method cannot be found on it but is present on `T`. #102288

Suggest unwrapping `???<T>` if a method cannot be found on it but is present on `T`. #102288

mejrs commented Sep 25, 2022 •

edited

Loading

compiler-errors Sep 27, 2022 •

edited

Loading