needless_borrows_for_generic_args: Fix for &mut
This commit fixes a bug introduced in #12706, where the behavior of the lint has been changed, to avoid suggestions that introduce a move. The motivation in the commit message is quite poor (if the detection for significant drops is not sufficient because it's not transitive, the proper fix would be to make it transitive). However, #12454, the linked issue, provides a good reason for the change — if the value being borrowed is bound to a variable, then moving it will only introduce friction into future refactorings.
Thus #12706 changes the logic so that the lint triggers if the value being borrowed is Copy, or is the result of a function call, simplifying the logic to the point where analysing "is this the only use of this value" isn't necessary.
However, said PR also introduces an undocumented carveout, where referents that themselves are mutable references are treated as Copy, to catch some cases that we do want to lint against. However, that is not sound — it's possible to consume a mutable reference by moving it.
To avoid emitting false suggestions, this PR reintroduces the referent_used_exactly_once logic and runs that check for referents that are themselves mutable references.
Thinking about the code shape of &mut x, where x: &mut T, raises the point that while removing the &mut outright won't work, the extra indirection is still undesirable, and perhaps instead we should suggest reborrowing: &mut *x. That, however, is left as possible future work.
Fixes#12856
changelog: none
Changelog for Clippy 1.80 🌞
Roses are red,
Violets are blue,
Summer is fun,
So much sun
---
### The cat of this release is *Maunzer* submitted by `@llogiq:`
<img height=500 src="https://github.com/rust-lang/rust-clippy/assets/4200835/a1da6948-446d-4ccf-95a7-c816a8afdc3f" alt="The cats of this Clippy release" />
Cats for the next release can be nominated in the comments :D
---
changelog: none
I wish everyone reading this a beautiful and happy day =^.^=
Use ControlFlow in more places
Now, instead of manually using variables in visitors to signify that a visit is "done" and that the visitor should stop traversing. We use the trait type "Result" to signify this (in relevant places).
I'll schedule a perf run, I don't think it will be much of a difference, but every bit of performance is welcomed :)
changelog: Improve performance, less memory use in visitors
Fixes#12829
r? `@y21`
Make ast `MutVisitor` have the same method name and style as `Visitor`
It doesn't map 100% because some `MutVisitor` methods can filter or even expand to multiple items, but consistency seems nicer.
tracking issue: https://github.com/rust-lang/rust/issues/127615
Misc refactorings
Various small re-orderings to check the HIR tree or AST before doing other checks. Also includes a small bug fix for `arc_with_small_send_sync` not actually checking for `Arc::new`.
changelog: none
Misc refactorings part 5
`toplevel_ref_arg` gets a small fix so it can be allowed on function arguments. Otherwise just some rearrangements.
changelog: none
add lint for recreation of an entire struct
This lint makes Clippy warn about situations where an owned struct is
essentially recreated by moving all its fields into a new instance of
the struct. The lint is not machine-applicable because the source
struct may have been partially moved.
This lint originated in something I spotted during peer review. While
working on their branch a colleague ended up with a commit where a
function returned a struct that 1:1 replicated one of its owned inputs
from its members. Initially I suspected they hadn’t run their code
through Clippy but AFAICS there is no lint for this situation yet.
changelog: new lint: [`redundant_owned_struct_recreation`]
### New lint checklist
- \[+] Followed [lint naming conventions][lint_naming]
- \[+] Added passing UI tests (including committed `.stderr` file)
- \[+] `cargo test` passes locally
- \[+] Executed `cargo dev update_lints`
- \[+] Added lint documentation
- \[+] Run `cargo dev fmt`
Remove unnecessary impl sorting in queries and metadata
Removes unnecessary impl sorting because queries already return their keys in HIR definition order: https://github.com/rust-lang/rust/issues/120371#issuecomment-1926422838
r? `@cjgillot` or `@lcnr` -- unless I totally misunderstood what was being asked for here? 😆fixes#120371
[`pathbuf_init_then_push`]: Checks for calls to `push` immediately a…
changelog: [`pathbuf_init_then_push`]: new lint: Checks for calls to `push` immediately after creating a new `PathBuf`
Just a mirror of VEC_INIT_THEN_PUSH
Represent type-level consts with new-and-improved `hir::ConstArg`
### Summary
This is a step toward `min_generic_const_exprs`. We now represent all const
generic arguments using an enum that differentiates between const *paths*
(temporarily just bare const params) and arbitrary anon consts that may perform
computations. This will enable us to cleanly implement the `min_generic_const_args`
plan of allowing the use of generics in paths used as const args, while
disallowing their use in arbitrary anon consts. Here is a summary of the salient
aspects of this change:
- Add `current_def_id_parent` to `LoweringContext`
This is needed to track anon const parents properly once we implement
`ConstArgKind::Path` (which requires moving anon const def-creation
outside of `DefCollector`).
- Create `hir::ConstArgKind` enum with `Path` and `Anon` variants. Use it in the
existing `hir::ConstArg` struct, replacing the previous `hir::AnonConst` field.
- Use `ConstArg` for all instances of const args. Specifically, use it instead
of `AnonConst` for assoc item constraints, array lengths, and const param
defaults.
- Some `ast::AnonConst`s now have their `DefId`s created in
rustc_ast_lowering rather than `DefCollector`. This is because in some
cases they will end up becoming a `ConstArgKind::Path` instead, which
has no `DefId`. We have to solve this in a hacky way where we guess
whether the `AnonConst` could end up as a path const since we can't
know for sure until after name resolution (`N` could refer to a free
const or a nullary struct). If it has no chance as being a const
param, then we create a `DefId` in `DefCollector` -- otherwise we
decide during ast_lowering. This will have to be updated once all path
consts use `ConstArgKind::Path`.
- We explicitly use `ConstArgHasType` for array lengths, rather than
implicitly relying on anon const type feeding -- this is due to the
addition of `ConstArgKind::Path`.
- Some tests have their outputs changed, but the changes are for the
most part minor (including removing duplicate or almost-duplicate
errors). One test now ICEs, but it is for an incomplete, unstable
feature and is now tracked at https://github.com/rust-lang/rust/issues/127009.
### Followup items post-merge
- Use `ConstArgKind::Path` for all const paths, not just const params.
- Fix (no github dont close this issue) #127009
- If a path in generic args doesn't resolve as a type, try to resolve as a const
instead (do this in rustc_resolve). Then remove the special-casing from
`rustc_ast_lowering`, so that all params will automatically be lowered as
`ConstArgKind::Path`.
- (?) Consider making `const_evaluatable_unchecked` a hard error, or at least
trying it in crater
r? `@BoxyUwU`
Lint `zero_repeat_side_effects` only if array length is a literal zero
changelog: [`zero_repeat_side_effects` ] Lint only if array length is a literal zero
Fixes#13110 .
r? y21
* Construct lint passes by taking `Conf` by reference.
* Use `HashSet` configs in less places
* Move some `check_crate` code into the pass constructor when possible.
This is a very large commit since a lot needs to be changed in order to
make the tests pass. The salient changes are:
- `ConstArgKind` gets a new `Path` variant, and all const params are now
represented using it. Non-param paths still use `ConstArgKind::Anon`
to prevent this change from getting too large, but they will soon use
the `Path` variant too.
- `ConstArg` gets a distinct `hir_id` field and its own variant in
`hir::Node`. This affected many parts of the compiler that expected
the parent of an `AnonConst` to be the containing context (e.g., an
array repeat expression). They have been changed to check the
"grandparent" where necessary.
- Some `ast::AnonConst`s now have their `DefId`s created in
rustc_ast_lowering rather than `DefCollector`. This is because in some
cases they will end up becoming a `ConstArgKind::Path` instead, which
has no `DefId`. We have to solve this in a hacky way where we guess
whether the `AnonConst` could end up as a path const since we can't
know for sure until after name resolution (`N` could refer to a free
const or a nullary struct). If it has no chance as being a const
param, then we create a `DefId` in `DefCollector` -- otherwise we
decide during ast_lowering. This will have to be updated once all path
consts use `ConstArgKind::Path`.
- We explicitly use `ConstArgHasType` for array lengths, rather than
implicitly relying on anon const type feeding -- this is due to the
addition of `ConstArgKind::Path`.
- Some tests have their outputs changed, but the changes are for the
most part minor (including removing duplicate or almost-duplicate
errors). One test now ICEs, but it is for an incomplete, unstable
feature and is now tracked at #127009.
