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

On nightly rustc, E0599 emits a massive diagnostic to suggest wrapping the receiver in every combination of {Box,Pin,Rc,Arc}::new({,&,&mut} expr) #84769

Closed
PatchMixolydic opened this issue May 1, 2021 · 14 comments · Fixed by #84808
Labels
A-diagnostics Area: Messages for errors, warnings, and lints D-verbose Diagnostics: Too much output caused by a single piece of incorrect code. P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Milestone

Comments

@PatchMixolydic
Copy link
Contributor

PatchMixolydic commented May 1, 2021

Given the following code (playground):

trait Bar {}
impl Bar for String {}

fn main() {
  let s = String::from("hey");
  let x: &dyn Bar = &s;
  x.as_ref();
}

The output on 1.53.0-nightly (2021-04-29 478a07df05e3fe8df964) is:

Long diagnostic
error[E0599]: the method `as_ref` exists for reference `&dyn Bar`, but its trait bounds were not satisfied
   --> src/main.rs:7:5
    |
1   | trait Bar {}
    | --------- doesn't satisfy `dyn Bar: AsRef<_>`
...
7   |   x.as_ref();
    |     ^^^^^^ method cannot be called on `&dyn Bar` due to unsatisfied trait bounds
    |
    = note: the following trait bounds were not satisfied:
            `dyn Bar: AsRef<_>`
            which is required by `&dyn Bar: AsRef<_>`
help: consider wrapping the receiver expression with the appropriate type
    |
7   |   Box::new(x).as_ref();
    |   ^^^^^^^^^ ^
help: consider wrapping the receiver expression with the appropriate type
    |
7   |   Pin::new(x).as_ref();
    |   ^^^^^^^^^ ^
help: consider wrapping the receiver expression with the appropriate type
    |
7   |   Arc::new(x).as_ref();
    |   ^^^^^^^^^ ^
help: consider wrapping the receiver expression with the appropriate type
    |
7   |   Rc::new(x).as_ref();
    |   ^^^^^^^^ ^
help: consider wrapping the receiver expression with the appropriate type
    |
7   |   Box::new(&mut x).as_ref();
    |   ^^^^^^^^^^^^^  ^
help: consider wrapping the receiver expression with the appropriate type
    |
7   |   Pin::new(&mut x).as_ref();
    |   ^^^^^^^^^^^^^  ^
help: consider wrapping the receiver expression with the appropriate type
    |
7   |   Arc::new(&mut x).as_ref();
    |   ^^^^^^^^^^^^^  ^
help: consider wrapping the receiver expression with the appropriate type
    |
7   |   Rc::new(&mut x).as_ref();
    |   ^^^^^^^^^^^^  ^
help: consider wrapping the receiver expression with the appropriate type
    |
7   |   Box::new(&x).as_ref();
    |   ^^^^^^^^^^ ^
help: consider wrapping the receiver expression with the appropriate type
    |
7   |   Pin::new(&x).as_ref();
    |   ^^^^^^^^^^ ^
help: consider wrapping the receiver expression with the appropriate type
    |
7   |   Arc::new(&x).as_ref();
    |   ^^^^^^^^^^ ^
help: consider wrapping the receiver expression with the appropriate type
    |
7   |   Rc::new(&x).as_ref();
    |   ^^^^^^^^^ ^

Box, Pin, Arc, and Rc are suggested three times each: once for x, once for &x, and once for &mut x. I'm not sure if this is a problem, but this seems incredibly verbose to me.

Note that the output on stable 1.51 and beta 1.52 is different:

error[E0599]: the method `as_ref` exists for reference `&dyn Bar`, but its trait bounds were not satisfied
 --> src/main.rs:7:5
  |
1 | trait Bar {}
  | --------- doesn't satisfy `dyn Bar: AsRef<_>`
...
7 |   x.as_ref();
  |     ^^^^^^ method cannot be called on `&dyn Bar` due to unsatisfied trait bounds
  |
  = note: the following trait bounds were not satisfied:
          `dyn Bar: AsRef<_>`
          which is required by `&dyn Bar: AsRef<_>`
  = help: items from traits can only be used if the trait is implemented and in scope
  = note: the following trait defines an item `as_ref`, perhaps you need to implement it:
          candidate #1: `AsRef`
@PatchMixolydic PatchMixolydic added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 1, 2021
@PatchMixolydic
Copy link
Contributor Author

PatchMixolydic commented May 1, 2021

My mistake, each repetition is actually different in that two suggest taking a reference to x:

7   |   Box::new(x).as_ref()
7   |   Box::new(&x).as_ref()
7   |   Box::new(&mut x).as_ref()

This still seems like a fairly massive diagnostic, though. I've updated the original post in light of this. Feel free to close if a diagnostic of this size is fine here.

@PatchMixolydic PatchMixolydic changed the title On nightly rustc, E0599 repeatedly suggests wrapping the receiver with four possible receiver types On nightly rustc, E0599 emits a massive diagnostic to suggest wrapping the receiver in every permutation of {Box,Pin,Rc,Arc}::new({,&,&mut} expr) May 1, 2021
@Aaron1011
Copy link
Member

cc @estebank

@PatchMixolydic PatchMixolydic changed the title On nightly rustc, E0599 emits a massive diagnostic to suggest wrapping the receiver in every permutation of {Box,Pin,Rc,Arc}::new({,&,&mut} expr) On nightly rustc, E0599 emits a massive diagnostic to suggest wrapping the receiver in every combination of {Box,Pin,Rc,Arc}::new({,&,&mut} expr) May 1, 2021
@PatchMixolydic
Copy link
Contributor Author

PatchMixolydic commented May 1, 2021

Might be a duplicate of #84272, or at least fixed by the same PR that fixed it

@workingjubilee
Copy link
Member

workingjubilee commented May 1, 2021

Even if it was considered reasonable to suggest each and every possible type combination, there is no pressing reason for it to generate the intervening lines as well: rustc is a compiler, not a can of spam. This remains unpatched on nightly, and is thus a stable-to-beta diagnostic regression.

@workingjubilee workingjubilee added the regression-from-stable-to-beta Performance or correctness regression from stable to beta. label May 1, 2021
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label May 1, 2021
@camelid
Copy link
Member

camelid commented May 1, 2021

This remains unpatched on nightly, and is thus a stable-to-beta diagnostic regression.

AFAICT, this is not present on beta, and is only present on nightly, so it's a stable-to-nightly regression. Please correct me if I misunderstood!

@camelid camelid added regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. and removed regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels May 1, 2021
@steffahn
Copy link
Member

steffahn commented May 1, 2021

searched nightlies: from nightly-2021-01-01 to nightly-2021-04-24
regressed nightly: nightly-2021-04-09
searched commits: from 361bfce to 2e495d2
regressed commit: 2118526

That is:

Rollup of 5 pull requests

Successful merges:

 - #82497 (Fix handling of `--output-format json` flag)
 - #83689 (Add more info for common trait resolution and async/await errors)
 - #83952 (Account for `ExprKind::Block` when suggesting .into() and deref)
 - #83965 (Add Debug implementation for hir::intravisit::FnKind)
 - #83974 (Fix outdated crate names in `rustc_interface::callbacks`)

so it’s #83689. I guess @estebank has already been mentioned.

@rustbot label D-verbose

@rustbot rustbot added the D-verbose Diagnostics: Too much output caused by a single piece of incorrect code. label May 1, 2021
@estebank
Copy link
Contributor

estebank commented May 2, 2021

Yes, yes, I introduced this regression. Mea culpa, everybody. In my defense I keep flip-flopping between being too restrictive and too permissive for new suggestions and someone always ends up getting the short end of the stick. This week, I'm leaning towards "too permissive" and if the suggestion triggers too often, then detect the cases and make it more constrained, because the alternative is that the suggestion isn't shown when it could, and we'll never find out. This is IMO better, as long as regressions are caught in nightly, like it was in this case.</rant>

Anyways, #84808 fixes this.

@JohnTitor
Copy link
Member

Assigning P-high as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.

@JohnTitor JohnTitor added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels May 2, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 4, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 4, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 4, 2021
@pietroalbini
Copy link
Member

This now affects 1.53.0 beta. Updating the labels accordingly.

@pietroalbini pietroalbini added regression-from-stable-to-beta Performance or correctness regression from stable to beta. and removed regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. labels May 4, 2021
@pietroalbini pietroalbini added this to the 1.53.0 milestone May 4, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 4, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 4, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 4, 2021
RalfJung added a commit to RalfJung/rust that referenced this issue May 5, 2021
RalfJung added a commit to RalfJung/rust that referenced this issue May 5, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 5, 2021
RalfJung added a commit to RalfJung/rust that referenced this issue May 5, 2021
@bors bors closed this as completed in 1e89b58 May 5, 2021
@pietroalbini pietroalbini reopened this May 5, 2021
@pietroalbini
Copy link
Member

Still needs to be backported.

workingjubilee pushed a commit to workingjubilee/rustc that referenced this issue May 6, 2021
Mark-Simulacrum pushed a commit to Mark-Simulacrum/rust that referenced this issue May 6, 2021
bors added a commit to rust-lang-ci/rust that referenced this issue May 7, 2021
…ulacrum

[beta] backports

First-ish round of beta backports:

*  [beta] backport for rust-lang#84769 rust-lang#84969
*  [beta] Bump stage0 to production 1.52.0 rust-lang#84994
*  Deduplicate ParamCandidates with the same value except for bound vars rust-lang#84559
@apiraino
Copy link
Contributor

@pietroalbini @Mark-Simulacrum the beta backport for patch #84808 was approved, can this now be closed?

@Mark-Simulacrum
Copy link
Member

I think we should wait until it actually lands onto beta

@estebank
Copy link
Contributor

This was merged in beta already, right? #84996

@Mark-Simulacrum
Copy link
Member

Ah, so it was.

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 D-verbose Diagnostics: Too much output caused by a single piece of incorrect code. P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. 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.