Skip to content

Clippy subtree update #139337

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

Closed
wants to merge 184 commits into from
Closed

Conversation

flip1995
Copy link
Member

@flip1995 flip1995 commented Apr 3, 2025

r? @Manishearth

Cargo.lock update due to the Clippy version bump and because Clippy moved from rinja (unmaintained) to askama.

blyxyas and others added 30 commits February 10, 2025 23:32
Introducing a new chapter to the book, known as "Benchmarking Clippy".
It explains the benchmarking capabilities of lintcheck --perf
and gives a concrete example on how benchmark and compare a PR with
master
- Now lintcheck perf deletes target directory after benchmarking,
benchmarking with a cache isn't very useful or telling of any
precise outcome.

- Support for benchmarking several times without having to do
a cargo clean. Now we can benchmark a PR and master (or a single
change in the same commit) without having to move the perf.data files
into an external directory.

- Compress perf.data to allow for allowing multiple stacks and
occupy much less space
By default, lintcheck will use the `clippy.toml` file found at the
toplevel of the repository (`CARGO_MANIFEST_DIR`). This file is meant
for configuration of Clippy applied to Clippy sources.

This creates a new `lintcheck/ci-config/clippy.toml` file which is used
by the CI when running lintcheck. By default this uses the default
Clippy configuration.
A check for `#[non_exhaustive]` is often done in combination with
checking whether the type is local to the crate, in a variety of ways.
Create a helper method and standardize on it as the way to check for
this.
Parameter patterns are lowered to an `Ident` by
`lower_fn_params_to_names`, which is used when lowering bare function
types, trait methods, and foreign functions. Currently, there are two
exceptional cases where the lowered param can become an empty `Ident`.

- If the incoming pattern is an empty `Ident`. This occurs if the
  parameter is anonymous, e.g. in a bare function type.

- If the incoming pattern is neither an ident nor an underscore. Any
  such parameter will have triggered a compile error (hence the
  `span_delayed_bug`), but lowering still occurs.

This commit replaces these empty `Ident` results with `None`, which
eliminates a number of `kw::Empty` uses, and makes it impossible to fail
to check for these exceptional cases.

Note: the `FIXME` comment in `is_unwrap_or_empty_symbol` is removed. It
actually should have been removed in rust-lang#138482, the precursor to this PR.
That PR changed the lowering of wild patterns to `_` symbols instead of
empty symbols, which made the mentioned underscore check load-bearing.
… r=Nadrieril

mir_build: consider privacy when checking for irrefutable patterns

This PR fixes rust-lang#137999.

Note that, since this makes the compiler reject code that was previously accepted, it will probably need a crater run.

I include a commit that factors out a common code pattern into a helper function, purely because the fact that this was repeated all over the place was bothering me. Let me know if I should split that into a separate PR instead.
Add support for postfix yield expressions

We've been having a discussion about whether we want postfix yield, or want to stick with prefix yield, or have both. I figured it's easy enough to support both for now and let us play around with them while the feature is still experimental.

This PR treats `yield x` and `x.yield` as semantically equivalent. There was a suggestion to make `yield x` have a `()` type (so it only works in coroutines with `Resume = ()`. I think that'd be worth trying, either in a later PR, or before this one merges, depending on people's opinions.

rust-lang#43122
…owered-param-names, r=compiler-errors

Use `Option<Ident>` for lowered param names.

Parameter patterns are lowered to an `Ident` by `lower_fn_params_to_names`, which is used when lowering bare function types, trait methods, and foreign functions. Currently, there are two exceptional cases where the lowered param can become an empty `Ident`.

- If the incoming pattern is an empty `Ident`. This occurs if the parameter is anonymous, e.g. in a bare function type.

- If the incoming pattern is neither an ident nor an underscore. Any such parameter will have triggered a compile error (hence the `span_delayed_bug`), but lowering still occurs.

This commit replaces these empty `Ident` results with `None`, which eliminates a number of `kw::Empty` uses, and makes it impossible to fail to check for these exceptional cases.

Note: the `FIXME` comment in `is_unwrap_or_empty_symbol` is removed. It actually should have been removed in rust-lang#138482, the precursor to this PR. That PR changed the lowering of wild patterns to `_` symbols instead of empty symbols, which made the mentioned underscore check load-bearing.

r? ``@compiler-errors``
samueltardieu and others added 21 commits April 1, 2025 19:00
This PR resolves rust-lang#11432 by checking that paths resolve in `disallowed_*`
configurations.

It also does some lightweight validation of definition kinds. For
example, only paths that resolve to `DefKind::Macro` definitions are
allowed in `disallowed_macro` configurations.

~The PR is currently two commits. The first is rust-lang#14376 (cc: @Jarcho),
which I submitted just a few days ago. The second commit is everything
else.~

Side note: For me, the most difficult part of this PR was getting the
spans of the individual `DisallowedPath` configurations. There is some
discussion at toml-rs/toml#840, if
interested.

changelog:  validate paths in `disallowed_*` configurations
This eliminates all methods on `Map`. Actually removing `Map` will occur
in a follow-up PR.
…ompiler-errors

Various local trait item iteration cleanups

Adding a trait impl for `Foo` unconditionally affected all queries that are interested in a completely independent trait `Bar`. Perf has no effect on this. We probably don't have a good perf test for this tho.

r? `@compiler-errors`

I am unsure about rust-lang@9d05efb as it doesn't improve anything wrt incremental, because we still do all the checks for valid `Drop` impls, which subsequently will still invoke many queries and basically keep the depgraph the same.

I want to do

https://github.com/rust-lang/rust/blob/9549077a47099dc826039c051b528d1013740e6f/compiler/rustc_middle/src/ty/trait_def.rs#L141

but would leave that to a follow-up PR, this one changes enough things as it is
Move methods from `Map` to `TyCtxt`, part 5.

This eliminates all methods on `Map`. Actually removing `Map` will occur in a follow-up PR.

A follow-up to rust-lang#137504.

r? `@Zalathar`
r? @flip1995

After seeing some PRs get relabelled I think it's probably for the best
not to, a conflict doesn't mean it's no longer waiting for review after
all

changelog: none
Not all host tools platforms support `f16`/`f128` builtins yet due to
LLVM assertion failures and miscompilations. Until them, Clippy should
avoid using `f16`/`f128` at runtime itself. See
rust-lang#137630.

cc @tgross35

changelog: none
Askama maintenance was handed over to rinja maintainers so new `rinja`
release is actually `askama`. More information
[here](https://blog.guillaume-gomez.fr/articles/2025-03-19+Askama+and+Rinja+merge).

r? @flip1995

changelog: none
Violets are red,
Roses are blue,
We miss our penguin,
This is hard to do.

---

Penguin of the release

![image](https://github.com/user-attachments/assets/4c8031fd-1ce7-4b78-af3e-4af26f07bcf1)

This is not a cat. But nominations are open in the comments for the next
release! 😻

---

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 Apr 3, 2025
@rustbot
Copy link
Collaborator

rustbot commented Apr 3, 2025

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

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.

@rust-log-analyzer
Copy link
Collaborator

The job mingw-check-tidy failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
info: removing rustup binaries
info: rustup is uninstalled
##[group]Image checksum input
mingw-check-tidy
# We use the ghcr base image because ghcr doesn't have a rate limit
# and the mingw-check-tidy job doesn't cache docker images in CI.
FROM ghcr.io/rust-lang/ubuntu:22.04

ARG DEBIAN_FRONTEND=noninteractive
RUN apt-get update && apt-get install -y --no-install-recommends \
  g++ \
  make \
---

COPY host-x86_64/mingw-check/validate-toolstate.sh /scripts/
COPY host-x86_64/mingw-check/validate-error-codes.sh /scripts/

# NOTE: intentionally uses python2 for x.py so we can test it still works.
# validate-toolstate only runs in our CI, so it's ok for it to only support python3.
ENV SCRIPT TIDY_PRINT_DIFF=1 python2.7 ../x.py test \
           --stage 0 src/tools/tidy tidyselftest --extra-checks=py,cpp
#
# This file is autogenerated by pip-compile with Python 3.10
# by the following command:
#
#    pip-compile --allow-unsafe --generate-hashes reuse-requirements.in
---
#12 2.988 Building wheels for collected packages: reuse
#12 2.989   Building wheel for reuse (pyproject.toml): started
#12 3.199   Building wheel for reuse (pyproject.toml): finished with status 'done'
#12 3.200   Created wheel for reuse: filename=reuse-4.0.3-cp310-cp310-manylinux_2_35_x86_64.whl size=132719 sha256=5bb60f62728aaedff7162745ce743c7f2f55069b3e7f82e6a37d70df455797cc
#12 3.200   Stored in directory: /tmp/pip-ephem-wheel-cache-uvidxc0i/wheels/3d/8d/0a/e0fc6aba4494b28a967ab5eaf951c121d9c677958714e34532
#12 3.202 Successfully built reuse
#12 3.203 Installing collected packages: boolean-py, binaryornot, tomlkit, reuse, python-debian, markupsafe, license-expression, jinja2, chardet, attrs
#12 3.603 Successfully installed attrs-23.2.0 binaryornot-0.4.4 boolean-py-4.0 chardet-5.2.0 jinja2-3.1.4 license-expression-30.3.0 markupsafe-2.1.5 python-debian-0.1.49 reuse-4.0.3 tomlkit-0.13.0
#12 3.603 WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv
#12 4.152 Collecting virtualenv
#12 4.208   Downloading virtualenv-20.30.0-py3-none-any.whl (4.3 MB)
#12 4.478      ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 4.3/4.3 MB 16.2 MB/s eta 0:00:00
#12 4.548 Collecting filelock<4,>=3.12.2
#12 4.558   Downloading filelock-3.18.0-py3-none-any.whl (16 kB)
#12 4.599 Collecting platformdirs<5,>=3.9.1
#12 4.610   Downloading platformdirs-4.3.7-py3-none-any.whl (18 kB)
#12 4.634 Collecting distlib<1,>=0.3.7
#12 4.646   Downloading distlib-0.3.9-py2.py3-none-any.whl (468 kB)
#12 4.662      ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 469.0/469.0 KB 32.7 MB/s eta 0:00:00
#12 4.744 Installing collected packages: distlib, platformdirs, filelock, virtualenv
#12 4.929 Successfully installed distlib-0.3.9 filelock-3.18.0 platformdirs-4.3.7 virtualenv-20.30.0
#12 4.929 WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv
#12 DONE 5.0s

#13 [7/8] COPY host-x86_64/mingw-check/validate-toolstate.sh /scripts/
#13 DONE 0.0s
---
DirectMap4k:      120768 kB
DirectMap2M:     7219200 kB
DirectMap1G:    11534336 kB
##[endgroup]
Executing TIDY_PRINT_DIFF=1 python2.7 ../x.py test            --stage 0 src/tools/tidy tidyselftest --extra-checks=py,cpp
+ TIDY_PRINT_DIFF=1 python2.7 ../x.py test --stage 0 src/tools/tidy tidyselftest --extra-checks=py,cpp
##[group]Building bootstrap
    Finished `dev` profile [unoptimized] target(s) in 0.05s
##[endgroup]
WARN: currently no CI rustc builds have rustc debug assertions enabled. Please either set `rust.debug-assertions` to `false` if you want to use download CI rustc or set `rust.download-rustc` to `false`.
[TIMING] core::build_steps::tool::LibcxxVersionTool { target: x86_64-unknown-linux-gnu } -- 0.224
---
[TIMING] core::build_steps::tool::Tidy { compiler: Compiler { stage: 0, host: x86_64-unknown-linux-gnu, forced_compiler: false }, target: x86_64-unknown-linux-gnu } -- 0.000
fmt check
fmt: checked 5939 files
tidy check
Run `./x.py test tidy --bless` to regenerate the list
tidy error: proc-macro crate dependency `winnow` is not registered in `src/bootstrap/src/utils/proc_macro_deps.rs`
tidy error: proc-macro crate dependency `askama_parser` is not registered in `src/bootstrap/src/utils/proc_macro_deps.rs`
tidy: Skipping binary file check, read-only filesystem
removing old virtual environment
creating virtual environment at '/checkout/obj/build/venv' using 'python3.10' and 'venv'
creating virtual environment at '/checkout/obj/build/venv' using 'python3.10' and 'virtualenv'
Requirement already satisfied: pip in ./build/venv/lib/python3.10/site-packages (25.0.1)
linting python files
All checks passed!
checking python file formatting
26 files already formatted
checking C++ file formatting
some tidy checks failed
Command has failed. Rerun with -v to see more details.
Build completed unsuccessfully in 0:01:48
  local time: Thu Apr  3 20:48:02 UTC 2025
  network time: Thu, 03 Apr 2025 20:48:02 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

@Manishearth

This comment was marked as outdated.

@bors
Copy link
Collaborator

bors commented Apr 7, 2025

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

@flip1995 flip1995 closed this Apr 17, 2025
@flip1995 flip1995 deleted the clippy-subtree-update branch April 17, 2025 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.