rust
rust copied to clipboard
rust-lang
Improve dead code analysis for structs and traits defined locally
This PR does some refactor and improvement on the dead code analysis, and doesn't lint pub structs.
- refactors the two-phase check for impls and impl items of trait
- checks them all later because we must use the trait/trait item and the adt defined locally firstly
- makes the logic cleaner and doesn't require special checks about whether it's public or not
- mark the adt live if it appears in pattern, like generic argument, it also implies the use of the adt
- based on 1 and 2, we can detect unused private adts impl
Default, without adding special logics forDefault - so that we can remove
rustc_trivial_field_readsonDefault, and the logic inshould_ignore_item
- based on 1 and 2, we can detect unused private adts impl
- extends rules to impls for types which refer to adts, like
&Foo/[Foo]things - lints unused assoc consts and assoc tys, like assoc fns
Fixes #120770 Fixes #126729 Fixes #127911 Fixes #128839
cc @pnkfelix r? @compiler-errors
Some changes occurred in tests/ui/sanitizer
cc @rust-lang/project-exploit-mitigations, @rcvalle
The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)
The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)
Compiling rustc_borrowck v0.0.0 (/checkout/compiler/rustc_borrowck)
Compiling rustc_codegen_llvm v0.0.0 (/checkout/compiler/rustc_codegen_llvm)
Compiling rustc_privacy v0.0.0 (/checkout/compiler/rustc_privacy)
Compiling rustc_passes v0.0.0 (/checkout/compiler/rustc_passes)
error: struct `MustUseAsync` is never constructed
|
376 | pub struct MustUseAsync {
| ^^^^^^^^^^^^
|
The job x86_64-gnu-llvm-17 failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)
------
> importing cache manifest from ghcr.io/rust-lang/rust-ci-cache:3aacb9c90579defe09351ac5e8ee504359f8054da6326ff19038f1b7c90e3cb2aafe33685c6d9b76ee8d2ccbd187ca80c46ab5380485abdd8c0ce7d69cd8d8fd:
------
##[endgroup]
Setting extra environment values for docker: --env ENABLE_GCC_CODEGEN=1 --env GCC_EXEC_PREFIX=/usr/lib/gcc/
[CI_JOB_NAME=x86_64-gnu-llvm-17]
---
sccache: Starting the server...
##[group]Configure the build
configure: processing command line
configure:
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-17', '--enable-llvm-link-shared', '--set', 'rust.thin-lto-import-instr-limit=10', '--set', 'change-id=99999999', '--enable-verbose-configure', '--enable-sccache', '--disable-manage-submodules', '--enable-locked-deps', '--enable-cargo-native-static', '--set', 'rust.codegen-units-std=1', '--set', 'dist.compression-profile=balanced', '--dist-compression-formats=xz', '--set', 'rust.lld=false', '--disable-dist-src', '--release-channel=nightly', '--enable-debug-assertions', '--enable-overflow-checks', '--enable-llvm-assertions', '--set', 'rust.verify-llvm-ir', '--set', 'rust.codegen-backends=llvm,cranelift,gcc', '--set', 'llvm.static-libstdcpp', '--enable-new-symbol-mangling']
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-17/bin/llvm-config
configure: llvm.link-shared := True
configure: rust.thin-lto-import-instr-limit := 10
configure: change-id := 99999999
---
Downloaded boml v0.3.1
Compiling boml v0.3.1
Compiling y v0.1.0 (/checkout/compiler/rustc_codegen_gcc/build_system)
Finished `release` profile [optimized] target(s) in 3.74s
Running `/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-codegen/x86_64-unknown-linux-gnu/release/y test --use-system-gcc --use-backend gcc --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/cg_gcc --release --mini-tests --std-tests`
Using system GCC
[BUILD] example
[AOT] mini_core_hello_world
/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/cg_gcc/mini_core_hello_world
abc
Please separate this into separate commits each implementing an individual tweak to the dead code analysis, with the tests adjusted at each commit so I can see the fallout from each change specifically. It's very difficult to map the changes to the UI tests to each code change without that. --- I want to think very hard about each of these changes to determine if there are any false positives that are caused by each change, and that is harder to do with a single commit.
Also, if you want, please open a separate PR that removes the dead code from the compiler/standard library that is now detected after these changes. That can be landed separately.
The job mingw-check failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)
#16 2.845 Building wheels for collected packages: reuse
#16 2.847 Building wheel for reuse (pyproject.toml): started
#16 3.089 Building wheel for reuse (pyproject.toml): finished with status 'done'
#16 3.091 Created wheel for reuse: filename=reuse-4.0.3-cp310-cp310-manylinux_2_35_x86_64.whl size=132715 sha256=dfa09868353292d98f811d3efdb0d54d07389e808efc71d68e3b93c514bf8bec
#16 3.091 Stored in directory: /tmp/pip-ephem-wheel-cache-_t8d3ibb/wheels/3d/8d/0a/e0fc6aba4494b28a967ab5eaf951c121d9c677958714e34532
#16 3.094 Installing collected packages: boolean-py, binaryornot, tomlkit, reuse, python-debian, markupsafe, license-expression, jinja2, chardet, attrs
#16 3.496 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
#16 3.497 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
#16 DONE 3.6s
:umbrella: The latest upstream changes (presumably #128761) made this pull request unmergeable. Please resolve the merge conflicts.
The job mingw-check failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)
#15 2.709 Building wheels for collected packages: reuse
#15 2.710 Building wheel for reuse (pyproject.toml): started
#15 2.959 Building wheel for reuse (pyproject.toml): finished with status 'done'
#15 2.960 Created wheel for reuse: filename=reuse-4.0.3-cp310-cp310-manylinux_2_35_x86_64.whl size=132715 sha256=dfa09868353292d98f811d3efdb0d54d07389e808efc71d68e3b93c514bf8bec
#15 2.960 Stored in directory: /tmp/pip-ephem-wheel-cache-nthjnaws/wheels/3d/8d/0a/e0fc6aba4494b28a967ab5eaf951c121d9c677958714e34532
#15 2.963 Installing collected packages: boolean-py, binaryornot, tomlkit, reuse, python-debian, markupsafe, license-expression, jinja2, chardet, attrs
#15 3.362 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
#15 3.363 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
#15 DONE 3.5s
:umbrella: The latest upstream changes (presumably #129046) made this pull request unmergeable. Please resolve the merge conflicts.
Could you submit 1317d543306ec010d149e374f52c5504a1450ab6 as a separate PR?
The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)
It is probably necessary to make the lint handle associated types, I'd like an explanation why it is necessary
Just detecting more dead codes? I think removing more unused items is helpful. And we have detected unused inherit assoc types, although the feature is unstable for now.
and why we can't just rely on HIR and typeck-output resolutions.
We can only get Res::Err when meeting a assoc type in hir. typeck_results.qpath_res works but it only work for QPaths in an Expr or Pat node. And we may have cases like:
trait TwoWayStrategy {
type Output;
fn use_early_reject() -> bool;
fn rejecting(a: usize, b: usize) -> Self::Output;
fn matching(a: usize, b: usize) -> Self::Output;
}
I asked such things before in zulip
:umbrella: The latest upstream changes (presumably #129595) made this pull request unmergeable. Please resolve the merge conflicts.
:umbrella: The latest upstream changes (presumably #129941) made this pull request unmergeable. Please resolve the merge conflicts.
It is probably necessary to make the lint handle associated types, I'd like an explanation why it is necessary
@cjgillot
Updated: using simpler rules for assoc tys instead of reasoning on types, i.e., only marking assoc tys live only if the corresponding trait is live, which can help to lint more unused traits (is same to #126618 reverted in #128404).
Removes old logic for unused assoc tys. I think the it needs further consideration and could later be made as a separate PR.
The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)
Hi @cjgillot, it looks like this PR is ready for review again when you have time. Thanks 🙂
seems cjgillot hasn't had much time recently, so let me roll the dice
r? compiler
:umbrella: The latest upstream changes (presumably #135272) made this pull request unmergeable. Please resolve the merge conflicts.
I feel like I'm missing a lot of context as to the recent work on dead code lints and the description of the PR doesn't really explain any of it :thinking:
Failed to set assignee to pnkfelix: invalid assignee
Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.
:umbrella: The latest upstream changes (presumably #134248) made this pull request unmergeable. Please resolve the merge conflicts.
:umbrella: The latest upstream changes (presumably #137163) made this pull request unmergeable. Please resolve the merge conflicts.
:umbrella: The latest upstream changes (presumably #137162) made this pull request unmergeable. Please resolve the merge conflicts.