Fix int_plus_one suggestion causing parse error when LHS is an
`AssocOp::Cast`
When int_plus_one rewrites an expression like `x as usize + 1 <= y`, it
suggests `x as usize < y`. This doesn't compile because the parser
interprets `usize<y>` as the start of generic arguments rather than a
lt comparison.
The fix parenthesizes the LHS if it's an `AssocOp::Cast` and the
replacement
operator is `<`: `(x as usize) < y`.
---
changelog: [int_plus_one]: Parenthesize `AssocOp::Cast` in suggestion
when replacement operator is < to avoid parse error
Post-attribute ports cleanup pt. 1 (again)
This is a re-implementation of most (but not all) of https://github.com/rust-lang/rust/pull/154808
The code is the same as in that PR, other than a few changes, I'll leave comments where I changed things.
I did re-implement most of the commits rather than cherry-picking, so it's probably good to check the entire diff regardless
r? @jdonszelmann
Start using pattern types in libcore
cc rust-lang/rust#135996
Replaces the innards of `NonNull` with `*const T is !null`.
This does affect LLVM's optimizations, as now reading the field preserves the metadata that the field is not null, and transmuting to another type (e.g. just a raw pointer), will also preserve that information for optimizations. This can cause LLVM opts to do more work, but it's not guaranteed to produce better machine code.
Once we also remove all uses of rustc_layout_scalar_range_start from rustc itself, we can remove the support for that attribute entirely and handle all such needs via pattern types
Implement EII for statics
This PR implements EII for statics. I've tried to mirror the implementation for functions in a few places, this causes some duplicate code but I'm also not really sure whether there's a clean way to merge the implementations.
This does not implement defaults for static EIIs yet, I will do that in a followup PR
Rollup of 6 pull requests
Successful merges:
- rust-lang/rust#152901 (Introduce a `#[diagnostic::on_unknown]` attribute)
- rust-lang/rust#155078 (Reject dangling attributes in where clauses)
- rust-lang/rust#154449 (Invert dependency between `rustc_errors` and `rustc_abi`.)
- rust-lang/rust#154646 (Add suggestion to `.to_owned()` used on `Cow` when borrowing)
- rust-lang/rust#154993 (compiletest: pass -Zunstable-options for unpretty and no-codegen paths)
- rust-lang/rust#155097 (Make `rustc_attr_parsing::SharedContext::emit_lint` take a `MultiSpan` instead of a `Span`)
Add suggestion to `.to_owned()` used on `Cow` when borrowing
fixesrust-lang/rust#144792
supersedes rust-lang/rust#144925 with the review comments addressed
the tests suggested from @Kivooeo from https://github.com/rust-lang/rust/pull/144925#discussion_r2252703007 didn't work entirely, because these tests failed due to error `[E0308]` mismatched types, which actually already provides a suggestion, that actually makes the code compile:
```
$ cargo check
error[E0308]: mismatched types
--> src/main.rs:3:5
|
1 | fn test_cow_suggestion() -> String {
| ------ expected `std::string::String` because of return type
2 | let os_string = std::ffi::OsString::from("test");
3 | os_string.to_string_lossy().to_owned()
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `String`, found `Cow<'_, str>`
|
= note: expected struct `std::string::String`
found enum `std::borrow::Cow<'_, str>`
help: try using a conversion method
|
3 | os_string.to_string_lossy().to_owned().to_string()
| ++++++++++++
```
now this suggestion is of course not good or efficient code, but via clippy with `-Wclippy::nursery` lint group you can actually get to the correct code, so i don't think this is too much of an issue:
<details>
<summary>the clippy suggestions</summary>
```
$ cargo clippy -- -Wclippy::nursery
warning: this `to_owned` call clones the `Cow<'_, str>` itself and does not cause its contents to become owned
--> src/main.rs:3:5
|
3 | os_string.to_string_lossy().to_owned().to_string()
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/beta/index.html#suspicious_to_owned
= note: `#[warn(clippy::suspicious_to_owned)]` on by default
help: depending on intent, either make the `Cow` an `Owned` variant
|
3 | os_string.to_string_lossy().into_owned().to_string()
| ++
help: or clone the `Cow` itself
|
3 - os_string.to_string_lossy().to_owned().to_string()
3 + os_string.to_string_lossy().clone().to_string()
|
$ # apply first suggestion
$ cargo c -- -Wclippy::nursery
warning: redundant clone
--> src/main.rs:3:45
|
3 | os_string.to_string_lossy().into_owned().to_string()
| ^^^^^^^^^^^^ help: remove this
|
note: this value is dropped without further use
--> src/main.rs:3:5
|
3 | os_string.to_string_lossy().into_owned().to_string()
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= help: for further information visit https://rust-lang.github.io/rust-clippy/beta/index.html#redundant_clone
= note: `-W clippy::redundant-clone` implied by `-W clippy::nursery`
= help: to override `-W clippy::nursery` add `#[allow(clippy::redundant_clone)]`
```
</details>
the actual error that we are looking for is error `[E0515]`, cannot return value referencing local variable, which was only present in the original issue due to automatic type inference assuming you want to return a cloned `Cow<'_, str>`, which is of course not possible. this is why i took the original test functions and turned them into closures, where it's less obvious that it's trying to return the wrong type.
r? davidtwco
(because you reviewed the last attempt)
(also, let me know if i should squash this down to one commit and add myself as the co-contributor)
Use fine grained component-wise span tracking in use trees
This often produces nicer spans and even doesn't need a Span field anymore (not that I expect the unused field to affect any perf, but still neat).
After performance concers on the `nonminimal_bool` lint, we [performed a
crater run](https://github.com/rust-lang/rust/issues/153883)
This crater run revealed that in 19120 suggestions taken from 1.3
million crates, only about 36% had resulted in multi-terminal
suggestions (suggestions that were not only a single boolean).
This suggests that most cases of this lint firing, the expression that
triggered was pretty simple. And thus, we should not take such a huge
toll for a lint that isn't that useful.
In the future, a rewrite of that `bool_expr` function could be very
useful to alleviate the performance toll. Because clearly running the
Quine McCluskey algorithm on all those boolean operations is not ideal.
---
## Performance report:
`cargo clippy` -> 5,077,447,157 icount
`cargo check` -> 3,629,576,891 icount
So the Clippy that `rustup` delivers is about 1.4b instructions more
than `cargo check`.
`NonminimalBool` lint pass -> 318,169,034
$$\frac{nonminimal\\_bool}{clippy-check}*100 = 21.97496844$$
So, the `NonminimalBool` lint pass is about 22% of all Clippy-exclusive
instruction count.
---
After this change, we can now compare the Clippy BUILT FROM MASTER, NOT
DELIVERED BY RUSTUP, with the Clippy from this PR.
`master/target/release/cargo-clippy` -> 5,296,838,955 (Slower than
Rustup)
`pr/target/release/cargo-clippy` -> 4,977,035,941.
So, $master - PR = 319803014$, which is the exact icount of
`Nonminimal_bool` and about 22% of Clippy's whole execution time.
Profiled on `syn-2.0.71`, with Callgrind.
changelog:[`nonminimal_bool`]: Move to `pedantic`.
changelog:[`overly_complex_bool_expr`]: Move to `pedantic`.
This is going to be used as part of more thoroughly checking whether we
can change the type of an expression. e.g. `range_plus_one` suggests
changing `Range` to `RangeInclusive`.
changelog: none
In some cases, removing a cast that looks unnecessary can
still change type inference, even when the cast is to the
expected same type.
The lint now handles casts that flow through
intermediate expressions (e.g. calls, blocks, let
bindings, loops...). It also treats placeholder
generics (e.g. `::<_>`) as inference sensitive.
Added regression tests with similar behavior
as the original bug, some edge cases and
tests to check if false negative cases
weren't created.
Closesrust-lang/rust-clippy#16449
changelog: [`unnecessary_cast`]: fixed a suggestion to remove a cast
that can cause a compilation error if followed.
Philipp Krones