-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Fix incorrect Box::pin suggestion #89390
Conversation
r? @cjgillot (rust-highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
1673b80
to
68595d4
Compare
r? @estebank |
| | expected `&dyn Trait`, found struct `Box` | ||
| | help: consider borrowing here: `&x` | ||
| ---------- ^ expected `&dyn Trait`, found struct `Box` | ||
| | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have positive tests for these where the type involved can be coerced and the suggestion occurs?
r=me with either an answer or a new test (to avoid regressions there, I'm on the road right now or I'd check myself)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like yep, ui/inference/deref-suggestion
The suggestion checked if Pin<Box<T>> could be coeerced to the expected type, but did not check predicates created by the coercion. We now look for predicates that definitely cannot be satisfied before giving the suggestion. The suggestion is marked MaybeIncorrect because we allow predicates that are still ambiguous and can't be proven.
This only changed two tests and I consider both changes an improvement.
@bors r=estebank |
📌 Commit a8558e9 has been approved by |
Fix incorrect Box::pin suggestion The suggestion checked if `Pin<Box<T>>` could be coeerced to the expected type, but did not check predicates created by the coercion. We now look for predicates that definitely cannot be satisfied before giving the suggestion. The suggestion is still marked MaybeIncorrect because we allow predicates that are still ambiguous and can't be proven. Fixes rust-lang#72117.
…askrgr Rollup of 6 pull requests Successful merges: - rust-lang#89390 (Fix incorrect Box::pin suggestion) - rust-lang#89433 (Fix ctrl-c causing reads of stdin to return empty on Windows.) - rust-lang#89823 (Switch order of terms to prevent overflow) - rust-lang#89865 (Allow static linking LLVM with ThinLTO) - rust-lang#89873 (Add missing word to `FromStr` trait documentation) - rust-lang#89878 (Fix missing remaining compiler specific cfg information) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Suggest Box::pin when Pin::new is used instead This fixes an incorrect diagnostic. **Based on rust-lang#89390**; only the last commit is specific to this PR. "Ignore whitespace changes" also helps here.
The suggestion checked if
Pin<Box<T>>
could be coeerced to the expectedtype, but did not check predicates created by the coercion. We now
look for predicates that definitely cannot be satisfied before giving
the suggestion.
The suggestion is still marked MaybeIncorrect because we allow predicates that
are still ambiguous and can't be proven.
Fixes #72117.