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

Fix incorrect Box::pin suggestion #89390

Merged
merged 5 commits into from
Oct 14, 2021
Merged

Fix incorrect Box::pin suggestion #89390

merged 5 commits into from
Oct 14, 2021

Conversation

tmandry
Copy link
Member

@tmandry tmandry commented Sep 30, 2021

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 #72117.

@rust-highfive
Copy link
Collaborator

r? @cjgillot

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 30, 2021
@rust-log-analyzer

This comment has been minimized.

@tmandry
Copy link
Member Author

tmandry commented Oct 8, 2021

r? @estebank

@rust-highfive rust-highfive assigned estebank and unassigned cjgillot Oct 8, 2021
| | expected `&dyn Trait`, found struct `Box`
| | help: consider borrowing here: `&x`
| ---------- ^ expected `&dyn Trait`, found struct `Box`
| |
Copy link
Contributor

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)

Copy link
Member Author

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.
@tmandry
Copy link
Member Author

tmandry commented Oct 14, 2021

@bors r=estebank

@bors
Copy link
Contributor

bors commented Oct 14, 2021

📌 Commit a8558e9 has been approved by estebank

@bors 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 Oct 14, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 14, 2021
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.
@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 14, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 14, 2021
…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
@bors bors merged commit f9c9774 into rust-lang:master Oct 14, 2021
@rustbot rustbot added this to the 1.57.0 milestone Oct 14, 2021
@tmandry tmandry deleted the issue-72117 branch October 14, 2021 21:32
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 15, 2021
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

confusing diagnostic for incorrect type parameter on BoxFuture
8 participants