Refactor HIR item-like traversal (part 1)
Issue #95004
- Create hir_crate_items query which traverses tcx.hir_crate(()).owners to return a hir::ModuleItems
- use tcx.hir_crate_items in tcx.hir().items() to return an iterator of hir::ItemId
- use tcx.hir_crate_items to introduce a tcx.hir().par_items(impl Fn(hir::ItemId)) to traverse all items in parallel;
Signed-off-by: Miguel Guarniz <mi9uel9@gmail.com>
cc `@cjgillot`
Fix formatting of `cast_abs_to_unsigned` docs
The "use instead" section of the example was not being formatted as Rust code, and the "configuration" documentation was being formatted as Rust code.
changelog: `[cast_abs_to_unsigned]` Fix example/configuration formatting
Implement sym operands for global_asm!
Tracking issue: #93333
This PR is pretty much a complete rewrite of `sym` operand support for inline assembly so that the same implementation can be shared by `asm!` and `global_asm!`. The main changes are:
- At the AST level, `sym` is represented as a special `InlineAsmSym` AST node containing a path instead of an `Expr`.
- At the HIR level, `sym` is split into `SymStatic` and `SymFn` depending on whether the path resolves to a static during AST lowering (defaults to `SynFn` if `get_early_res` fails).
- `SymFn` is just an `AnonConst`. It runs through typeck and we just collect the resulting type at the end. An error is emitted if the type is not a `FnDef`.
- `SymStatic` directly holds a path and the `DefId` of the `static` that it is pointing to.
- The representation at the MIR level is mostly unchanged. There is a minor change to THIR where `SymFn` is a constant instead of an expression.
- At the codegen level we need to apply the target's symbol mangling to the result of `tcx.symbol_name()` depending on the target. This is done by calling the LLVM name mangler, which handles all of the details.
- On Mach-O, all symbols have a leading underscore.
- On x86 Windows, different mangling is used for cdecl, stdcall, fastcall and vectorcall.
- No mangling is needed on other platforms.
r? `@nagisa`
cc `@eddyb`
changelog: [`await_holding_invalid_type`]
This lint allows users to create a denylist of types which are not allowed to be
held across await points. This is essentially a re-implementation of the
language-level [`must_not_suspend`
lint](https://github.com/rust-lang/rust/issues/83310). That lint has a lot of
work still to be done before it will reach Rust stable, and in the meantime
there are a lot of types which can trip up developers if they are used
improperly.
New lint `format_add_strings`
Closes#6261
changelog: Added [`format_add_string`]: recommend using `write!` instead of appending the result of `format!`
Add `usize` cast to `clippy::manual_bits` suggestion
A fix for the suggestion from https://github.com/rust-lang/rust-clippy/pull/8213
changelog: [`manual_bits`]: The suggestion now includes a cast for proper type conversion
This adds test to make sure correct behavior of lint
- The first test's option variable is not a temporary variable
- The second test does not make usage of `take()`
- The third test makes usage of `take()` and uses a temporary variable
Instead of type checking the entire expression (causing a false
positive), only type check for a subset of the expression (the receiver of
the matched function: `take()`)
This lint checks if Option::take() is used on a temporary value (a value
that is not of type &mut Option and that is not a Place expression) to
suggest omitting take()
Check for loops/closures in `local_used_after_expr`
Follow up to #8646, catches when a local is used multiple times because it's in a loop or a closure
changelog: none
errors: lazily load fallback fluent bundle
Addresses (hopefully) https://github.com/rust-lang/rust/pull/95667#issuecomment-1094794087.
Loading the fallback bundle in compilation sessions that won't go on to emit any errors unnecessarily degrades compile time performance, so lazily create the Fluent bundle when it is first required.
r? `@ghost` (just for perf initially)
Use mir constant in thir instead of ty::Const
This is blocked on https://github.com/rust-lang/rust/pull/94059 (does include its changes, the first two commits in this PR correspond to those changes) and https://github.com/rust-lang/rust/pull/93800 being reinstated (which had to be reverted). Mainly opening since `@lcnr` offered to give some feedback and maybe also for a perf-run (if necessary).
This currently contains a lot of duplication since some of the logic of `ty::Const` had to be copied to `mir::ConstantKind`, but with the introduction of valtrees a lot of that functionality will disappear from `ty::Const`.
Only the last commit contains changes that need to be reviewed here. Did leave some `FIXME` comments regarding future implementation decisions and some things that might be incorrectly implemented.
r? `@oli-obk`
Loading the fallback bundle in compilation sessions that won't go on to
emit any errors unnecessarily degrades compile time performance, so
lazily create the Fluent bundle when it is first required.
Signed-off-by: David Wood <david.wood@huawei.com>
fix unnecessary_to_owned about msrv
This PR fixes ``[`unnecessary_owned`]``.
## What
```rust
# sample code
fn _msrv_1_35() {
#![clippy::msrv = "1.35"]
let _ = &["x"][..].to_vec().into_iter();
}
fn _msrv_1_36() {
#![clippy::msrv = "1.36"]
let _ = &["x"][..].to_vec().into_iter();
}
```
If we will check this code using clippy, ``[`unnecessary_owned`]`` will modify the code as follows.
```rust
error: unnecessary use of `to_vec`
--> $DIR/unnecessary_to_owned.rs:219:14
|
LL | let _ = &["x"][..].to_vec().into_iter();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `["x"][..].iter().copied()`
error: unnecessary use of `to_vec`
--> $DIR/unnecessary_to_owned.rs:224:14
|
LL | let _ = &["x"][..].to_vec().into_iter();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `["x"][..].iter().copied()`
```
This is incorrect. Because `Iterator::copied` was estabilished in 1.36.
## Why
This bug was caused by not separating "copied" and "clone" by reference to msrv.
89ee6aa6e3/clippy_lints/src/methods/unnecessary_to_owned.rs (L195)
So, I added a conditional branch and described the corresponding test.
Thank you in advance.
changelog: fix wrong suggestions about msrv in [`unnecessary_to_owned`]
r! `@giraffate`
Don't lint `manual_non_exhaustive` when the enum variant is used
fixes#5714
changelog: Don't lint `manual_non_exhaustive` when the enum variant is used
Do not trigger ``[`rest_pat_in_fully_bound_structs`]`` on `#[non_exhaustive]` structs
fixes#8029
Just adds an additional check to ensure that the`ty::VariantDef` is not marked as `#[non_exhaustive]`.
changelog: Do not apply ``[`rest_pat_in_fully_bound_structs`]`` on structs marked as non exhaustive.
Prevent infinite (exponential) recursion in only_used_in_recursion
This simplifies the visitor code a bit and prevents checking expressions
multiple times. I still think this lint should be removed for now,
because its code isn't really tested.
Fixes#8689
**NOTE:** Before merging this, we should talk about removing and revisiting this lint. See my comment in #8689
changelog: prevent infinite recursion in [`only_used_in_recursion`]
This simplifies the visitor code a bit and prevents checking expressions
multiple times. I still think this lint should be removed for now,
because its code isn't really tested.
adding condition for map_clone message
This PR fixes the message about `map_clone`.
if msrv >= 1.36, the message is correct.
```bash
$ cat main.rs
fn main() {
let x: Vec<&i32> = vec![&1, &2];
let y: Vec<_> = x.iter().map(|i| *i).collect();
println!("{:?}", y);
}
$ cargo clippy
warning: you are using an explicit closure for copying elements
--> main.rs:3:20
|
3 | let y: Vec<_> = x.iter().map(|i| *i).collect();
| ^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `x.iter().copied()`
|
= note: `#[warn(clippy::map_clone)]` on by default
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_clone
warning: `test` (build script) generated 1 warning
warning: `test` (bin "test") generated 1 warning (1 duplicate)
Finished dev [unoptimized + debuginfo] target(s) in 0.00s
```
but, if msrv < 1.36, the suggestion is `cloned`, but the message is `copying`.
```bash
$ cat clippy.toml
msrv = "1.35"
$ cargo clippy
warning: you are using an explicit closure for copying elements
--> main.rs:3:20
|
3 | let y: Vec<_> = x.iter().map(|i| *i).collect();
| ^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `x.iter().cloned()`
```
I think the separation of messages will make it more user-friendly.
thank you in advance.
changelog: Fixed a message in map_clone.
New lint `is_digit_ascii_radix`
Closes#6399
changelog: Added [`is_digit_ascii_radix`]: recommend `is_ascii_digit()` or `is_ascii_hexdigit()` in place of `is_digit(10)` and `is_digit(16)`
Fix subtraction overflow in `cast_possible_truncation`
changelog: Fix false negative due to subtraction overflow in `cast_possible_truncation`
I *think* a false negative is the worst that can happen from this
Don't lint various match lints when expanded by a proc-macro
fixes#4952
As always for proc-macro output this is a hack-job of a fix. It would be really nice if more proc-macro authors would set spans correctly.
changelog: Don't lint various lints on proc-macro output.
Remove overlap between `manual_split_once` and `needless_splitn`
changelog: Remove overlap between [`manual_split_once`] and [`needless_splitn`]. Fixes some incorrect `rsplitn` suggestions for [`manual_split_once`]
Things that can trigger `needless_splitn` no longer trigger `manual_split_once`, e.g.
```rust
s.[r]splitn(2, '=').next();
s.[r]splitn(2, '=').nth(0);
s.[r]splitn(3, '=').next_tuple();
```
Fixes some suggestions:
```rust
let s = "should not match";
s.rsplitn(2, '.').nth(1);
// old -> Some("should not match")
Some(s.rsplit_once('.').map_or(s, |x| x.0));
// new -> None
s.rsplit_once('.').map(|x| x.0);
s.rsplitn(2, '.').nth(1)?;
// old -> "should not match"
s.rsplit_once('.').map_or(s, |x| x.0);
// new -> early returns
s.rsplit_once('.')?.0;
```
Cached stable hash cleanups
r? `@nnethercote`
Add a sanity assertion in debug mode to check that the cached hashes are actually the ones we get if we compute the hash each time.
Add a new data structure that bundles all the hash-caching work to make it easier to re-use it for different interned data structures
- Create hir_crate_items query which traverses tcx.hir_crate(()).owners to return a hir::ModuleItems
- use tcx.hir_crate_items in tcx.hir().items() to return an iterator of hir::ItemId
- add par_items(impl Fn(hir::ItemId)) to traverse all items in parallel
Signed-off-by: Miguel Guarniz <mi9uel9@gmail.com>
ignore `&x | &y` in unnested_or_patterns
replacing it with `&(x | y)` is actually more characters
Fixes#6973
changelog: [`unnested_or_patterns`] ignore `&x | &y`, nesting would result in more characters
Add a lint to detect cast to unsigned for abs() and suggest unsigned_…
…abs()
changelog: Add a [`cast_abs_to_unsigned`] that checks for uses of `abs()` that are cast to the corresponding unsigned integer type and suggest to replace them with `unsigned_abs()`.
Fix `as_deref_mut` false positives in `needless_option_as_deref`
Also moves it into `methods/`
Fixes#7846Fixes#8047
changelog: [`needless_option_as_deref`]: No longer lints for `as_deref_mut` on Options that cannot be moved
supersedes #8064
fix FP in lint `[needless_match]`
fixes: #8542fixes: #8551fixes: #8595fixes: #8599
---
changelog: check for more complex custom type, and ignore type coercion in [`needless_match`]
Suggest from_utf8_unchecked in const contexts
Unfortunately I couldn't figure out how to check whether a given expression is in an `unsafe` context or not, so I just unconditionally emit the wrapping `unsafe {}` block in the suggestion. If there is an easy way to get it to work better then I would love to hear it.
changelog: Suggest `from_utf8_unchecked` instead of `from_utf8` in const contexts for ``[`transmute_bytes_to_str`]``
refs: #8379
Fix unnecessary_cast suggestion for type aliasses
Fix#6923. The [`unnecessary_cast`] lint now will skip casting to non-primitive type.
changelog: fix lint [`unnecessary_cast `]
`indexing_slicing` should not fire if a valid array index comes from a constant function that is evaluated at compile-time
fix#8348
changelog: [`indexing_slicing`] fewer false positives in `const` contexts and with `const` indices
Add an option for enabling and disabling Fluent's directionality
isolation markers in output. Disabled by default as these can render in
some terminals and applications.
Signed-off-by: David Wood <david.wood@huawei.com>
Extend loading of Fluent bundles so that bundles can be loaded from the
sysroot based on the language requested by the user, or using a nightly
flag.
Sysroot bundles are loaded from `$sysroot/share/locale/$locale/*.ftl`.
Signed-off-by: David Wood <david.wood@huawei.com>
This commit updates the signatures of all diagnostic functions to accept
types that can be converted into a `DiagnosticMessage`. This enables
existing diagnostic calls to continue to work as before and Fluent
identifiers to be provided. The `SessionDiagnostic` derive just
generates normal diagnostic calls, so these APIs had to be modified to
accept Fluent identifiers.
In addition, loading of the "fallback" Fluent bundle, which contains the
built-in English messages, has been implemented.
Each diagnostic now has "arguments" which correspond to variables in the
Fluent messages (necessary to render a Fluent message) but no API for
adding arguments has been added yet. Therefore, diagnostics (that do not
require interpolation) can be converted to use Fluent identifiers and
will be output as before.
`MultiSpan` contains labels, which are more complicated with the
introduction of diagnostic translation and will use types from
`rustc_errors` - however, `rustc_errors` depends on `rustc_span` so
`rustc_span` cannot use types like `DiagnosticMessage` without
dependency cycles. Introduce a new `rustc_error_messages` crate that can
contain `DiagnosticMessage` and `MultiSpan`.
Signed-off-by: David Wood <david.wood@huawei.com>
Rework `undocumented_unsafe_blocks`
fixes: #8264fixes: #8449
One thing came up while working on this. Currently comments on the same line are supported like so:
```rust
/* SAFETY: reason */ unsafe {}
```
Is this worth supporting at all? Anything other than a couple of words doesn't really fit well.
edit: [zulip topic](https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/.60undocumented_unsafe_blocks.60.20same.20line.20comment)
changelog: Don't lint `undocumented_unsafe_blocks` when the unsafe block comes from a proc-macro.
changelog: Don't lint `undocumented_unsafe_blocks` when the preceding line has a safety comment and the unsafe block is a sub-expression.
add `empty_structs_with_brackets`
<!-- Thank you for making Clippy better!
We're collecting our changelog from pull request descriptions.
If your PR only includes internal changes, you can just write
`changelog: none`. Otherwise, please write a short comment
explaining your change. Also, it's helpful for us that
the lint name is put into brackets `[]` and backticks `` ` ` ``,
e.g. ``[`lint_name`]``.
If your PR fixes an issue, you can add "fixes #issue_number" into this
PR description. This way the issue will be automatically closed when
your PR is merged.
If you added a new lint, here's a checklist for things that will be
checked during review or continuous integration.
- \[ ] 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`
[lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints
Note that you can skip the above if you are just opening a WIP PR in
order to get feedback.
Delete this line and everything above before opening your PR.
--
*Please write a short comment explaining your change (or "none" for internal only changes)*
-->
Closes#8591
I'm already sorry for the massive diff 😅
changelog: New lint [`empty_structs_with_brackets`]
Handle relative paths in module_files lints
The problem being that when clippy is run in the project's directory `lp` would be a relative path, this wasn't caught by the tests as there `lp` is an absolute path. Being a relative path it did not start with `trim_src_path` and so was ignored
Also allowed the removal of some `.to_os_string`/`.to_owned`s
changelog: Fixes [`self_named_module_files`] and [`mod_module_files`] not linting
Fixes#8123, cc `@DevinR528`
single_element_loop: handle arrays for Edition2021
changelog: [`single_element_loop`] handle arrays in Edition 2021, handle `.iter_mut()` and `.into_iter()`, and wrap in parens if necessary
Rollup of 6 pull requests
Successful merges:
- #93901 (Stabilize native library modifier syntax and the `whole-archive` modifier specifically)
- #94806 (Fix `cargo run tidy`)
- #94869 (Add the generic_associated_types_extended feature)
- #95011 (async: Give predictable name to binding generated from .await expressions.)
- #95251 (Reduce max hash in raw strings from u16 to u8)
- #95298 (Fix double drop of allocator in IntoIter impl of Vec)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
Remember mutability in `DefKind::Static`.
This allows to compute the `BodyOwnerKind` from `DefKind` only, and
removes a direct dependency of some MIR queries onto HIR.
As a side effect, it also simplifies metadata, since we don't need 4
flavours of `EntryKind::*Static` any more.
Add `crate_in_macro_def` lint
This PR adds a lint to check for `crate` as opposed to `$crate` used in a macro definition.
I think this can close#4798. That issue focused on the case where the macro author "imports something into said macro."
But I think use of `crate` is likely to be a bug whether it appears in a `use` statement or not. There could be some use case I am failing to see, though. (cc: `@nilscript` `@flip1995)`
changelog: `crate_in_macro_def`
This allows to compute the `BodyOwnerKind` from `DefKind` only, and
removes a direct dependency of some MIR queries onto HIR.
As a side effect, it also simplifies metadata, since we don't need 4
flavours of `EntryKind::*Static` any more.
More `transmute_undefined_repr` fixes
fixes: #8498fixes: #8501fixes: #8503
changelog: Allow `transumte_undefined_repr` between fat pointers and `(usize, usize)`
changelog: Allow `transumte_undefined_repr` when one side is a union
changelog: Fix `transumte_undefined_repr` on tuples with one non-zero-sized type.
new lint: `only_used_in_recursion`
changed:
- added `only_used_in_recursion`.
- fixed code that variables are only used in recursion.
- this would not lint when `unused_variable`
This fixes: #8390
-----
changelog: add lint [`only_used_in_recursion`]
This commit makes `AdtDef` use `Interned`. Much the commit is tedious
changes to introduce getter functions. The interesting changes are in
`compiler/rustc_middle/src/ty/adt.rs`.
Rollup of 4 pull requests
Successful merges:
- #93350 (libunwind: readd link attrs to _Unwind_Backtrace)
- #93827 (Stabilize const_fn_fn_ptr_basics, const_fn_trait_bound, and const_impl_trait)
- #94696 (Remove whitespaces and use CSS to align line numbers to the right instead)
- #94700 (rustdoc: Update minifier version)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
`Layout` is another type that is sometimes interned, sometimes not, and
we always use references to refer to it so we can't take any advantage
of the uniqueness properties for hashing or equality checks.
This commit renames `Layout` as `LayoutS`, and then introduces a new
`Layout` that is a newtype around an `Interned<LayoutS>`. It also
interns more layouts than before. Previously layouts within layouts
(via the `variants` field) were never interned, but now they are. Hence
the lifetime on the new `Layout` type.
Unlike other interned types, these ones are in `rustc_target` instead of
`rustc_middle`. This reflects the existing structure of the code, which
does layout-specific stuff in `rustc_target` while `TyAndLayout` is
generic over the `Ty`, allowing the type-specific stuff to occur in
`rustc_middle`.
The commit also adds a `HashStable` impl for `Interned`, which was
needed. It hashes the contents, unlike the `Hash` impl which hashes the
pointer.
Llint for casting between raw slice pointers with different element sizes
This lint disallows using `as` to convert from a raw pointer to a slice (e.g. `*const [i32]`, `*mut [Foo]`) to any other raw pointer to a slice if the element types have different sizes. When a raw slice pointer is cast, the data pointer and count metadata are preserved. This means that when the size of the inner slice's element type changes, the total number of bytes pointed to by the count changes. For example a `*const [i32]` with length 4 (four `i32` elements) is cast `as *const [u8]` the resulting pointer points to four `u8` elements at the same address, losing most of the data. When the size *increases* the resulting pointer will point to *more* data, and accessing that data will be UB.
On its own, *producing* the pointer isn't actually a problem, but because any use of the pointer as a slice will either produce surprising behavior or cause UB I believe this is a correctness lint. If the pointer is not intended to be used as a slice, the user should instead use any of a number of methods to produce just a data pointer including an `as` cast to a thin pointer (e.g. `p as *const i32`) or if the pointer is being created from a slice, the `as_ptr` method on slices. Detecting the intended use of the pointer is outside the scope of this lint, but I believe this lint will also lead users to realize that a slice pointer is only for slices.
There is an exception to this lint when either of the slice element types are zero sized (e.g `*mut [()]`). The total number of bytes pointed to by the slice with a zero sized element is zero. In that case preserving the length metadata is likely intended as a workaround to get the length metadata of a slice pointer though a zero sized slice.
The lint does not forbid casting pointers to slices with the *same* element size as the cast was likely intended to reinterpret the data in the slice as some equivalently sized data and the resulting pointer will behave as intended.
---
changelog: Added ``[`cast_slice_different_sizes`]``, a lint that disallows using `as`-casts to convert between raw pointers to slices when the elements have different sizes.
Add lint to detect `allow` attributes without reason
I was considering putting this lint into the pedantic group. However, that would result in countless warnings for existing projects. Having it in restriction also seems good to me 🙃 (And now I need sleep 💤 )
---
changelog: New lint [`allow_lint_without_reason`] (Requires the `lint_reasons` feature)
Closes: rust-lang/rust-clippy#8502
