Avoid redundant clone suggestions in borrowck diagnostics
Fixesrust-lang/rust#153886
Removed redundant `.clone()` suggestions.
I found that there are two patterns to handle this issue while I was implementing:
- Should suggest only UFCS
- Should suggest only simple `.clone()`
For the target issue, we can just remove the UFCS (`<Option<String> as Clone>::clone(&selection.1)`) side.
However, for the `BorrowedContentSource::OverloadedDeref` pattern like `Rc<Vec<i32>>`, for instance the `borrowck-move-out-of-overloaded-auto-deref.rs` test case, I think we need to employ the UFCS way. The actual test case is:
```rust
//@ run-rustfix
use std::rc::Rc;
pub fn main() {
let _x = Rc::new(vec![1, 2]).into_iter();
//~^ ERROR [E0507]
}
```
And another error will be shown if we simply use the simple `.clone()` pattern. Like:
```rust
use std::rc::Rc;
pub fn main() {
let _x = Rc::new(vec![1, 2]).clone().into_iter();
}
```
then we will get
```
error[E0507]: cannot move out of an `Rc`
--> src/main.rs:5:14
|
5 | let _x = Rc::new(vec![1, 2]).clone().into_iter();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ ----------- value moved due to this method call
| |
| move occurs because value has type `Vec<i32>`, which does not implement the `Copy` trait
|
note: `into_iter` takes ownership of the receiver `self`, which moves value
--> /playground/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/iter/traits/collect.rs:310:18
|
310 | fn into_iter(self) -> Self::IntoIter;
| ^^^^
help: you can `clone` the value and consume it, but this might not be your desired behavior
|
5 - let _x = Rc::new(vec![1, 2]).clone().into_iter();
5 + let _x = <Vec<i32> as Clone>::clone(&Rc::new(vec![1, 2])).into_iter();
|
For more information about this error, try `rustc --explain E0507`.
```
[Rust Playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=7e767bed3f1c573c03642f20f454ed03)
In this case, `Rc::clone` only increments the reference count and returns a new `Rc<Vec<i32>>`; it does not grant ownership of the inner `Vec<i32>`. As a result, calling into_iter() attempts to move the `Vec<i32>`, leading to the same E0507 error again.
On the other hand, in UFCS form:
```
<Vec<i32> as Clone>::clone(&Rc::new(vec![1, 2])).into_iter()
```
This explicitly calls `<Vec<i32> as Clone>::clone`, and the argument `&Rc<Vec<i32>>` is treated as `&Vec<i32>` via Rc’s `Deref` implementation. As a result, the `Vec<i32>` itself is cloned, yielding an owned `Vec<i32>`, which allows `into_iter()` to succeed, if my understanding is correct.
I addressed the issue as far as I could find the edge cases but please advice me if I'm overlooking something.
Rollup of 3 pull requests
Successful merges:
- rust-lang/rust#155754 (make the `core::ffi::va_list` module private)
- rust-lang/rust#155522 (cmse: test returning `MaybeUninit<T>`)
- rust-lang/rust#155741 (std: Refactor BufWriter::flush to use the `?` operator)
cmse: test returning `MaybeUninit<T>`
tracking issue: https://github.com/rust-lang/rust/issues/81391
tracking issue: https://github.com/rust-lang/rust/issues/75835
Some tests from https://github.com/rust-lang/rust/pull/147697 that already work and are useful. Extracting them shrinks that (currently blocked) PR.
The code in `tests/ui/cmse-nonsecure/cmse-nonsecure-call/return-via-stack.rs` checks that `MaybeUninit<T>` is considered abi-compatible with `T`. The code in `tests/ui/cmse-nonsecure/cmse-nonsecure-entry/params-via-stack.rs` really only tests that no errors/warnings are emitted.
r? davidtwco
Permit `{This}` in diagnostic attribute format literals
My motivation was that yesterday I wanted to write something like this and reference `$name` in the string literal.
```rust
pub mod sym {
// stuff here
}
macro_rules! my_macro {
($name:ident $(,)?) => {{
#[diagnostic::on_unknown(
message = "this is not present in symbol table",
note = "you must add it to rustc_span::symbol::symbol!"
)]
use sym::$name as name;
// ...
}}
}
```
That is (as far as I can tell) impossible or at least very unergonomic. This adds the ability to just reference the name of the item the attribute is on. I imagine that's useful for use inside macros generally, so it's also added for some other attributes.
The affected attributes are all unstable, it is not implemented for diagnostic::on_unimplemented (will do in its own PR).
Note that `{This}` is already usable in `#[rustc_on_unimplemented]`, so this does not implement it but just enables some more.
This PR also migrates one lint away from AttributeLintKind, and improves the messages for that lint.
Rollup of 12 pull requests
Successful merges:
- rust-lang/rust#149452 (Refactor out common code into a `IndexItem::new` constructor)
- rust-lang/rust#155621 (Document #[diagnostic::on_move] in the unstable book.)
- rust-lang/rust#155635 (delegation: rename `Self` generic param to `This` in recursive delegations)
- rust-lang/rust#155730 (Some cleanups around per parent disambiguators)
- rust-lang/rust#153537 (rustc_codegen_ssa: Define ELF flag value for sparc-unknown-linux-gnu)
- rust-lang/rust#155219 (Do not suggest borrowing enclosing calls for nested where-clause obligations)
- rust-lang/rust#155408 (rustdoc: Fix Managarm C Library name in cfg pretty printer)
- rust-lang/rust#155571 (Enable AddressSanitizer on arm-unknown-linux-gnueabihf and armv7-unknown-linux-gnueabihf)
- rust-lang/rust#155713 (test: Add a regression test for Apple platforms aborting on `free`)
- rust-lang/rust#155723 (Fix tier level for 5 thumb bare-metal ARM targets)
- rust-lang/rust#155735 (Fix typo by removing extra 'to')
- rust-lang/rust#155736 (Remove `AllVariants` workaround for rust-analyzer)
Remove `AllVariants` workaround for rust-analyzer
Part of https://github.com/rust-lang/rust/issues/155677
Removes the `ALL_VARIANTS` alias added to work around rust-analyzer not supporting `#![feature(macro_derive)]`, which has since been fixed (rust-lang/rust-analyzer/issues/21043).
Fix typo by removing extra 'to'
Fixesrust-lang/rust#155695
Fix a typo in the `std::convert` module documentation by removing an extra "to" in the module-level docs.
Fix tier level for 5 thumb bare-metal ARM targets
The spec files for 5 Thumb-mode bare-metal ARM targets incorrectly set tier: Some(2), while the documentation correctly lists them as Tier 3. This mismatch was introduced in PR #150556 — the intent was Tier 2 eventually, but these targets should sit at Tier 3 until a proper Tier 3 → Tier 2 promotion MCP is submitted and approved.
This PR changes tier: Some(2) → Some(3) in the following spec files, making them consistent with the docs:
thumbv7a-none-eabi
thumbv7a-none-eabihf
thumbv7r-none-eabi
thumbv7r-none-eabihf
thumbv8r-none-eabihf
PS: No doc changes needed — they already correctly state Tier 3.
r?
test: Add a regression test for Apple platforms aborting on `free`
Add a regression test for https://github.com/rust-lang/rust/issues/150898 to make users aware that if this test failures, they may encounter unusual behavior elsewhere.
Enable AddressSanitizer on arm-unknown-linux-gnueabihf and armv7-unknown-linux-gnueabihf
Add SanitizerSet::ADDRESS to the supported_sanitizers for the arm-unknown-linux-gnueabihf and armv7-unknown-linux-gnueabihf targets.
The AddressSanitizer is already enabled on the armv7-linux-androideabi platform, which shares the same ARM architecture. There is no reason these Linux GNU targets should not also support it, as the underlying LLVM support for ASan on 32-bit ARM is already in place.
Do not suggest borrowing enclosing calls for nested where-clause obligations
In rust-lang/rust#155088, the compiler was blaming the whole call expr instead of the value that actually failed the trait bound, so for foo(&[String::from("a")]) it was suggesting stuff like &foo(...). I changed the suggestion logic so it only emits borrow help if the expr it found actually matches the failed self type, and used the same check for the “similar impl exists” help too. So now the compiler should give the normal error + required bound note.
Fixrust-lang/rust#155088
Some cleanups around per parent disambiguators
r? @petrochenkov
follow-up to rust-lang/rust#155547
The two remaining uses are
* resolve_bound_vars, where it is a reasonable way to do it instead of having another field in the visitor that needs to get scoped (set & reset) every time we visit an opaque type. May still change that at some point, but it's not really an issue
* `create_def` in the resolver: will get removed together with my other refactorings for `node_id_to_def_id` (making that per-owner)
delegation: rename `Self` generic param to `This` in recursive delegations
This PR supports renaming of `Self` generic parameter to `This` in recursive delegations scenario, this allows propagation of `This` as we rely on `Self` naming to check whether it is implicit Self of a trait. Comment with a bit deeper explanation is in `uplift_delegation_generic_params`. Part of rust-lang/rust#118212.
r? @petrochenkov
Add a regression test for RUST-150898 to make users aware that if this
test fails, they may encounter unusual behavior elsewhere.
Original repro authored by dianqk.
Forbid `*-pass` and `*-fail` directives in tests/crashes
Crash tests are always expected to crash during compilation, so there is no sensible meaning for specifying a pass expectation or a run-fail expectation in a crash test.
It could conceivably be useful to use failure expectations to specify whether a crash test requires codegen in order to crash, but currently none of the crash tests try to do that. If that functionality is desired in the future, we can always look into re-adding it after the internals of pass/fail expectations have been cleaned up a bit.
---
After this change, pass/fail directives are only allowed in UI tests, which should make it easier to overhaul and simplify their implementation.
r? jieyouxu
Remove `AttributeLintKind` variants - part 7
Part of https://github.com/rust-lang/rust/issues/153099.
It's the last easy one. Next one will require to get the crate name and to pass `Session` to the remaining lints. Fun times ahead. :)
r? @JonathanBrouwer
Remove myself as a maintainer of `wasm32-wasip1-threads`
Over time the landscape for myself has changed, and I no longer would like to be officially listed as a maintainer of this target in Rust, so I'm going to step down. There are still a number of others listed on this target, however, so I'm sure they can address issues should they come up.
Syntactically reject tuple index shorthands in struct patterns to fix a correctness regression
Split out of PR rust-lang/rust#154492. This fixes a correctness regression introduced in PR rust-lang/rust#81235 from 2021. Crater was run in my other PR and didn't report any real regressions (https://github.com/rust-lang/rust/pull/154492#issuecomment-4187544786); a rerun has been issued for a few spurious builds (https://github.com/rust-lang/rust/pull/154492#issuecomment-4237077272) but I'm certain it won't find anything either.
This is a theoretical breaking change that doesn't need any T-lang input IMHO since it's such a minute, niche and crystal clear bug that's not worth bothering them with (such a decision is not unprecedented). I'm adding it to the compatibility section of the release notes as is customary.
The Reference doesn't need updating since it didn't adopt this bug and thus accurately describes this part of the grammar as it used to be before 2021-02-23 and as it's meant to be.
The majority of the diff is doc comment additions & necessary UI test restructurings.
Eliminate `CrateMetadataRef`.
There are a number of things I dislike about `CrateMetadataRef`.
- It contains two fields `cstore` and `cdata`. The latter points to data within the former. It's like having an `Elem` type that has a reference to a vec element and also a reference to the vec itself. Weird.
- The `cdata` field gets a lot of use, and the `Deref` impl just derefs that field. The `cstore` field is rarely used.
- `CrateMetadataRef` is not a good name.
- Variables named `cdata` sometimes refer to values of this type and sometimes to values of type `CrateMetadata`, which is confusing.
The good news is that `CrateMetadataRef` is not necessary and can be replaced with `&CrateMetadata`. Why? Everywhere that `CrateMetadataRef` is used, a `TyCtxt` is also present, and the `CStore` is accessible from the `TyCtxt` with `CStore::from_tcx`.
So this commit removes `CrateMetadataRef` and replaces all its uses with `&CrateMetadata`. Notes:
- This requires adding only two uses of `CStore::from_tcx`, which shows how rarely the `cstore` field was used.
- `get_crate_data` now matches `get_crate_data_mut` more closely.
- A few variables are renamed for consistency, e.g. `data`/`cmeta` -> `cdata`.
- An unnecessary local variable (`local_cdata`) in `decode_expn_id` is removed.
- All the `CrateMetadataRef` methods become `CrateMetadata` methods, and their receiver changes from `self` to `&self`.
- `RawDefId::decode_from_cdata` is inlined and removed, because it has a single call site.
r? @mejrs
Move and clean up some ui test
`ui/reserved` -> `ui/keyword`
`ui/deref-patterns` -> `ui/pattern/deref-patterns`
`ui/unknown-unstable-lints` -> `ui/lint/unknown-lints`
Tests related to unknown_lints that were located above lint have also been moved to a subdirectory, and duplicate tests have been deleted.
And delete unnecessary `//@ check-fail`
r? Kivooeo
Generalize IO Traits for `Arc<T>` where `&T: IoTrait`
ACP: https://github.com/rust-lang/libs-team/issues/755
Tracking issue: https://github.com/rust-lang/rust/issues/154046
Related: rust-lang/rust#94744
## Description
After experimenting with rust-lang/rust#155625, I noticed `Seek` and `SeekFrom` can almost be moved to `core::io`. Unfortunately, the implementation of `Seek` for `Arc<File>` is a blocker for such a move, since `Arc` is not a fundamental type. This PR attempts to resolve this potential blocker by replacing the implementation with a more general alternative. An internal trait `IoHandle` has been added which types can implement to opt-in to `Read`/`Write`/`Seek` implementations for `Arc<Self>` as long as `&Self` implements said trait. Note that `BufRead` is excluded as the signature for `fill_buf` would require returning from a temporary.
Since this "blanket" implementation only applies to a single type which already implements the same traits, I believe this should have no user-facing impact.
If this PR was merged, rust-lang/rust#134190 could be replaced with a 2 line PR:
```rust
impl IoHandle for TcpStream {}
impl IoHandle for UnixStream {}
```
Likewise for any other types, a table of which can be found [here](https://github.com/rust-lang/libs-team/issues/504#issuecomment-2539569736). This is out of scope for this PR to avoid the need for an ACP.
---
## Notes
* See [this comment](https://github.com/rust-lang/rust/issues/154046#issuecomment-4303975612) for further details.
* No AI tooling of any kind was used during the creation of this PR.
Added a marker trait `IoHandle` which can be used by the standard library to opt-in types to a blanket implementation of the various IO traits on `Arc<T>` where `&T: IoTrait` for some `IoTrait`.
The marker is required to avoid types like `Arc<[u8]>` being included, since they don't have interior mutability and would not give expected results.
Jacob Pratt