Skip to content

Clippy subtree update #133588

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

Merged
merged 72 commits into from
Nov 29, 2024
Merged

Clippy subtree update #133588

merged 72 commits into from
Nov 29, 2024

Conversation

flip1995
Copy link
Member

y21 and others added 30 commits November 10, 2024 19:58
Closes rust-lang#6947

This changes the lint to allow futures which are not `Send` as a result
of a generic type parameter not having a `Send` bound and only lint
futures that are always `!Send` for any type, which I believe is the
more useful behavior (like the comments in the linked issue explain).

This is still only a heuristic (I'm not sure if there's a more general
way to do this), but it should cover the common cases I could think of
(including the code examples in the linked issue)

changelog: [`future_not_send`]: allow conditional `Send` futures
…ang#13691)

In preparation of rust-lang#13556, I want to remove the dependency on
`clippy_config`, as I don't think that we want to publish that for
outside consumers. To do this the 2 dependecies on `clippy_config` had
to be removed:

1. The MSRV implementation was in `clippy_config`, but was required in
`qualify_min_const`. I think exposing the MSRV infrastructure and the
MSRVs we defined might also be helpful for `clippy_utils` users. I don't
see why it should not be able to live in `clippy_utils` from a technical
point of few.
2. The `create_disallowed_map` function that took in a
`clippy_utils::types::DisallowedPath` is moved to the `DisallowedPath`
implementation. This also fits there and is only useful for Clippy and
not in `clippy_utils` for external consumers.

`clippy_config` now depends in `clippy_utils`, so the dependecy just got
reversed. But having the `clippy_utils` crate as the base of the
dependency tree in Clippy makes sense.

changelog: none
Add support for `#[clippy::format_args]` attribute that can be attached to any macro to indicate that it functions the same as the built-in format macros like `format!`, `println!` and `write!`
Add support for `#[clippy::format_args]` attribute that can be attached
to any macro to indicate that it functions the same as the built-in
format macros like `format!`, `println!` and `write!`

---

changelog: Enhancement: [`format_in_format_args`],
[`recursive_format_impl`], [`to_string_in_format_args`],
[`uninlined_format_args`], [`unused_format_specs`]: Recognizes
`#[clippy::format_args]` to support custom 3rs party format macros.
rust-lang#13698)

Two changes to `redundant_guards`:

- Lint float literals. We used to do that before but that was changed in
rust-lang#11305 because at the time there was a future compat warning and it was
planned to make pattern matching floats a hard error.

In rust-lang#116284 it was decided to actually remove the lint and
only make matching `NAN` specifically a hard error. The `NAN` part isn't
relevant/important here because this PR only changes what literals are
warned and `f64::NAN` isn't a literal, but I've added a test anyway to
make sure we continue to not lint there.
- Don't lint CStr literals because that can't be a pattern right now
(fixes rust-lang#13681)

changelog: none
- Add metadata to clippy_utils/Cargo.toml file
- Add clippy_utils README.md file
…3695)

Close rust-lang#13679

changelog: [`if_let_mutex`]: disable lint from Edition 2024 since
[stabilized if_let_rescope
](rust-lang#131154)
…g#13693)

Follow up to rust-lang/rust-clippy#13691

Adds metadata to the `clippy_utils/Cargo.toml`, which is mostly copied
from the root `Cargo.toml`.
Adds a `README.md` file listing the nightly version `clippy_utils` can
be used with, mentions that there are no stability guarantees and the
license.

The next PR will add automation to update the nightly toolchains in
those files and the versions in the `Cargo.toml`s.

cc rust-lang/rust-clippy#13556

changelog: none
There was some dependence between the different subcommands of clippy_dev. And
this dependence will increased with the introduction of the sync and release
subcommands. This moves the common functions to a `utils` module, to decouple
the other modules.
the behavior of the type system not only depends on the current
assumptions, but also the currentnphase of the compiler. This is
mostly necessary as we need to decide whether and how to reveal
opaque types. We track this via the `TypingMode`.
…ang#13653)

changelog: [`unnecessary_map_or`]: handle `Option::map_or(true, …)`
changelog: [`missing_safety_doc`]: accept uppercase "SAFETY"

In [Oxc](https://github.com/oxc-project/oxc)'s codebase, we try to draw
attention as clearly as possible to the safety constraints of unsafe
code by including an uppercase `# SAFETY` doc comment.

Clippy's `missing_safety_doc` lint does not recognise "SAFETY" in upper
case, so we also need to include `#[expect(clippy::missing_safety_doc)]`
on every unsafe function, to avoid a false positive. Unfortunately this
defeats the purpose of the lint, as if someone later removes the safety
docs, the lint rule does not trigger.

I don't know how common this style of documenting unsafe functions is,
but I don't imagine also supporting `# SAFETY` would disturb other users
who prefer `# Safety`.
On arm64 I get

```
error[E0570]: `"vectorcall"` is not a supported ABI for the current target
```

We don't seem to be doing any ABI specific handling so it seems fine to
just remove this one since there are other tests

changelog: none
changelog: don't consider lifetimes in bounded types unused (fix `extra_unused_lifetimes` FP rust-lang#13578)
…ifetimes` FP) (rust-lang#13583)

Fixes rust-lang#13578

r? @Alexendoo

changelog: don't consider lifetimes in bounded types unused (fix
`extra_unused_lifetimes` FP rust-lang#13578)
Currently this only provides the feature to auto-update the nightly version in
the `rust-toolchain` file and the `clippy_utils/README.md` file. The actual sync
to and from the Rust repo will be added with the move to Josh.
Currently this only provides the feature to auto-update the versions in the
`Cargo.toml` files. With the move to Josh, a command to get beta and stable
release commits will be added.
samueltardieu and others added 15 commits November 25, 2024 23:24
Inline ExprPrecedence::order into Expr::precedence

The representation of expression precedence in rustc_ast has been an obstacle to further improvements in the pretty-printer (continuing from rust-lang#119105 and rust-lang#119427).

Previously the operation of *"does this expression have lower precedence than that one"* (relevant for parenthesis insertion in macro-generated syntax trees) consisted of 3 steps:

1. Convert `Expr` to `ExprPrecedence` using `.precedence()`
2. Convert `ExprPrecedence` to `i8` using `.order()`
3. Compare using `<`

As far as I can guess, the reason for the separation between `precedence()` and `order()` was so that both `rustc_ast::Expr` and `rustc_hir::Expr` could convert as straightforwardly as possible to the same `ExprPrecedence` enum, and then the more finicky logic performed by `order` could be present just once.

The mapping between `Expr` and `ExprPrecedence` was intended to be as straightforward as possible:

```rust
match self.kind {
    ExprKind::Closure(..) => ExprPrecedence::Closure,
    ...
}
```

although there were exceptions of both many-to-one, and one-to-many:

```rust
    ExprKind::Underscore => ExprPrecedence::Path,
    ExprKind::Path(..) => ExprPrecedence::Path,
    ...
    ExprKind::Match(_, _, MatchKind::Prefix) => ExprPrecedence::Match,
    ExprKind::Match(_, _, MatchKind::Postfix) => ExprPrecedence::PostfixMatch,
```

Where the nature of `ExprPrecedence` becomes problematic is when a single expression kind might be associated with multiple different precedence levels depending on context (outside the expression) and contents (inside the expression). For example consider what is the precedence of an ExprKind::Closure `$closure`. Well, on the left-hand side of a binary operator it would need parentheses in order to avoid the trailing binary operator being absorbed into the closure body: `($closure) + Rhs`, so the precedence is something lower than that of `+`. But on the right-hand side of a binary operator, a closure is just a straightforward prefix expression like a unary op, which is a relatively high precedence level, higher than binops but lower than method calls: `Lhs + $closure` is fine without parens but `($closure).method()` needs them. But as a third case, if the closure contains an explicit return type, then the precedence is an even higher level than that, never needing parenthesization even in a binop left-hand side or method call: `|| -> bool { false } + Rhs` or `|| -> bool { false }.method()`.

You can see that trying to capture all of this resolution about expressions into `ExprPrecedence` violates the intention of `ExprPrecedence` being a straightforward one-to-one correspondence from each AST and HIR `ExprKind` variant. It would be possible to attempt that by doing stuff like `ExprPrecedence::Closure(Side::Leading, ReturnType::No)`, but I don't foresee the original envisioned benefit of the `precedence()`/`order()` distinction being retained in this approach. Instead I want to move toward a model that Syn has been using successfully. In Syn, there is a Precedence enum but it differs from rustc in the following ways:

- There are [relatively few variants](https://github.com/dtolnay/syn/blob/2.0.87/src/precedence.rs#L11-L47) compared to rustc's `ExprPrecedence`. For example there is no distinction at the precedence level between returns and closures, or between loops and method calls.

- We distinguish between [leading](https://github.com/dtolnay/syn/blob/2.0.87/src/fixup.rs#L293) and [trailing](https://github.com/dtolnay/syn/blob/2.0.87/src/fixup.rs#L309) precedence, taking into account an expression's context such as what token follows it (for various syntactic bail-outs in Rust's grammar, like ambiguities around break-with-value) and how it relates to operators from the surrounding syntax tree.

- There are no hardcoded mysterious integer quantities like rustc's `PREC_CLOSURE = -40`. All precedence comparisons are performed via PartialOrd on a C-like enum.

This PR is just a first step in these changes. As you can tell from Syn, I definitely think there is value in having a dedicated type to represent precedence, instead of what `order()` is doing with `i8`. But that is a whole separate adventure because rustc_ast doesn't even agree consistently on `i8` being the type for precedence order; `AssocOp::precedence` instead uses `usize` and there are casts in both directions. It is likely that a type called `ExprPrecedence` will re-appear, but it will look substantially different from the one that existed before this PR.
This should address rust-lang#13099 for the `derivable_impls` test. As I've not
contributed to clippy before, I'd like to make sure i'm on the right
track before doing more :)

changelog: [`derivable_impls`]: Use multipart_suggestion to aggregate
feedback
…g#13723)

changelog: [`trait_duplication_in_bounds`]: trigger on duplicate const
associated constraint as well

~~The first commit is part of rust-lang#13722 which must be merged first.~~
`clap-3.1.6` was not compiling on the rustc-perf benchmarks because of
this lint. Also, the user could not allow the lint on the macro itself
because it was checking Exprs, not whatever the input of `bitflags!` is.

```
error: &-masking with zero
   --> src/build/arg_settings.rs:197:1
    |
            (#[allow] here)
197 | / bitflags! {
198 | |     struct Flags: u32 {
199 | |         const REQUIRED         ...
200 | |         const MULTIPLE_OCC     ...
...   |
226 | |     }
227 | | }
    | |_^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#bad_bit_mask
    = note: this error originates in the macro `__impl_bitflags` which comes from the expansion of the macro `bitflags` (in Nightly builds, run with -Z macro-backtrace for more info)
```

changelog:  [`bad_bit_mask`]: Fix FP on procedural macros
This is in honor of `@bors` who is no longer used in this repo. I've
been saving this poem for a special PR, but it just seems fitting to use
it now:

Bors the bot,
Handsome and strong!
Will you go with me,
To prom?

---

### The cat of this release is Abu nominated by @jdonszelmann :

<img height=700
src="https://github.com/user-attachments/assets/414f24a1-8bbf-4fed-bcbc-acc5ca6a1353"
alt="The cats of this Clippy release" />

Cats for the next release can be nominated in the comments :D

---

changelog: none
r? @ghost

changelog: none
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 28, 2024
@rustbot
Copy link
Collaborator

rustbot commented Nov 28, 2024

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@Manishearth
Copy link
Member

@bors r+ p=1 rollup=never

@bors
Copy link
Collaborator

bors commented Nov 29, 2024

📌 Commit 315b47e has been approved by Manishearth

It is now in the queue for this repository.

@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 Nov 29, 2024
@bors
Copy link
Collaborator

bors commented Nov 29, 2024

⌛ Testing commit 315b47e with merge d10a682...

@bors
Copy link
Collaborator

bors commented Nov 29, 2024

☀️ Test successful - checks-actions
Approved by: Manishearth
Pushing d10a682 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 29, 2024
@bors bors merged commit d10a682 into rust-lang:master Nov 29, 2024
7 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Nov 29, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (d10a682): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.9% [0.9%, 0.9%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary -1.2%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-1.2% [-1.2%, -1.2%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -1.2% [-1.2%, -1.2%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 771.899s -> 773.513s (0.21%)
Artifact size: 332.31 MiB -> 332.31 MiB (0.00%)

flip1995 pushed a commit to flip1995/rust that referenced this pull request Dec 15, 2024
…nishearth

Clippy subtree update

r? `@Manishearth`
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.