Separate lifetime ident from lifetime resolution in HIR
Drive-by: change how suggested generic args are computed.
Fixes https://github.com/rust-lang/rust/issues/103815
I recommend reviewing commit-by-commit.
Make rustc_target usable outside of rustc
I'm working on showing type size in rust-analyzer (https://github.com/rust-lang/rust-analyzer/pull/13490) and I currently copied rustc code inside rust-analyzer, which works, but is bad. With this change, I would become able to use `rustc_target` and `rustc_index` directly in r-a, reducing the amount of copy needed.
This PR contains some feature flag to put nightly features behind them to make crates buildable on the stable compiler + makes layout related types generic over index type + removes interning of nested layouts.
Avoid `GenFuture` shim when compiling async constructs
Previously, async constructs would be lowered to "normal" generators, with an additional `from_generator` / `GenFuture` shim in between to convert from `Generator` to `Future`.
The compiler will now special-case these generators internally so that async constructs will *directly* implement `Future` without the need to go through the `from_generator` / `GenFuture` shim.
The primary motivation for this change was hiding this implementation detail in stack traces and debuginfo, but it can in theory also help the optimizer as there is less abstractions to see through.
---
Given this demo code:
```rust
pub async fn a(arg: u32) -> Backtrace {
let bt = b().await;
let _arg = arg;
bt
}
pub async fn b() -> Backtrace {
Backtrace::force_capture()
}
```
I would get the following with the latest stable compiler (on Windows):
```
4: async_codegen:🅱️:async_fn$0
at .\src\lib.rs:10
5: core::future::from_generator::impl$1::poll<enum2$<async_codegen:🅱️:async_fn_env$0> >
at /rustc/897e37553bba8b42751c67658967889d11ecd120\library\core\src\future\mod.rs:91
6: async_codegen:🅰️:async_fn$0
at .\src\lib.rs:4
7: core::future::from_generator::impl$1::poll<enum2$<async_codegen:🅰️:async_fn_env$0> >
at /rustc/897e37553bba8b42751c67658967889d11ecd120\library\core\src\future\mod.rs:91
```
whereas now I get a much cleaner stack trace:
```
3: async_codegen:🅱️:async_fn$0
at .\src\lib.rs:10
4: async_codegen:🅰️:async_fn$0
at .\src\lib.rs:4
```
Previously, async constructs would be lowered to "normal" generators,
with an additional `from_generator` / `GenFuture` shim in between to
convert from `Generator` to `Future`.
The compiler will now special-case these generators internally so that
async constructs will *directly* implement `Future` without the need
to go through the `from_generator` / `GenFuture` shim.
The primary motivation for this change was hiding this implementation
detail in stack traces and debuginfo, but it can in theory also help
the optimizer as there is less abstractions to see through.
Add `clippy_utils::msrv::Msrv` to keep track of the current MSRV
changelog: Fix the scoping of the `#![clippy::msrv]` attribute
Fixes#6920
r? `@Jarcho`
Update Clippy
r? `@Manishearth`
Sorry for taking so long. There were so many blockers and so little time. This situation should be mitigated with #104007 in the future.
Fix#9771 (`unnecessary_to_owned` false positive)
Fixes#9771
In that issue's example(s), the lint tried to add a `&` to a value, which implicitly changed the type of a field to a reference. The fix is to add the reference to `receiver_ty` (the type of the receiver of the `to_owned`-like method), before passing `receiver_ty` to `can_change_type`. `can_change_type` properly rejects the modified `receiver_ty`.
cc: `@mikerite` just because I think he was the author of `can_change_type`.
changelog: fix `unnecessary_to_owned` false positive which implicitly tried to change the type of a field to a reference
Fix `redundant_closure_for_method_calls` suggestion
Fixes#7746. The issue turns out to be more general than raw pointers. The `redundant_closure_for_method_calls` lint produces incorrect suggestions when the method is associated with a type that must be enclosed in angle brackets or must be written with generic arguments substituted. For example:
```rust
fn main() {
// Clippy's suggestion: [T; N]::as_slice
// Correct suggestion: <[u8; 3]>::as_slice
let array_opt: Option<&[u8; 3]> = Some(&[4, 8, 7]);
array_opt.map(|a| a.as_slice());
// Clippy's suggestion: [T]::len
// Correct suggestion: <[u8]>::len
let slice_opt: Option<&[u8]> = Some(b"slice");
slice_opt.map(|s| s.len());
// Clippy's suggestion: *const T::is_null
// Correct suggestion: <*const usize>::is_null
let ptr_opt: Option<*const usize> = Some(&487);
ptr_opt.map(|p| p.is_null());
// Clippy's suggestion: dyn TestTrait::method_on_dyn
// Correct suggestion: <dyn TestTrait>::method_on_dyn
let test_struct = TestStruct {};
let dyn_opt: Option<&dyn TestTrait> = Some(&test_struct);
dyn_opt.map(|d| d.method_on_dyn());
}
// For the trait object example:
trait TestTrait {}
struct TestStruct {}
impl TestTrait for TestStruct {}
impl dyn TestTrait + '_ {
fn method_on_dyn(&self) -> bool {
false
}
}
```
The issue also affects references and tuples, though I had to patch the standard library with non-trait methods for those types to test that. Just in case, I also included handling for `!`, since it appeared to be possible to call methods on it with angle brackets. I just couldn't verify the resulting suggestion, since dead-code analysis eliminates the code first.
This is my first exposure to Rust compiler internals, so please let me know if I'm taking the wrong approach here!
changelog: [`redundant_closure_for_method_calls`]: add angle brackets and substitute generic arguments in suggestion when needed
Rollup of 11 pull requests
Successful merges:
- #103396 (Pin::new_unchecked: discuss pinning closure captures)
- #104416 (Fix using `include_bytes` in pattern position)
- #104557 (Add a test case for async dyn* traits)
- #104559 (Split `MacArgs` in two.)
- #104597 (Probe + better error messsage for `need_migrate_deref_output_trait_object`)
- #104656 (Move tests)
- #104657 (Do not check transmute if has non region infer)
- #104663 (rustdoc: factor out common button CSS)
- #104666 (Migrate alias search result to CSS variables)
- #104674 (Make negative_impl and negative_impl_exists take the right types)
- #104692 (Update test's cfg-if dependency to 1.0)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
`MacArgs` is an enum with three variants: `Empty`, `Delimited`, and `Eq`. It's
used in two ways:
- For representing attribute macro arguments (e.g. in `AttrItem`), where all
three variants are used.
- For representing function-like macros (e.g. in `MacCall` and `MacroDef`),
where only the `Delimited` variant is used.
In other words, `MacArgs` is used in two quite different places due to them
having partial overlap. I find this makes the code hard to read. It also leads
to various unreachable code paths, and allows invalid values (such as
accidentally using `MacArgs::Empty` in a `MacCall`).
This commit splits `MacArgs` in two:
- `DelimArgs` is a new struct just for the "delimited arguments" case. It is
now used in `MacCall` and `MacroDef`.
- `AttrArgs` is a renaming of the old `MacArgs` enum for the attribute macro
case. Its `Delimited` variant now contains a `DelimArgs`.
Various other related things are renamed as well.
These changes make the code clearer, avoids several unreachable paths, and
disallows the invalid values.
Add new lint [`misnamed-getters`]
```
changelog: Add new lint [`misnamed-getters`]
```
Closes#9769
The current lint matches all methods with a body of just one expression under the form `(&mut?)? <expr>.field` where field doesn't match the name of the method but there is a field of the same type in `<expr>` that matches the name. This allows matching nested structs, for example for newtype wrappers. This may cast the net a bit too wide and cause false positives. I'll run [clippy_lint_tester](https://github.com/mikerite/clippy_lint_tester) on the top crates to see how frequently false positives happen.
There also may be room for improvement by checking that the replacement field would work taking into account implementations of `Deref` and `DerefMut` even if the types don't exactly match but I don't know yet how this could be done.
Add `PolyExistentialPredicate` type alias
Wrapping `ExistentialPredicate`s in a binder is very common, and this alias already exists for the `PolyExistential{TraitRef,Projection}` types.
[arithmetic-side-effects] Detect overflowing associated constants of integers
Triggers the negation of maximum unsigned integers using associated constants. Rustc already handles `-128i8` but doesn't handle `-i8::MAX`.
At the same time, allows stuff like `-1234`.
changelog: FP: [arithmetic-side-effects] Detect overflowing associated constants of integers
Keep original literal notation in suggestion
While I did some investigation of https://github.com/rust-lang/rust-clippy/issues/9866 (I couldn't reproduce it though) I found that `unused_rounding` formats as follows:
```rust
3.0_f64.round() // => 3.0f64
```
This PR makes them preserve as the original notation.
```rust
3.0_f64.round() // => 3.0_f64
```
changelog: Suggestion Enhancement: [`unused_rounding`]: The suggestion now preserves the original float literal notation
Improve spans for RPITIT object-safety errors
No reason why we can't point at the `impl Trait` that causes the object-safety violation.
Also [drive-by: Add is_async fn to hir::IsAsync](c4165f3a96), which touches clippy too.
Return multiple resolutions from `def_path_res`
Changes `def_path_res` to return all the resolutions matching the path rather than the first one (with a namespace hint that covered some cases). This would fix any issues that come up with multiple versions of the same crate being present as they all have the same crate name
It also adds resolution of `impl _ {}` items for local items, and removes struct field resolution as it didn't seem to be used anywhere
I tested it on a local crate and it worked for the multiple crate issue, but I couldn't come up with a test that worked well with `// aux-build`, maybe `// aux-crate` after https://github.com/rust-lang/rust/pull/103266 could work but I'm not sure on that either
changelog: [`disallowed_methods`], [`disallowed_types`], [`disallowed_macros`]: fix path resolution with multiple versions of the same crate
changelog: [`disallowed_methods`]: Resolve methods in `impl`s in the current crate
Rollup of 8 pull requests
Successful merges:
- #101162 (Migrate rustc_resolve to use SessionDiagnostic, part # 1)
- #103386 (Don't allow `CoerceUnsized` into `dyn*` (except for trait upcasting))
- #103405 (Detect incorrect chaining of if and if let conditions and recover)
- #103594 (Fix non-associativity of `Instant` math on `aarch64-apple-darwin` targets)
- #104006 (Add variant_name function to `LangItem`)
- #104494 (Migrate GUI test to use functions)
- #104516 (rustdoc: clean up sidebar width CSS)
- #104550 (fix a typo)
Failed merges:
- #104554 (Use `ErrorGuaranteed::unchecked_claim_error_was_emitted` less)
r? `@ghost`
`@rustbot` modify labels: rollup
Extend `needless_borrowed_reference` to structs and tuples, ignore _
changelog: [`needless_borrowed_reference`]: Lint struct and tuple patterns, and patterns containing `_`
Now lints patterns like
```rust
&(ref a, ref b)
&Tuple(ref a, ref b)
&Struct { ref a, ref b }
&(ref a, _)
```
Add variant_name function to `LangItem`
Clippy has an internal lint that checks for the usage of hardcoded def paths and suggests to replace them with a lang or diagnostic item, if possible. This was implemented with a hack, by getting all the variants of the `LangItem` enum and then index into it with the position of the `LangItem` in the `items` list. This is no longer possible, because the `items` list can't be accessed anymore.
Follow up to #103603
cc `@camsteffen`
r? `@oli-obk`
This is blocking the sync between Clippy and Rust. I'm not sure if this is the best solution here, or if I should add a method `items()` to `LanguageItems` and keep the code in Clippy unchanged.
Fix typo in `expect_used` and `unwrap_used` warning messages
"\`an Option\`" -> "an \`Option\`" and "\`a Result\`" -> "a \`Result\`".
changelog: fix typo in `expect_used` and `unwrap_used` warning messages
`never_loop`: don't emit AlwaysBreaks if it targets a block
ref: https://github.com/rust-lang/rust-clippy/pull/9837#issuecomment-1312788194
The previous fix (#9837) was too simple and ignored all break commands inside a labelled block, regardless of whether their destination was a labelled block or a loop. This fix tracks all the labelled blocks in scope to ensure that only breaks targeting loops are considered.
changelog: [`never_loop`]: prevent false negatives from `breaks` nested in labelled blocks
Convert predicates into Predicate in the Obligation constructor
instead of having almost all callers do that.
This reduces a bit of boilerplate, and also paves the way for my work towards https://github.com/rust-lang/compiler-team/issues/531 (as it makes it easier to accept both goals and clauses where right now it only accepts predicates).
Clippy has an internal lint that checks for the usage of hardcoded def
paths and suggests to replace them with a lang or diagnostic item, if
possible. This was implemented with a hack, by getting all the variants
of the `LangItem` enum and then index into it with the position of the
`LangItem` in the `items` list. This is no longer possible, because the
`items` list can't be accessed anymore.
Record `LocalDefId` in HIR nodes instead of a side table
This is part of an attempt to remove the `HirId -> LocalDefId` table from HIR.
This attempt is a prerequisite to creation of `LocalDefId` after HIR lowering (https://github.com/rust-lang/rust/pull/96840), by controlling how `def_id` information is accessed.
This first part adds the information to HIR nodes themselves instead of a table.
The second part is https://github.com/rust-lang/rust/pull/103902
The third part will be to make `hir::Visitor::visit_fn` take a `LocalDefId` as last parameter.
The fourth part will be to completely remove the side table.
When we run `cargo test` in `clippy_lints` directory, it will generate
`foo.txt` in the directory.
In order to avoid that, add `no_run` to rustdoc which contains
`File::create`.
Introduced an ignored_ids parameter.
Takes O(n^2) time in the worst case.
Can be changed to collect block ids in first phase,
and then filter with binary search in second.
Instead of `ast::Lit`.
Literal lowering now happens at two different times. Expression literals
are lowered when HIR is crated. Attribute literals are lowered during
parsing.
This commit changes the language very slightly. Some programs that used
to not compile now will compile. This is because some invalid literals
that are removed by `cfg` or attribute macros will no longer trigger
errors. See this comment for more details:
https://github.com/rust-lang/rust/pull/102944#issuecomment-1277476773
feat: lint unchecked subtraction of a 'Duration' from an 'Instant'
Hello all, I tried to tackle the open issue #9371 and this is what I came up with.
I have a difficulty currently - some tests are failing:
```
failures:
[ui] ui/manual_instant_elapsed.rs
```
The `manual_instant_elapsed` is failing because of `Instant::now() - duration` test, this now gets also picked by `unchecked_duration_subtraction` lint.
What is the correct way to proceed in this case? Simply update the `.stderr` file for `manual_instant_elapsed` lint?
changelog: [`unchecked_duration_subtraction`]: Add lint for unchecked subtraction of a `Duration` from an `Instant`.
fixes#9371
Make it clear that `or_fun_call` can be a false-positive
Also move it to nursery so that the false-positives can be dealt with.
CC #8574
changelog: [`or_fun_call`]: Mention false-positives, move to nursery.
Add `unnecessary_safety_doc` lint
changelog: [`unnecessary_safety_doc`]: Add `unnecessary_safety_doc` lint
fixes https://github.com/rust-lang/rust-clippy/issues/6880
This lint does not trigger for private functions, just like `missing_safety_docs`. Reason for that was implementation simplicity and because I figured asking first would make more sense, so if it should trigger for private functions as well let me know and I'll fix that up as well.
[`fn_params_excessive_bools`] Make it possible to allow the lint at the method level
changelog: FP: [`fn_params_excessive_bools`]: `#[allow]` now works on methods
fix https://github.com/rust-lang/rust-clippy/issues/9687
Tested without committing but `#[allow]`ing now works. Also rewrote the lint to be a late lint while at it :)
r? `@xFrednet`
In all the cases I've observed, it did not make the code clearer. Using
bools as integer is frowned upon in some languages, in others it's
simply not possible.
You can find comments on the original pull request #8131 that agree with
this point of view.
Fix `explicit_auto_deref` fp
fixes#9763fixes#9811
changelog: `explicit_auto_deref`: Don't lint when the target type is a projection with generic arguments
Update lint example for `collapsible_str_replace`
To fix this warning:
```
warning: the borrowed expression implements the required traits
--> src/fixes/quote_character.rs:16:43
|
16 | let hello = "hesuo worpd".replace(&['s', 'u', 'p'], "l");
| ^^^^^^^^^^^^^^^^ help: change this to: `['s', 'u', 'p']`
|
= note: `#[warn(clippy::needless_borrow)]` on by default
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
```
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: none
Address issues 9739 and 9782
This PR fixes#9739 in the manner I suggested in https://github.com/rust-lang/rust-clippy/issues/9739#issuecomment-1296802376.
This PR also fixes the compilation failures in #9782 (but doesn't address `@e00E's` other objections).
Fixes#9739
r? `@Jarcho`
changelog: Fix two `needless_borrow` false positives, one involving borrows in `if`-`else`s, the other involving qualified function calls
Add `manual_is_ascii_check` lint
Addresses https://github.com/rust-lang/rust-clippy/issues/9290
This PR adds new lint `manual_is_ascii_check`, which detects comparison with ascii ranges using `matches!` macros.
As I mentioned as following in the Issue;
> Yes, that's true. we'll start small and then grow it.
> So I'll try to handle matches! macro with single range as suggested above.
However during writing first version, I was thinking that the changes to support alphabetic and digits will be small patch, so I made a single PR in hope review cost can be reduced.
changelog: add new lint [`manual_is_ascii_check`]
r? `@xFrednet`
Move needless_collect to nursery
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: [`needless_collect`]: Move `needless_collect` to nursery (Now allow-by-default)
After chatting with a few folks, it seems like `needless_collect` is giving false positives pretty regularly (https://github.com/rust-lang/rust-clippy/issues?q=is%3Aissue+is%3Aopen+needless_collect). We're big supporters of clippy in Nushell, and it's one of the passes we require for CI, but we've had to disable this particular lint. Perhaps it should be moved to the nursery until it's improved?
(apologies if this isn't the right approach to disabling a lint by default. I tried to follow the idea I saw other PRs doing in the past)
Add allow-print-in-tests config
Add a config, allow-print-in-tests, that can be set in clippy.toml which allows the usage of `[e]print[ln]!` macros in tests.
Closes#9795
---
changelog: Enhancement: [print_stdout], [print_stderr]: Can now be enabled in test with the `allow-print-in-tests` config value
Changelog for Rust 1.65
Roses are red,
violets are blue,
another 6 weeks,
another changelog, yahoo!
This did not rhyme,
luckily, not a crime.
I want to say something smart,
but I am out of time.
---
Note for the reviewer. This PR is written, as if Rust was already released on 2022-11-03. Please review the changes and approve if you agree, but only merge it after the release. :)
---
changelog: none
Track where diagnostics were created.
This implements the `-Ztrack-diagnostics` flag, which uses `#[track_caller]` to track where diagnostics are created. It is meant as a debugging tool much like `-Ztreat-err-as-bug`.
For example, the following code...
```rust
struct A;
struct B;
fn main(){
let _: A = B;
}
```
...now emits the following error message:
```
error[E0308]: mismatched types
--> src\main.rs:5:16
|
5 | let _: A = B;
| - ^ expected struct `A`, found struct `B`
| |
| expected due to this
-Ztrack-diagnostics: created at compiler\rustc_infer\src\infer\error_reporting\mod.rs:2275:31
```
Improve `needless_lifetimes`
This PR makes the following improvements to `needless_lifetimes`.
* It fixes the following false negative, where `foo` is flagged but `bar` is not:
```rust
fn foo<'a>(x: &'a u8, y: &'_ u8) {}
fn bar<'a>(x: &'a u8, y: &'_ u8, z: &'_ u8) {}
```
* It flags more cases, generally. Previously, `needless_borrow` required *all* lifetimes to be used only once. With the changes, individual lifetimes are flagged for being used only once, even if not all lifetimes are.
* Finally, it tries to produce more clear error messages.
changelog: fix `needless_lifetimes` false negative involving functions with multiple unnamed lifetimes
changelog: in `needless_lifetimes`, flag individual lifetimes used only once, rather than require all lifetimes to be used only once
changelog: in `needless_lifetimes`, emit "replace with `'_`" warnings only when applicable, and point to a generic argument
Add lint for confusing use of `^` instead of `.pow`
fixes#4205
Adds a lint named [`confusing_xor_and_pow`], it warns the user when `a ^ b` is used as the `.pow()` function, it doesn't warn for Hex, Binary... etc.
---
changelog: New lint: [`confusing_xor_and_pow`]
Warn when `clippy::restriction` is enabled via the command line
Currently it catches `#![warn(clippy::restriction)]`, it'll now catch `-W clippy::restriction` from the CLI. Also tweaks the message slightly
changelog: [`blanket_clippy_restriction_lints`]: Warn when `clippy::restriction` is enabled via the command line
Accept `TyCtxt` instead of `TyCtxtAt` in `Ty::is_*` functions
Functions in answer:
- `Ty::is_freeze`
- `Ty::is_sized`
- `Ty::is_unpin`
- `Ty::is_copy_modulo_regions`
This allows to remove a lot of useless `.at(DUMMY_SP)`, making the code a bit nicer :3
r? `@compiler-errors`
spastorino noticed some silly expressions like `item_id.def_id.def_id`.
This commit renames several `def_id: OwnerId` fields as `owner_id`, so
those expressions become `item_id.owner_id.def_id`.
`item_id.owner_id.local_def_id` would be even clearer, but the use of
`def_id` for values of type `LocalDefId` is *very* widespread, so I left
that alone.
privacy: Rename "accessibility levels" to "effective visibilities"
And a couple of other naming and comment tweaks.
Related to https://github.com/rust-lang/rust/issues/48054
For `enum Level` I initially used naming `enum EffectiveVisibilityLevel`, but it was too long and inconvenient because it's used pretty often.
So I shortened it to just `Level`, if it needs to be used from some context where this name would be ambiguous, then it can be imported with renaming like `use rustc_middle::privacy::Level as EffVisLevel` or something.
fix `undocumented-unsafe-blocks` false positive
This fixes#9142 by iterating over the parent nodes as long as within a block, expression, statement, local, const or static.
---
changelog: none
Certain types must be enclosed in angle brackets and must have generic
arguments substituted to create a working suggestion. For example, if
`s` has type `&[u8]`, then `|s| s.len()` may be replaced with
`<[u8]>::len`. Previously, Clippy erroneously suggested `[T]::len`.
Ensure new_ret_no_self is not fired if impl Trait<Self> is returned.
Fix#7344: ensure new_ret_no_self is not fired if `impl Trait<Self>` is returned.
changelog: [`new_ret_no_self`]: No longer lints when `impl Trait<Self>` is returned
[`use_self`] fix suggestion when full path to struct was given
Previously the following wrong suggestion was given
```rust
impl Error for std::fmt::Error {
fn custom<T: std::fmt::Display>(_msg: T) -> Self {
- std::fmt::Error // Should lint
+ Self::Error // Should lint
}
}
```
Also remove known problem line related to #4140 since it's been closed, and refactor the lint
changelog: [`use_self`] fix suggestion when full path to struct was given
`question_mark` don't lint on `if let Err` with `else`
cc #9518
AFAICT the only time this would be a valid suggestion is the rather esoteric
```rust
let _ = if let Err(e) = x {
return Err(e);
} else {
// no side effects
x.unwrap()
}
```
which doesn't seem worth checking to me. Please correct me if I'm missing something.
changelog: [`question_mark`] don't lint on `if let Err` with `else`
Previously the following wrong suggestion was given
```rust
impl Error for std::fmt::Error {
fn custom<T: std::fmt::Display>(_msg: T) -> Self {
- std::fmt::Error // Should lint
+ Self::Error // Should lint
}
}
```
Also remove known problem line related to #4140 since it's been closed, and refactor the lint
