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

Respect MSRV (MAYBE_BOUND_IN_WHERE) #12388

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

sanxiyn
Copy link
Member

@sanxiyn sanxiyn commented Feb 29, 2024

Fix #12370.

changelog: [multiple_bound_locations]: respect MSRV for combining ?Sized bound

@rustbot
Copy link
Collaborator

rustbot commented Feb 29, 2024

r? @y21

rustbot has assigned @y21.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Feb 29, 2024
Copy link
Member

@y21 y21 left a comment

Choose a reason for hiding this comment

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

+1 for having this be part of the trait_bounds pass, fits nicely with the other lints, and also nice seeing the cannot_combine_maybe_bound function get some use

clippy_lints/src/trait_bounds.rs Outdated Show resolved Hide resolved
clippy_lints/src/trait_bounds.rs Outdated Show resolved Hide resolved
{
}
}

Copy link
Member

Choose a reason for hiding this comment

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

Can we have a test case for something like

fn f<'a, a: Sized>() where 'a: 'static {}

where the type parameter is named after a lifetime? I think this currently happens to work (ie not get linted) because the name of lifetimes include the quote, which differentiates them, but it would be good to have this as a guarantee in the testsuite.

clippy_lints/src/trait_bounds.rs Outdated Show resolved Hide resolved
@@ -99,10 +126,11 @@ impl TraitBounds {
}
}

impl_lint_pass!(TraitBounds => [TYPE_REPETITION_IN_BOUNDS, TRAIT_DUPLICATION_IN_BOUNDS]);
impl_lint_pass!(TraitBounds => [TYPE_REPETITION_IN_BOUNDS, TRAIT_DUPLICATION_IN_BOUNDS, MULTIPLE_BOUND_LOCATIONS]);

impl<'tcx> LateLintPass<'tcx> for TraitBounds {
fn check_generics(&mut self, cx: &LateContext<'tcx>, gen: &'tcx Generics<'_>) {
Copy link
Member

Choose a reason for hiding this comment

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

Since we now not only lint on functions, but also impls and a bunch of other items, I wonder how this interacts with derive macros such as Debug. Would be good to have a test, to make sure it doesn't lint here:

#[derive(Debug)]
struct S<T> where T: Sized { t: T }

@y21
Copy link
Member

y21 commented Feb 29, 2024

Hmm, looking more closely into the lints covered by this pass... Doesn't the type_repetition_in_bounds lint basically cover everything that this lint catches, but more thorough (as in, will also catch regular types and not just type parameters, though it is in the nursery category)?
Not necessarily related to what this PR addresses though, so that's for another time ^^

@y21 y21 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 Mar 5, 2024
@bors
Copy link
Collaborator

bors commented Mar 5, 2024

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

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.

multiple_bound_locations false positive: msrv < 1.15
4 participants