Implement pin-project in pattern matching for &pin mut|const T#139751
Implement pin-project in pattern matching for &pin mut|const T#139751bors merged 8 commits intorust-lang:masterfrom
&pin mut|const T#139751Conversation
|
rustbot has assigned @compiler-errors. Use |
rustbot
added
A-attributes
This comment has been minimized.
This comment has been minimized.
|
☔ The latest upstream changes (presumably #139996) made this pull request unmergeable. Please resolve the merge conflicts. |
frank-king
force-pushed
the
feature/pin-project
branch
from
0ad1543 to
e9c97df
Compare
This comment has been minimized.
This comment has been minimized.
frank-king
force-pushed
the
feature/pin-project
branch
3 times, most recently
from
24e0b14 to
aa27a06
Compare
frank-king
changed the title
&pin mut|const T&pin mut|const T
This comment has been minimized.
This comment has been minimized.
frank-king
force-pushed
the
feature/pin-project
branch
from
aa27a06 to
c9ca4f8
Compare
This comment has been minimized.
This comment has been minimized.
|
Some changes occurred to the CTFE machinery Some changes occurred in match checking cc @Nadrieril Some changes occurred in compiler/rustc_codegen_ssa Some changes occurred in src/tools/clippy cc @rust-lang/clippy Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt The Miri subtree was changed cc @rust-lang/miri Some changes occurred in compiler/rustc_monomorphize/src/partitioning/autodiff.rs cc @ZuseZ4 rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead. cc @rust-lang/rust-analyzer Some changes occurred in exhaustiveness checking cc @Nadrieril Some changes occurred in match lowering cc @Nadrieril Some changes occurred to the CTFE / Miri interpreter cc @rust-lang/miri Some changes occurred in compiler/rustc_codegen_cranelift cc @bjorn3 |
oli-obk
added
S-waiting-on-author
|
It seems like you're implementing a sort of match ergonomics through I think pinnedness should be part of the binding mode instead of a separate notion. I haven't looked deeply yet but presumably this will raise questions similar to deref patterns and the recent match ergonomics changes. cc @dianne |
|
+1 to representing pinnedness as part of the binding mode. Regarding match ergonomics interactions, I think we'll either want explicit Regarding deref patterns interactions, we'll probably also want pin ergonomics for deref patterns eventually. I don't think that'll be a problem, if I understand what |
|
Another ergonomics question: is there a way to get a non-pinned by-shared-ref binding when matching through |
Does "match ergonomics" refer to rfcs#2005? I see it has been stabilized in 2018, and I'm not quite familiar with "ancient" Rust before (I started to learn Rust in 2021). My intuition is just based on the crate pin_project. Take an example from its doc: use std::pin::Pin;
use pin_project::pin_project;
#[pin_project(project = EnumProj)]
enum Enum<T, U> {
Pinned(#[pin] T),
Unpinned(U),
}
impl<T, U> Enum<T, U> {
fn method(self: Pin<&mut Self>) {
match self.project() {
EnumProj::Pinned(x) => {
let _: Pin<&mut T> = x;
}
EnumProj::Unpinned(y) = {
let _: &mut U = y;
}
}
}
}It uses the With #![feature(pin_ergonomics)]
enum Enum<T, U> {
// `#[pin]` is no longer needed, as we can infer from the trait bound whether `T` is `Unpin`.
Pinned(T),
Unpinned(U),
}
// `U: Unpin` is needed to inform the compiler that `U` can be projected to a normal reference.
impl<T, U: Unpin> Enum<T, U> {
fn method(&pin mut self) {
// `self.projection()` is no longer needed, as the compiler
// would understand how to project a `&pin mut Enum<T, U>`
match self {
// `EnumProj` is no longer needed
Enum::Pinned(x) => {
// for `T: ?Unpin`, it is projected to `&pin mut T`
let _: &pin mut T = x;
}
Enum::Unpinned(y) = {
// for `U: Unpin`, it is projected to `&mut U`
let _: &mut U = y;
}
}
}
}That's how I implemented this PR. |
That RFC is indeed pretty old, and isn't quite what's implemented in Rust 20241. I'm using "match ergonomics" loosely to mean a couple things (sorry for the jargon!):
From what I could tell, this PR supports the former of these but not the latter; for consistency with how matching on normal references works, I'd expect to be able to use explicit reference patterns to match on
It should indeed be possible to utilize the There might be additional subtleties/complications I'm not aware of, of course. I'm not deeply familiar with the trait solver, so I'm not totally sure how unambiguous you can guarantee its results to be. Hence my suggestion to raise an error when there's ambiguities. Footnotes
|
This comment was marked as duplicate.
This comment was marked as duplicate.
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
This error is weird and I cannot reproduce it by testing the @rustbot ready. |
|
@bors2 try |
This comment has been minimized.
This comment has been minimized.
|
@bors r=Nadrieril,traviscross rollup |
|
I think it's the one which failed in #148296 (comment). |
|
Strange... @bors r- |
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as resolved.
This comment was marked as resolved.
| std::arch::global_asm! { | ||
| "{}", | ||
| #[pin_v2] //~ ERROR this attribute is not supported on assembly | ||
| const 0 | ||
| } |
There was a problem hiding this comment.
This has to be limited to architectures on which it's supported.
(Thanks to @ehuss for spotting this.)
There was a problem hiding this comment.
Oh, I was misled by the stderr diff. The actual stderr output was clear.
There was a problem hiding this comment.
I don't (quickly) find how to restrict the UI test to specific target archs only, and #[cfg(target_arch = "xxx")] would make different errors on different archs that would fail the test, so I removed it as it is not quite important here.
This comment has been minimized.
This comment has been minimized.
|
@rustbot ready |
|
@bors2 try jobs=test-various |
This comment has been minimized.
This comment has been minimized.
|
@bors r=Nadrieril,traviscross rollup |
17 participants
This PR implements part of #130494. It supports pin-project in pattern matching for
&pin mut|const T.Pin-projection by field access (i.e.&pin mut|const place.field) is not fully supported yet since pinned-borrow is not ready (#135731).CC @traviscross