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

Remove invalid suggestion involving Fn trait bound #86400

Merged
merged 1 commit into from
Aug 3, 2021

Conversation

FabianWolff
Copy link
Contributor

This pull request closes #85735. The actual issue is a duplicate of #21974, but #85735 contains a further problem, which is an invalid suggestion if Fn/FnMut/FnOnce trait bounds are involved: The suggestion code checks whether the trait bound ends with > to determine whether it has any generic arguments, but the Fn* traits have a special syntax for generic arguments that doesn't involve angle brackets. The example given in #85735:

trait Foo {}
impl<'a, 'b, T> Foo for T
where
    T: FnMut(&'a ()),
    T: FnMut(&'b ()), {

    }

currently produces:

error[E0283]: type annotations needed
   --> src/lib.rs:4:8
    |
4   |       T: FnMut(&'a ()),
    |          ^^^^^^^^^^^^^ cannot infer type for type parameter `T`
    |
    = note: cannot satisfy `T: FnMut<(&'a (),)>`
help: consider specifying the type arguments in the function call
    |
4   |     T: FnMut(&'a ())::<Self, Args>,
    |                     ^^^^^^^^^^^^^^

error: aborting due to previous error

which is incorrect, because there is no function call, and applying the suggestion would lead to a parse error. With my changes, I get:

error[E0283]: type annotations needed
   --> test.rs:4:8
    |
4   |     T: FnMut(&'a ()),
    |        ^^^^^^^^^^^^^ cannot infer type for type parameter `T`
    | 
   ::: [...]/library/core/src/ops/function.rs:147:1
    |
147 | pub trait FnMut<Args>: FnOnce<Args> {
    | ----------------------------------- required by this bound in `FnMut`
    |
    = note: cannot satisfy `T: FnMut<(&'a (),)>`

error: aborting due to previous error

i.e. I have added a check to prevent the invalid suggestion from being issued for Fn* bounds, while the underlying issue #21974 remains for now.

@rust-highfive
Copy link
Collaborator

r? @LeSeulArtichaut

(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 Jun 17, 2021
Copy link
Contributor

@LeSeulArtichaut LeSeulArtichaut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, but I don't feel too comfortable approving, so I'll delegate to @estebank. Left two potential nitpicks

@LeSeulArtichaut
Copy link
Contributor

r? @estebank

@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 6, 2021
@crlf0710 crlf0710 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 24, 2021
@bors
Copy link
Contributor

bors commented Aug 3, 2021

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

Copy link
Contributor

@estebank estebank left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me after addressing @LeSeulArtichaut's comments.

@LeSeulArtichaut
Copy link
Contributor

@bors r=estebank

@bors
Copy link
Contributor

bors commented Aug 3, 2021

📌 Commit f8c10ff 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 Aug 3, 2021
@bors
Copy link
Contributor

bors commented Aug 3, 2021

⌛ Testing commit f8c10ff with merge a6ece56...

@bors
Copy link
Contributor

bors commented Aug 3, 2021

☀️ Test successful - checks-actions
Approved by: estebank
Pushing a6ece56 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 3, 2021
@bors bors merged commit a6ece56 into rust-lang:master Aug 3, 2021
@rustbot rustbot added this to the 1.56.0 milestone Aug 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

odd diagnostic with multiple where clauses that only differ by lifetimes
8 participants