Explain why clippy's HIR const eval exists
When I initially found this, I was wondering why clippy wasn't just using miri, but after some discussion with some rustc folks let's document why.
changelog: none
Remove `GenKillAnalysis`
There are two kinds of dataflow analysis in the compiler: `Analysis`, which is the basic kind, and `GenKillAnalysis`, which is a more specialized kind for gen/kill analyses that is intended as an optimization. However, it turns out that `GenKillAnalysis` is actually a pessimization! It's faster (and much simpler) to do all the gen/kill analyses via `Analysis`. This lets us remove `GenKillAnalysis`, and `GenKillSet`, and a few other things, and also merge `AnalysisDomain` into `Analysis`. The PR removes 500 lines of code and improves performance.
r? `@tmiasko`
Rollup of 9 pull requests
Successful merges:
- #122670 (Fix bug where `option_env!` would return `None` when env var is present but not valid Unicode)
- #131095 (Use environment variables instead of command line arguments for merged doctests)
- #131339 (Expand set_ptr_value / with_metadata_of docs)
- #131652 (Move polarity into `PolyTraitRef` rather than storing it on the side)
- #131675 (Update lint message for ABI not supported)
- #131681 (Fix up-to-date checking for run-make tests)
- #131702 (Suppress import errors for traits that couldve applied for method lookup error)
- #131703 (Resolved python deprecation warning in publish_toolstate.py)
- #131710 (Remove `'apostrophes'` from `rustc_parse_format`)
r? `@ghost`
`@rustbot` modify labels: rollup
Add `&pin (mut|const) T` type position sugar
This adds parser support for `&pin mut T` and `&pin const T` references. These are desugared to `Pin<&mut T>` and `Pin<&T>` in the AST lowering phases.
This PR currently includes #130526 since that one is in the commit queue. Only the most recent commits (bd450027eb4a94b814a7dd9c0fa29102e6361149 and following) are new.
Tracking:
- #130494
r? `@compiler-errors`
Check MethodCall/Call arg count earlier or at all
This gets rid of a bunch of possible panic spots, as well as bailing out earlier for optimisation reasons.
I started doing this because I saw that a significant amount of time was being spent in the `create_dir` restriction lint when running clippy with `perf`, but this also helps with robustness.
changelog: none
Turn declare_clippy_lint into a declarative macro
Ease of development, and hopefully compile times (the dependencies are still there because of ui-test). The procedural macro was doing just some very basic processing (like assigning a lint level to each category), so it didn't have a reason to stay IMO
changelog: None
Don't warn on proc macro generated code in `needless_return`
Fixes#13458Fixes#13457Fixes#13467Fixes#13479Fixes#13481Fixes#13526Fixes#13486
The fix is unfortunately a little more convoluted than just simply adding a `is_from_proc_macro`. That check *does* fix the issue, however it also introduces a bunch of false negatives in the tests, specifically when the returned expression is in a different syntax context, e.g. `return format!(..)`.
The proc macro check builds up a start and end pattern based on the HIR nodes and compares it to a snippet of the span, however that would currently fail for `return format!(..)` because we would have the patterns `("return", <something inside of the format macro>)`, which doesn't compare equal. So we now return an empty string pattern for when it's in a different syntax context.
"Hide whitespace" helps a bit for reviewing the proc macro detection change
changelog: none
Don't warn on proc macro generated code in `needless_return`
Fixes#13458Fixes#13457Fixes#13467Fixes#13479Fixes#13481Fixes#13526Fixes#13486
The fix is unfortunately a little more convoluted than just simply adding a `is_from_proc_macro`. That check *does* fix the issue, however it also introduces a bunch of false negatives in the tests, specifically when the returned expression is in a different syntax context, e.g. `return format!(..)`.
The proc macro check builds up a start and end pattern based on the HIR nodes and compares it to a snippet of the span, however that would currently fail for `return format!(..)` because we would have the patterns `("return", <something inside of the format macro>)`, which doesn't compare equal. So we now return an empty string pattern for when it's in a different syntax context.
"Hide whitespace" helps a bit for reviewing the proc macro detection change
changelog: none
Show interior mutability chain in `mutable_key_type`
Fixes#10619
Just ran into this myself and I definitely agree it's not very nice to have to manually go through all the types involved to figure out why this happens and to evaluate if this is really a problem (knowing if the field of a struct is something that a hash impl relies on), so this changes the lint to emit notes for each step involved.
changelog: none
Make opaque types regular HIR nodes
Having opaque types as HIR owner introduces all sorts of complications. This PR proposes to make them regular HIR nodes instead.
I haven't gone through all the test changes yet, so there may be a few surprises.
Many thanks to `@camelid` for the first draft.
Fixes https://github.com/rust-lang/rust/issues/129023Fixes#129099Fixes#125843Fixes#119716Fixes#121422
Stabilize the `map`/`value` methods on `ControlFlow`
And fix the stability attribute on the `pub use` in `core::ops`.
libs-api in https://github.com/rust-lang/rust/issues/75744#issuecomment-2231214910 seemed reasonably happy with naming for these, so let's try for an FCP.
Summary:
```rust
impl<B, C> ControlFlow<B, C> {
pub fn break_value(self) -> Option<B>;
pub fn map_break<T>(self, f: impl FnOnce(B) -> T) -> ControlFlow<T, C>;
pub fn continue_value(self) -> Option<C>;
pub fn map_continue<T>(self, f: impl FnOnce(C) -> T) -> ControlFlow<B, T>;
}
```
Resolves#75744
``@rustbot`` label +needs-fcp +t-libs-api -t-libs
---
Aside, in case it keeps someone else from going down the same dead end: I looked at the `{break,continue}_value` methods and tried to make them `const` as part of this, but that's disallowed because of not having `const Drop`, so put it back to not even unstably-const.
Compare trait references in `trait_duplication_in_bounds` correctly
Fixes#13476Fixes#11067Fixes#9915Fixes#9626
Currently, the `trait_duplication_in_bounds` lints has a helper type for a trait reference that can be used for comparison and hashing, represented as `{trait: Res, generic_args: Vec<Res>}`. However, there are a lot of issues with this. For one, a `Res` can't represent e.g. references, slices, or lots of other types, as well as const generics and associated type equality. In those cases, the lint simply ignores them and has no way of checking if they're actually the same.
So, instead of using `Res` for this, use `SpanlessEq` and `SpanlessHash` for comparisons with the trait path for checking if there are duplicates.
However, using `SpanlessEq` as is alone lead to a false negative in the test. `std::clone::Clone` + `foo::Clone` wasn't recognized as a duplicate, because it has different segments. So this also adds a new "mode" to SpanlessEq which compares by final resolution. (I've been wondering if this can't just be the default but it's quite a large scale change as it affects a lot of lints and I haven't yet looked at all uses of it to see if there are lints that really do care about having exactly the same path segments).
Maybe an alternative would be to turn the hir types/consts into middle types/consts and compare them instead but I'm not sure there's really a good way to do that
changelog: none
Refactoring to `OpaqueTyOrigin`
Pulled out of a larger PR that uses these changes to do cross-crate encoding of opaque origin, so we can use them for edition 2024 migrations. These changes should be self-explanatory on their own, tho 😄
Extend `needless_lifetimes` to suggest eliding `impl` lifetimes
Example:
```
error: the following explicit lifetimes could be elided: 'a
--> tests/ui/needless_lifetimes.rs:332:10
|
LL | impl<'a> Foo for Baz<'a> {}
| ^^ ^^
|
help: elide the lifetimes
|
LL - impl<'a> Foo for Baz<'a> {}
LL + impl Foo for Baz<'_> {}
```
The main change is in how `impl` lifetime uses are tracked. Previously, a hashmap was created, and lifetimes were removed from the hashmap as their uses were discovered. However, the uses are needed to generate elision suggestions. So, now, uses are added to the hashmap as they are discovered.
The PR is currently organized as six commits, which I think are self-explanatory:
- Extend `needless_lifetimes` to suggest eliding `impl` lifetimes
- Reorder functions _[not strictly necessary, but IMHO, the code is better structured as a result]_
- Fix lifetime tests
- Fix non-lifetime tests
- Fix `clippy_lints` and `clippy_utils`
- Fix typo in `needless_lifetimes` test
r? `@Alexendoo` (I think you are `needless_lifetimes`' primary author? Sorry if I have this wrong.)
---
changelog: Extend `needless_lifetimes` to suggest eliding `impl` lifetimes
Clippy subtree update
r? `@Manishearth`
Really delayed sync (2 1/2 weeks), because of a `debug_assertion` we hit, and I didn't have the time to investigate earlier.
It would be nice to merge this PR with some priority, as it includes a lot of formatting changes due to the rustfmt bump.
Include Cargo.lock update due to Clippy version bump and ui_test bump in Clippy.
Separate collection of crate-local inherent impls from error tracking
#119895 changed the return type of the `crate_inherent_impls` query from `CrateInherentImpls` to `Result<CrateInherentImpls, ErrorGuaranteed>` to avoid needing to use the non-parallel-friendly `track_errors()` to track if an error was reporting from within the query... This was mostly fine until #121113, which stopped halting compilation when we hit an `Err(ErrorGuaranteed)` in the `crate_inherent_impls` query.
Thus we proceed onwards to typeck, and since a return type of `Result<CrateInherentImpls, ErrorGuaranteed>` means that the query can *either* return one of "the list inherent impls" or "error has been reported", later on when we want to assemble method or associated item candidates for inherent impls, we were just treating any `Err(ErrorGuaranteed)` return value as if Rust had no inherent impls defined anywhere at all! This leads to basically every inherent method call failing with an error, lol, which was reported in #127798.
This PR changes the `crate_inherent_impls` query to return `(CrateInherentImpls, Result<(), ErrorGuaranteed>)`, i.e. returning the inherent impls collected *and* whether an error was reported in the query itself. It firewalls the latter part of that query into a new `crate_inherent_impls_validity_check` just for the `ensure()` call.
This fixes#127798.
