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
Use `.into_iter()` rather than `.drain(..)`
Replacing `.drain(..)` with `.into_iter()` makes my project's binary size smaller.
Fixes#1908
Applicability of this suggestion is `MaybeIncorrect` rather than `MachineApplicable` due to the complexity of "checking otherwise usage" X-|
changelog: Add new lint [`iter_with_drain`]
There's still open discussion if this lint is ready to be enabled by
default. We want to give us more time to figure this out and prevent
this lint from getting to stable as an enabled-by-default lint.
Add `unnecessary_find_map` lint
This PR adds an `unnecessary_find_map` lint. It is essentially just a minor enhancement of `unnecessary_filter_map`.
Closes#8467
changelog: New lint `unnecessary_find_map`
new lint: `missing-spin-loop`
This fixes#7809. I went with the shorter name because the function is called `std::hint::spin_loop`. It doesn't yet detect `while let` loops. I left that for a follow-up PR.
---
changelog: new lint: [`missing_spin_loop`]
Transmute_undefined_repr to nursery again
This PR reinstates #8418, which was reverted in #8425 (incorrectly I think).
I don't want to start a revert war over this but I feel very strongly that this lint is not in a state that would be a net benefit to users of clippy. In its current form, making this an enabled-by-default `correctness` lint with authoritative-sounding proclamations of undefined behavior does more harm than the benefit of the true positive cases.
I can file a bunch more examples of false positives but I don't want to give the author of this lint the impression that it is ready to graduate from `nursery` as soon as I've exhausted the amount of time I am willing to spend revising this lint.
Instead I would recommend that the author of the lint try running it on some reputable codebases containing transmutes. Everywhere that the lint triggers please consider critically whether it should be triggering. For cases that you think are true positives, please raise a few of them with the crate authors (in a PR or issue) to better understand their perspective if they think the transmute is correct.
---
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: Re-remove [`transmute_undefined_repr`] from default set of enabled lints
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.
fix false positives of large_enum_variant
fixes: #8321
The size of enums containing generic type was calculated to be 0.
I changed [large_enum_variant] so that such enums are not linted.
changelog: none
rustc_errors: let `DiagnosticBuilder::emit` return a "guarantee of emission".
That is, `DiagnosticBuilder` is now generic over the return type of `.emit()`, so we'll now have:
* `DiagnosticBuilder<ErrorReported>` for error (incl. fatal/bug) diagnostics
* can only be created via a `const L: Level`-generic constructor, that limits allowed variants via a `where` clause, so not even `rustc_errors` can accidentally bypass this limitation
* asserts `diagnostic.is_error()` on emission, just in case the construction restriction was bypassed (e.g. by replacing the whole `Diagnostic` inside `DiagnosticBuilder`)
* `.emit()` returns `ErrorReported`, as a "proof" token that `.emit()` was called
(though note that this isn't a real guarantee until after completing the work on
#69426)
* `DiagnosticBuilder<()>` for everything else (warnings, notes, etc.)
* can also be obtained from other `DiagnosticBuilder`s by calling `.forget_guarantee()`
This PR is a companion to other ongoing work, namely:
* #69426
and it's ongoing implementation:
#93222
the API changes in this PR are needed to get statically-checked "only errors produce `ErrorReported` from `.emit()`", but doesn't itself provide any really strong guarantees without those other `ErrorReported` changes
* #93244
would make the choices of API changes (esp. naming) in this PR fit better overall
In order to be able to let `.emit()` return anything trustable, several changes had to be made:
* `Diagnostic`'s `level` field is now private to `rustc_errors`, to disallow arbitrary "downgrade"s from "some kind of error" to "warning" (or anything else that doesn't cause compilation to fail)
* it's still possible to replace the whole `Diagnostic` inside the `DiagnosticBuilder`, sadly, that's harder to fix, but it's unlikely enough that we can paper over it with asserts on `.emit()`
* `.cancel()` now consumes `DiagnosticBuilder`, preventing `.emit()` calls on a cancelled diagnostic
* it's also now done internally, through `DiagnosticBuilder`-private state, instead of having a `Level::Cancelled` variant that can be read (or worse, written) by the user
* this removes a hazard of calling `.cancel()` on an error then continuing to attach details to it, and even expect to be able to `.emit()` it
* warnings were switched to *only* `can_emit_warnings` on emission (instead of pre-cancelling early)
* `struct_dummy` was removed (as it relied on a pre-`Cancelled` `Diagnostic`)
* since `.emit()` doesn't consume the `DiagnosticBuilder` <sub>(I tried and gave up, it's much more work than this PR)</sub>,
we have to make `.emit()` idempotent wrt the guarantees it returns
* thankfully, `err.emit(); err.emit();` can return `ErrorReported` both times, as the second `.emit()` call has no side-effects *only* because the first one did do the appropriate emission
* `&mut Diagnostic` is now used in a lot of function signatures, which used to take `&mut DiagnosticBuilder` (in the interest of not having to make those functions generic)
* the APIs were already mostly identical, allowing for low-effort porting to this new setup
* only some of the suggestion methods needed some rework, to have the extra `DiagnosticBuilder` functionality on the `Diagnostic` methods themselves (that change is also present in #93259)
* `.emit()`/`.cancel()` aren't available, but IMO calling them from an "error decorator/annotator" function isn't a good practice, and can lead to strange behavior (from the caller's perspective)
* `.downgrade_to_delayed_bug()` was added, letting you convert any `.is_error()` diagnostic into a `delay_span_bug` one (which works because in both cases the guarantees available are the same)
This PR should ideally be reviewed commit-by-commit, since there is a lot of fallout in each.
r? `@estebank` cc `@Manishearth` `@nikomatsakis` `@mark-i-m`
better ObligationCause for normalization errors in `can_type_implement_copy`
Some logic is needed so we can point to the field when given totally nonsense types like `struct Foo(<u32 as Iterator>::Item);`
Fixes#93687
Don't lint `match` expressions with `cfg`ed arms
Somehow there are no open issues related to this for any of the affected lints. At least none that I could fine from a quick search.
changelog: Don't lint `match` expressions with `cfg`ed arms in many cases
Fix `await_holding_lock` not linting `parking_lot` Mutex/RwLock
This adds tests for `RwLock` and `parking_lot::{Mutex, RwLock}`, which were added before in 2dc8c083f5, but never tested in UI tests. I noticed this while reading [fasterthanli.me](https://fasterthanli.me/articles/a-rust-match-made-in-hell) latest blog post, complaining that Clippy doesn't catch this for `parking_lot`. (Too many people read his blog, he's too powerful)
Some more things:
- Adds a test for #6446
- Improves the lint message
changelog: [`await_holding_lock`]: Now also lints for `parking_lot::{Mutex, RwLock}`
Even though the FP for that the lints were moved to pedantic isn't fixed
yet, running the lintcheck tool over the most popular 279 crates didn't
trigger this lint once. I would say that this lint is valuable enough,
despite the known FP, to be warn-by-default. Especially since a pretty
nice workaround exists.
Improve `redundant_slicing` lint
fixes#7972fixes#7257
This can supersede #7976
changelog: Fix suggestion for `redundant_slicing` when re-borrowing for a method call
changelog: New lint `deref_as_slicing`
Don't lint `needless_borrow` in method receiver positions
fixes#8408fixes#8407fixes#8391fixes#8367fixes#8380
This is a temporary fix for `needless_borrow`. The proper fix is included in #8355.
This should probably be merged into rustc before beta branches on Friday. This issue has been reported six or seven times in the past couple of weeks.
changelog: Fix various issues with `needless_borrow` n´. Note to changelog writer: those issues might have been introduced in this release cycle, so this might not matter in the changelog.
Don't lint Default::default if it is the udpate syntax base
changelog: Don't lint `Default::default` it is part of the update syntax
Current clippy warns about this:
```
warning: calling `Foo::default()` is more clear than this expression
--> src/main.rs:12:11
|
12 | ..Default::default()
| ^^^^^^^^^^^^^^^^^^ help: try: `Foo::default()`
|
```
With these changes, it will not lint that particular expression anymore.
Correctly mark the span of captured arguments in `format_args!()`
It should not include the braces, or misspelling suggestions will be wrong.
Fixes#94010.
Move transmute_undefined_repr back to nursery
There's still open discussion if this lint is ready to be enabled by
default. We want to give us more time to figure this out and prevent
this lint from getting to stable as an enabled-by-default lint.
cc https://github.com/rust-lang/rust-clippy/pull/8432
r? `@Manishearth` `@dtolnay`
I think this is the way to go here. We can re-enable this lint with the next sync, if we should decide to do so. But I would hold of for this release.
We have until Friday (beta branching) to decide if we want to merge this.
There's still open discussion if this lint is ready to be enabled by
default. We want to give us more time to figure this out and prevent
this lint from getting to stable as an enabled-by-default lint.
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 `Region` from this:
```
pub type Region<'tcx> = &'tcx RegionKind;
```
to this:
```
pub struct Region<'tcx>(&'tcx Interned<RegionKind>);
```
This now matches `Ty` and `Predicate` more closely.
Things to note
- Regions have always been interned, but we haven't been using pointer-based
`Eq` and `Hash`. This is now happening.
- I chose to impl `Deref` for `Region` because it makes pattern matching a lot
nicer, and `Region` can be viewed as just a smart wrapper for `RegionKind`.
- Various methods are moved from `RegionKind` to `Region`.
- There is a lot of tedious sigil changes.
- A couple of types like `HighlightBuilder`, `RegionHighlightMode` now have a
`'tcx` lifetime because they hold a `Ty<'tcx>`, so they can call `mk_region`.
- A couple of test outputs change slightly, I'm not sure why, but the new
outputs are a little better.
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.)
Fix `transmute_undefined_repr` with single field `#[repr(C)]` structs
Fixes: #8417
The description has also been made more precise.
changelog: Fix `transmute_undefined_repr` with single field `#[repr(C)]` structs
changelog: Move `transmute_undefined_repr` back to `correctness`
Remove defaultness from ImplItem.
This information is not really used anywhere, except HIR pretty-printing. This makes ImplItem and TraitItem more similar.
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.
warn if we find multiple clippy configs
Fixes#8323
---
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: warn if we find multiple clippy configs
Add lint `transmute_undefined_repr`
Partially implements #3999 and #546
This doesn't consider `enum`s at all right now as those are going to be a pain to deal with. This also allows `#[repr(Rust)]` structs with only one non-zero sized fields. I think those are technically undefined when transmuted.
changelog: Add lint `transmute_undefined_repr`
Add `explicit_write` suggestions for `write!`s with format args
changelog: Add [`explicit_write`] suggestions for `write!`s with format args
Fixes#4542
```rust
writeln!(std::io::stderr(), "macro arg {}", one!()).unwrap();
```
Now suggests:
```
error: use of `writeln!(stderr(), ...).unwrap()`
--> $DIR/explicit_write.rs:36:9
|
LL | writeln!(std::io::stderr(), "macro arg {}", one!()).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `eprintln!("macro arg {}", one!())`
```
---------
r? `@camsteffen` (again, sorry 😛) for the `FormatArgsExpn` change
Before this change `inputs_span` returned a span pointing to just `1` in
```rust
macro_rules! one {
() => { 1 };
}
`writeln!(std::io::stderr(), "macro arg {}", one!()).unwrap();`
```
And the `source_callsite` of that span didn't include the format string, it was just `one!()`
make unwrap_used also trigger on .get().unwrap()
fixes#8124
changelog: make the [unwrap_used] lint trigger for code of the form such as `.get(i).unwrap()` and `.get_mut(i).unwrap()`
[explicit_counter_loop] suggests `.into_iter()`, despite that triggering [into_iter_on_ref] in some cases
I have modified `fn make_iterator_snippet` in clippy_lints/src/loops/utils.rs ,so this change has some little influence on another lint [manual_flatten] .
fixes#8155
---
changelog: Fix that [`explicit_counter_loop`] suggests `into_iter()` despite that triggering [`into_iter_on_ref`] in some cases
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
single_match: Don't lint non-exhaustive matches; support tuples
`single_match` lint:
* Don't lint exhaustive enum patterns without a wild.
Rationale: The definition of the enum could be changed, so the user can get non-exhaustive match after applying the suggested lint (see https://github.com/rust-lang/rust-clippy/issues/8282#issuecomment-1013566068 for context).
* Lint `match` constructions with tuples (as suggested at https://github.com/rust-lang/rust-clippy/issues/8282#issuecomment-1015621148)
Closes#8282
---
changelog: [`single_match`]: Don't lint exhaustive enum patterns without a wild.
changelog: [`single_match`]: Lint `match` constructions with tuples
Fix underflow in `manual_split_once` lint
Hi, a friend found clippy started crashing on a suspiciously large allocation of `u64::MAX` memory on their code.
The mostly minimized repro is:
```rust
fn _f01(title: &str) -> Option<()> {
let _ = title[1..].splitn(2, '[').next()?;
Some(())
}
```
The underflow happens in this case on line 57 of the patch but I've changed the other substraction to saturating as well since it could potentially cause the same issue.
I'm not sure where to put a regression test, or if it's even worth for such a thing.
Aside, has it been considered before to build clippy with overflow checks enabled?
changelog: fix ICE of underflow in `manual_split_once` lint
Store a `Symbol` instead of an `Ident` in `AssocItem`
This is the same idea as #92533, but for `AssocItem` instead
of `VariantDef`/`FieldDef`.
With this change, we no longer have any uses of
`#[stable_hasher(project(...))]`
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`
This commit changes the behavior of `single_match` lint.
After that, we won't lint non-exhaustive matches like this:
```rust
match Some(v) {
Some(a) => println!("${:?}", a),
None => {},
}
```
The rationale is that, because the type of `a` could be changed, so the
user can get non-exhaustive match after applying the suggested lint (see
https://github.com/rust-lang/rust-clippy/issues/8282#issuecomment-1013566068
for context).
We also will lint `match` constructions with tuples. When we see the
tuples on the both arms, we will check them both at the same time, and
if they form exhaustive match, we could display the warning.
Closes#8282
This is the same idea as #92533, but for `AssocItem` instead
of `VariantDef`/`FieldDef`.
With this change, we no longer have any uses of
`#[stable_hasher(project(...))]`
