Currently some `Allocation`s are interned, some are not, and it's very
hard to tell at a use point which is which.
This commit introduces `ConstAllocation` for the known-interned ones,
which makes the division much clearer. `ConstAllocation::inner()` is
used to get the underlying `Allocation`.
In some places it's natural to use an `Allocation`, in some it's natural
to use a `ConstAllocation`, and in some places there's no clear choice.
I've tried to make things look as nice as possible, while generally
favouring `ConstAllocation`, which is the type that embodies more
information. This does require quite a few calls to `inner()`.
The commit also tweaks how `PartialOrd` works for `Interned`. The
previous code was too clever by half, building on `T: Ord` to make the
code shorter. That caused problems with deriving `PartialOrd` and `Ord`
for `ConstAllocation`, so I changed it to build on `T: PartialOrd`,
which is slightly more verbose but much more standard and avoided the
problems.
This internal lint checks if the `extract_msrv_attrs!` macro is used if
a lint has a MSRV. If not, it suggests to add this attribute to the lint
pass implementation.
Specifically, rename the `Const` struct as `ConstS` and re-introduce `Const` as
this:
```
pub struct Const<'tcx>(&'tcx Interned<ConstS>);
```
This now matches `Ty` and `Predicate` more closely, including using
pointer-based `eq` and `hash`.
Notable changes:
- `mk_const` now takes a `ConstS`.
- `Const` was copy, despite being 48 bytes. Now `ConstS` is not, so need a
we need separate arena for it, because we can't use the `Dropless` one any
more.
- Many `&'tcx Const<'tcx>`/`&Const<'tcx>` to `Const<'tcx>` changes
- Many `ct.ty` to `ct.ty()` and `ct.val` to `ct.val()` changes.
- Lots of tedious sigil fiddling.
Specifically, change `Ty` from this:
```
pub type Ty<'tcx> = &'tcx TyS<'tcx>;
```
to this
```
pub struct Ty<'tcx>(Interned<'tcx, TyS<'tcx>>);
```
There are two benefits to this.
- It's now a first class type, so we can define methods on it. This
means we can move a lot of methods away from `TyS`, leaving `TyS` as a
barely-used type, which is appropriate given that it's not meant to
be used directly.
- The uniqueness requirement is now explicit, via the `Interned` type.
E.g. the pointer-based `Eq` and `Hash` comes from `Interned`, rather
than via `TyS`, which wasn't obvious at all.
Much of this commit is boring churn. The interesting changes are in
these files:
- compiler/rustc_middle/src/arena.rs
- compiler/rustc_middle/src/mir/visit.rs
- compiler/rustc_middle/src/ty/context.rs
- compiler/rustc_middle/src/ty/mod.rs
Specifically:
- Most mentions of `TyS` are removed. It's very much a dumb struct now;
`Ty` has all the smarts.
- `TyS` now has `crate` visibility instead of `pub`.
- `TyS::make_for_test` is removed in favour of the static `BOOL_TY`,
which just works better with the new structure.
- The `Eq`/`Ord`/`Hash` impls are removed from `TyS`. `Interned`s impls
of `Eq`/`Hash` now suffice. `Ord` is now partly on `Interned`
(pointer-based, for the `Equal` case) and partly on `TyS`
(contents-based, for the other cases).
- There are many tedious sigil adjustments, i.e. adding or removing `*`
or `&`. They seem to be unavoidable.
The to_string_in_display lint is renamed to recursive_format_impl
A check is added for the use of self formatted with Display or Debug
inside any format string in the same impl
The to_string_in_display check is kept as is - like in the
format_in_format_args lint
For now only Display and Debug are checked
This could also be extended to other Format traits (Binary, etc.)
Factor out several utils, add `path_def_id`
changelog: none
This is generally an effort to reduce the total number of utils. `path_def_id` is added which I believe is more "cross-cutting" and also complements `path_to_local`. Best reviewed one commit at a time.
Added:
* `path_def_id`
* `path_res`
Removed:
* `is_qpath_def_path`
* `match_any_diagnostic_items`
* `expr_path_res`
* `single_segment_path`
* `differing_macro_contexts`
* `is_ty_param_lang_item`
* `is_ty_param_diagnostic_item`
* `get_qpath_generics`
Renamed:
* `path_to_res` to `def_path_res`
* `get_qpath_generic_tys` to `qpath_generic_tys`
CC `@Jarcho` since this relates to some of your work and you may have input.
by using an opaque type obligation to bubble up comparisons between opaque types and other types
Also uses proper obligation causes so that the body id works, because out of some reason nll uses body ids for logic instead of just diagnostics.
Create `core::fmt::ArgumentV1` with generics instead of fn pointer
Split from (and prerequisite of) #90488, as this seems to have perf implication.
`@rustbot` label: +T-libs
Fix `needless_borrow` causing mutable borrows to be moved
fixes#8191
changelog: Fix `needless_borrow` causing mutable borrows to be moved
changelog: Rename `ref_in_deref` to `needless_borrow`
changelog: Suggest removing the borrow on method call receivers in `needless_borrow`
Check usages in `ptr_arg`
fixes#214fixes#1981fixes#3381fixes#6406fixes#6964
This does not take into account the return type of the function currently, so `(&Vec<_>) -> &Vec<_>` functions may still be false positives.
The name given for the type also has to match the real type name, so `type Foo = Vec<u32>` won't trigger the lint, but `type Vec = Vec<u32>` will. I'm not sure if this is the best way to handle this, or if a note about the actual type should be added instead.
changelog: Check if the argument is used in a way which requires the original type in `ptr_arg`
changelog: Lint mutable references in `ptr_arg`
Replace `NestedVisitorMap` with generic `NestedFilter`
This is an attempt to make the `intravisit::Visitor` API simpler and "more const" with regard to nested visiting.
With this change, `intravisit::Visitor` does not visit nested things by default, unless you specify `type NestedFilter = nested_filter::OnlyBodies` (or `All`). `nested_visit_map` returns `Self::Map` instead of `NestedVisitorMap<Self::Map>`. It panics by default (unreachable if `type NestedFilter` is omitted).
One somewhat trixty thing here is that `nested_filter::{OnlyBodies, All}` live in `rustc_middle` so that they may have `type Map = map::Map` and so that `impl Visitor`s never need to specify `type Map` - it has a default of `Self::NestedFilter::Map`.
Remove deprecated LLVM-style inline assembly
The `llvm_asm!` was deprecated back in #87590 1.56.0, with intention to remove
it once `asm!` was stabilized, which already happened in #91728 1.59.0. Now it
is time to remove `llvm_asm!` to avoid continued maintenance cost.
Closes#70173.
Closes#92794.
Closes#87612.
Closes#82065.
cc `@rust-lang/wg-inline-asm`
r? `@Amanieu`
Closure capture cleanup & refactor
Follow up of #89648
Each commit is self-contained and the rationale/changes are documented in the commit message, so it's advisable to review commit by commit.
The code is significantly cleaner (at least IMO), but that could have some perf implication, so I'd suggest a perf run.
r? `@wesleywiser`
cc `@arora-aman`
* Track the argument when used to initialize simple `let` bindings
* Check if the argument is passed to a function requiring the original type
* Use `multipart_suggestion` rather than multiple suggestions
* Check if the name given in the source code matches the name of the actual type
Some test code cleanup
changelog: none
Mainly moves /clippy_workspace_tests into /tests and combines the two dogfood tests which can't run concurrently.
Region info is completely unnecessary for upvar capture kind computation
and is only needed to create the final upvar tuple ty. Doing so makes
creation of UpvarCapture very cheap and expose further cleanup opportunity.
Fix `type_repetition_in_bounds`
fixes#7360fixes#8162fixes#8056
changelog: Check for full equality in `type_repetition_in_bounds` rather than just equal hashes
Remove in_macro from clippy_utils
changelog: none
Previously done in #7897 but reverted in #8170. I'd like to keep `in_macro` out of utils because if a span is from expansion in any way (desugaring or macro), we should not proceed without understanding the nature of the expansion IMO.
r? `@llogiq`
New macro utils
changelog: none
Sorry, this is a big one. A lot of interrelated changes and I wanted to put the new utils to use to make sure they are somewhat battle-tested. We may want to divide some of the lint-specific refactoring commits into batches for smaller reviewing tasks. I could also split into more PRs.
Introduces a bunch of new utils at `clippy_utils::macros::...`. Please read through the docs and give any feedback! I'm happy to introduce `MacroCall` and various functions to retrieve an instance. It feels like the missing puzzle piece. I'm also introducing `ExpnId` from rustc as "useful for Clippy too". `@rust-lang/clippy`
Fixes#7843 by not parsing every node of macro implementations, at least the major offenders.
I probably want to get rid of `is_expn_of` at some point.
changelog: none
Sorry, this is a big one. A lot of interrelated changes and I wanted to put the new utils to use to make sure they are somewhat battle-tested. We may want to divide some of the lint-specific refactoring commits into batches for smaller reviewing tasks. I could also split into more PRs.
Introduces a bunch of new utils at `clippy_utils::macros::...`. Please read through the docs and give any feedback! I'm happy to introduce `MacroCall` and various functions to retrieve an instance. It feels like the missing puzzle piece. I'm also introducing `ExpnId` from rustc as "useful for Clippy too". `@rust-lang/clippy`
Fixes#7843 by not parsing every node of macro implementations, at least the major offenders.
I probably want to get rid of `is_expn_of` at some point.
Remove `NullOp::Box`
Follow up of #89030 and MCP rust-lang/compiler-team#460.
~1 month later nothing seems to be broken, apart from a small regression that #89332 (1aac85bb716c09304b313d69d30d74fe7e8e1a8e) shows could be regained by remvoing the diverging path, so it shall be safe to continue and remove `NullOp::Box` completely.
r? `@jonas-schievink`
`@rustbot` label T-compiler
Clippy helpfully warns about code like this, telling you that you
probably meant "write_all":
fn say_hi<W:Write>(w: &mut W) {
w.write(b"hello").unwrap();
}
This patch attempts to extend the lint so it also covers this
case:
async fn say_hi<W:AsyncWrite>(w: &mut W) {
w.write(b"hello").await.unwrap();
}
(I've run into this second case several times in my own programming,
and so have my coworkers, so unless we're especially accident-prone
in this area, it's probably worth addressing?)
This patch covers the Async{Read,Write}Ext traits in futures-rs,
and in tokio, since both are quite widely used.
changelog: [`unused_io_amount`] now supports AsyncReadExt and AsyncWriteExt.
Limit the ``[`identity_op`]`` lint to integral operands.
changelog: limit ``[`identity_op`]`` to integral operands
In the ``[`identity_op`]`` lint, if the operands are non-integers, then the lint is likely
wrong.
Fix `enum_variants` FP on prefixes that are not camel-case
closes#8090
Fix FP on `enum_variants` when prefixes are only a substring of a camel-case word. Also adds some util helpers on `str_utils` to help parsing camel-case strings.
This changes how the lint behaves:
1. previously if the Prefix is only a length of 1, it's going to get ignored, i.e. these were previously ignored and now is warned
```rust
enum Foo {
cFoo,
cBar,
cBaz,
}
enum Something {
CCall,
CCreate,
CCryogenize,
}
```
2. non-ascii characters that doesn't have casing will not be split,
```rust
enum NonCaps {
PrefixXXX,
PrefixTea,
PrefixCake,
}
```
will be considered as `PrefixXXX`, `Prefix`, `Prefix`, so this won't lint as opposed to fired previously.
changelog: [`enum_variant_names`] Fix FP when first prefix are only a substring of a camel-case word.
---
(Edited by `@xFrednet` removed some non ascii characters)
cache test item names
This avoids quadratic behavior (collecting all test item names for each `eq_op` instance within the module). However, it invests a good deal of memory to buy this speedup. If that becomes a problem, I may need to change the cache to only store the chain of last visited modules.
This hopefully fixes#8171.
---
changelog: none
This makes sure that the tests in clippy_utils are run in CI.
When looking into this I discovered that two tests were failing and
multiple doc tests were failing. This fixes those tests and enables a
few more doc tests.
Remove `SymbolStr`
This was originally proposed in https://github.com/rust-lang/rust/pull/74554#discussion_r466203544. As well as removing the icky `SymbolStr` type, it allows the removal of a lot of `&` and `*` occurrences.
Best reviewed one commit at a time.
r? `@oli-obk`
Implement let-else type annotations natively
Tracking issue: #87335Fixes#89688, fixes#89807, edit: fixes #89960 as well
As explained in https://github.com/rust-lang/rust/issues/89688#issuecomment-940405082, the previous desugaring moved the let-else scrutinee into a dummy variable, which meant if you wanted to refer to it again in the else block, it had moved.
This introduces a new hir type, ~~`hir::LetExpr`~~ `hir::Let`, which takes over all the fields of `hir::ExprKind::Let(...)` and adds an optional type annotation. The `hir::Let` is then treated like a `hir::Local` when type checking a function body, specifically:
* `GatherLocalsVisitor` overrides a new `Visitor::visit_let_expr` and does pretty much exactly what it does for `visit_local`, assigning a local type to the `hir::Let` ~~(they could be deduplicated but they are right next to each other, so at least we know they're the same)~~
* It reuses the code in `check_decl_local` to typecheck the `hir::Let`, simply returning 'bool' for the expression type after doing that.
* ~~`FnCtxt::check_expr_let` passes this local type in to `demand_scrutinee_type`, and then imitates check_decl_local's pattern checking~~
* ~~`demand_scrutinee_type` (the blindest change for me, please give this extra scrutiny) uses this local type instead of of creating a new one~~
* ~~Just realised the `check_expr_with_needs` was passing NoExpectation further down, need to pass the type there too. And apparently this Expectation API already exists.~~
Some other misc notes:
* ~~Is the clippy code supposed to be autoformatted? I tried not to give huge diffs but maybe some rustfmt changes simply haven't hit it yet.~~
* in `rustc_ast_lowering/src/block.rs`, I noticed some existing `self.alias_attrs()` calls in `LoweringContext::lower_stmts` seem to be copying attributes from the lowered locals/etc to the statements. Is that right? I'm new at this, I don't know.
By changing `as_str()` to take `&self` instead of `self`, we can just
return `&str`. We're still lying about lifetimes, but it's a smaller lie
than before, where `SymbolStr` contained a (fake) `&'static str`!
Stabilize `iter::zip`
Hello all!
As the tracking issue (#83574) for `iter::zip` completed the final commenting period without any concerns being raised, I hereby submit this stabilization PR on the issue.
As the pull request that introduced the feature (#82917) states, the `iter::zip` function is a shorter way to zip two iterators. As it's generally a quality-of-life/ergonomic improvement, it has been integrated into the codebase without any trouble, and has been
used in many places across the rust compiler and standard library since March without any issues.
For more details, I would refer to `@cuviper's` original PR, or the [function's documentation](https://doc.rust-lang.org/std/iter/fn.zip.html).
fix clippy format using `cargo fmt -p clippy_{lints,utils}`
manually revert rustfmt line truncations
rename to hir::Let in clippy
Undo the shadowing of various `expr` variables after renaming `scrutinee`
reduce destructuring of hir::Let to avoid `expr` collisions
cargo fmt -p clippy_{lints,utils}
bless new clippy::author output
Add new lint to warn when #[must_use] attribute should be used on a method
This lint is somewhat similar to https://rust-lang.github.io/rust-clippy/master/index.html#must_use_candidate but also different: it emits a warning by default and only targets methods (so not functions nor associated functions).
Someone suggested it to me after this tweet: https://twitter.com/m_ou_se/status/1466439813230477312
I think it would reduce the number of cases of API misuses quite a lot.
What do you think?
---
changelog: Added new [`return_self_not_must_use`] lint
Fix `any()` not taking reference in `search_is_some` lint
`find` gives reference to the item, but `any` does not, so suggestion is broken in some specific cases.
Fixes: #7392
changelog: [`search_is_some`] Fix suggestion for `any()` not taking item by reference
Improve `strlen_on_c_string`
fixes: #7436
changelog: lint `strlen_on_c_string` when used without a fully-qualified path
changelog: suggest removing the surrounding unsafe block for `strlen_on_c_string` when possible
Support suggestion for #7854
I think the detection of parking_lot's mutex and rwlock is valuable, so submit this pr, please help judge and review, thank you.
Make let_underscore_lock support parking_lot.(Fixes#7854)
changelog: Make let_underscore_lock support parking_lot
I think the detection of parking_lot's mutex and rwlock is valuable, so submit this pr, please help judge and review, thank you.
Make let_underscore_lock support parking_lot.
changelog: Make let_underscore_lock support parking_lot
Lint for bool to integer casts in `cast_lossless`
The lint description says
> Checks for casts between *numerical* types that may be replaced by safe conversion functions.
Which is strictly speaking being violated here, but it seems within the spirit of the lint. I think it is still a useful lint to have, and having a different lint for just this feels excessive. Thoughts?
Fixes#7947
changelog: Lint for bool to integer casts in [`cast_lossless`]
* Finding pattern slices for `avoidable_slice_indexing`
* `avoidable_slice_indexing` analysing slice usage
* Add configuration to `avoidable_slice_indexing`
* Emitting `avoidable_slice_indexing` with suggestions
* Dogfooding and fixing bugs
* Add ui-toml test for `avoidable_slice_indexing`
* Correctly suggest `ref` keywords for `avoidable_slice_indexing`
* Test and document `mut` for `avoid_slice_indexing`
* Handle macros with `avoidable_slice_indexing` lint
* Ignore slices with sub patterns in `avoidable_slice_indexing`
* Update lint description for `avoidable_slice_indexing`
* Move `avoidable_slice_indexing` to nursery
* Added more tests for `avoidable_slice_indexing`
* Update documentation and message for `avoidable_slice_indexing`
* Teach `avoidable_slice_indexing` about `HirId`s and `Visitors`
* Rename lint to `index_refutable_slice` and connected config
Add Clippy version to Clippy's lint list
Hey, hey, the semester is finally over, and I wanted to get back into hacking on Clippy. It has also been some time since our metadata collection monster has been feed. So, this PR adds a new attribute `clippy::version` to document which version a lint was stabilized. I considered using `git blame` but that would be very hacky and probably not accurate.
I'm also thinking that this attribute can be used to have a `clippy::nightly` lint group which is allow-by-default that delays setting the actual lint group until the defined version is reached. Just something to consider regarding #6623🙃
This PR only adds the version to 4 lints to keep it reviewable. I'll do a followup PR to add the version to other lints if the implementation is accepted 🙃
Also, mobile approved xD
---
r? `@flip1995`
cc: #7172closes: #6492
changelog: [Clippy's lint list](https://rust-lang.github.io/rust-clippy/master/index.html) now displays the version a lint was added. 🎉
---
Example lint declaration after this update:
```rs
declare_clippy_lint! {
/// [...]
///
/// ### Example
/// ```rust
/// // Bad
/// let x = 3.14;
/// // Good
/// let x = std::f32::consts::PI;
/// ```
#[clippy::version = "pre 1.29.0"]
pub APPROX_CONSTANT,
correctness,
"the approximate of a known float constant (in `std::fXX::consts`)"
}
```
This commit adds a `no_std` and `no_core` check on `swap` lint and additionally suggest `core::mem::swap` whenever possible.
Remove warning if both `std` and `core` is not present.
Don't destructure args tuple in format_args!
This allows Clippy to parse the HIR more simply since `arg0` is changed to `_args.0`. (cc rust-lang/rust-clippy#7843). From rustc's perspective, I think this is something between a lateral move and a tiny improvement since there are fewer bindings.
r? `@m-ou-se`
TraitKind -> Trait
TyAliasKind -> TyAlias
ImplKind -> Impl
FnKind -> Fn
All `*Kind`s in AST are supposed to be enums.
Tuple structs are converted to braced structs for the types above, and fields are reordered in syntactic order.
Also, mutable AST visitor now correctly visit spans in defaultness, unsafety, impl polarity and constness.
Reference `clippy_utils` docs on nightly-rustc and some other documentation updates
The `clippy_utils` crate is now part of the nightly-rustc documentation. See [**very beautiful documentation**](https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/). This PR references them in our documentation and updates some other documentation.
changelog: none
`cmp_s_u` is a tiny helper function only used by `cmp` and isn't useful on
it's own. Making it a nested function of `cmp` makes that clear and as a
bonus it's easier to call and doesn't require a `#[must_use]` attribute.
Update `str` utils to prevent ICEs and FNs
This PR reworks some string handling for lints regarding enum naming. I hope the refactoring will prevent future ICEs and help with new bug free implementations.
It might be better to review this PR by going through the commits, as `clippy_utils::camel_case` was renamed to `clippy_utils::str_utils` and then changed further. GH sadly doesn't really make the changes that obvious 🙃
Not too much more to say. Have a nice day 🌞
---
Fixes: rust-lang/rust-clippy#7869
changelog: ICE Fix: [`enum_variant_names`] #7869
make test module detection more strict
I started with some small improvements to clippy_utils/src/lib.rs, but then found that our "test" module detection would also catch words containing "test" like e.g. "attestation". So I made this a bit more strict (splitting by `'_'` and checking for `test` or `tests`), adding a test case as I went.
---
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: none
Coerce const FnDefs to implement const Fn traits
You can now pass a FnDef to a function expecting `F` where `F: ~const FnTrait`.
r? ``@oli-obk``
``@rustbot`` label T-compiler F-const_trait_impl
Introduce `Rvalue::ShallowInitBox`
Polished version of #88700.
Implements MCP rust-lang/compiler-team#460, and should allow #43596 to go forward.
In short, creating an empty box is split from a nullary-op `NullOp::Box` into two steps, first a call to `exchange_malloc`, then a `Rvalue::ShallowInitBox` which transmutes `*mut u8` to a shallow-initialized `Box<T>`. This allows the `exchange_malloc` call to unwind. Details can be found in the MCP.
`NullOp::Box` is not yet removed, purely to make reverting easier in case anything goes wrong as the result of this PR. If revert is needed a reversion of "Use Rvalue::ShallowInitBox for box expression" commit followed by a test bless should be sufficient.
Experiments in #88700 showed a very slight compile-time perf regression due to (supposedly) slightly more time spent in LLVM. We could omit unwind edge generation (in non-`oom=panic` case) in box expression MIR construction to restore perf; but I don't think it's necessary since runtime perf isn't affected and perf difference is rather small.
This allows the format_args! macro to keep the pre-expansion code out of
the unsafe block without doing gymnastics with nested `match`
expressions. This reduces codegen.
Introduce NullOp::AlignOf
This PR introduces `Rvalue::NullaryOp(NullOp::AlignOf, ty)`, which will be lowered from `align_of`, similar to `size_of` lowering to `Rvalue::NullaryOp(NullOp::SizeOf, ty)`.
The changes are originally part of #88700 but since it's not dependent on other changes and could have performance impact on its own, it's separated into its own PR.
