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

Clippy: Remove TyCtyt::super_traits_of public comment and test behavior #11097

Closed

Conversation

xFrednet
Copy link
Member

@xFrednet xFrednet commented Jul 3, 2023

The TyCtyt::super_traits_of function is actually not needed. This PR removes the reference and adds a test as well :)

changelog: none

…vior

It turns out that the list of predicates used by `ty_is_fn_once_param` already includes super traits. The function would therefore not gain anything from using `TyCtyt::super_traits_of`.

At least this PR removes that todo, reformats the function and adds a related test.
@rustbot
Copy link
Collaborator

rustbot commented Jul 3, 2023

r? @Jarcho

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jul 3, 2023
@Jarcho
Copy link
Contributor

Jarcho commented Jul 5, 2023

That note is still accurate. while_let_on_iterator is working because the closure's kind is FnOnce.

I'm not sure if ty_is_fn_once_param is needed any more. It might be needed to handle cases where the closure is first assigned to a local, but that would have to be implemented first.

@xFrednet
Copy link
Member Author

xFrednet commented Jul 5, 2023

Could you give an example for code that wouldn't work? I tested this (Playground):

    // Lint if `FnOnce` is a super trait
    trait MySpecialFnMut: FnOnce() {}
    impl<T: FnOnce()> MySpecialFnMut for T {}
    fn f4(_: impl MySpecialFnMut) {}
    let mut it = 0..10;
    let x: Box<dyn MySpecialFnMut> = Box::new(|| {
        while let Some(x) = it.next() {
            if x % 2 == 0 {
                break;
            }
        }
    });
    f4(x);

and it lints just fine. I have the feeling I'm just overlooking something 😅

@Jarcho
Copy link
Contributor

Jarcho commented Jul 5, 2023

You're not missing anything. Looks like the closure's kind is now being set based on how it's used rather than how it's capable of being used. I guess the function can be removed then.

@bors
Copy link
Collaborator

bors commented Aug 11, 2023

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

@Jarcho
Copy link
Contributor

Jarcho commented Nov 11, 2023

Ping @xFrednet

ty_is_fn_once_param can just be removed and get_enclosing_loop_or_multi_call_closure can be simplified (is_once will always be false due to the closure kind check right before).

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Nov 11, 2023
@xFrednet
Copy link
Member Author

Hey, year sorry, I wasn't sure if I could just remove the function and then I lost track of it 🙈

I'll close this in favor of #11812, since that includes the suggested change.

@xFrednet xFrednet closed this Nov 19, 2023
@xFrednet xFrednet deleted the 00000-remove-super-trait-comment branch November 19, 2023 12:18
bors added a commit that referenced this pull request Feb 6, 2024
Return `Some` from `walk_to_expr_usage` more

fixes #11786
supersedes #11097

The code removed in the first commit would have needed changes due to the second commit. Since it's useless it just gets removed instead.

changelog: `needless_borrow`: Fix linting in tuple and array expressions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants