Implement derives for Reborrow and CoerceShared - #156338
Conversation
|
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 |
|
r? @tiif rustbot has assigned @tiif. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
c515771 to
1a83d37
Compare
This comment has been minimized.
This comment has been minimized.
1a83d37 to
8e2ead2
Compare
|
Reminder, once the PR becomes ready for a review, use |
8e2ead2 to
0d49f18
Compare
|
@rustbot ready |
| } | ||
|
|
||
| #[derive(Diagnostic)] | ||
| #[diag("`derive(CoerceShared)` requires exactly one `#[coerce_shared(Target)]` attribute", code = E0802)] |
There was a problem hiding this comment.
suggestion: I would've gone with derive(CoerceShared(Target)).
There was a problem hiding this comment.
those are hard to support in the current derive infra.
There was a problem hiding this comment.
Should def be an open question on the tracking issue, as #[coerce_shared(Thing<'a, T>)] looks a bit unclear to me, maybe #[coerce_shared_target = Thing<'a, T>] if our attribute syntax permits that?
There was a problem hiding this comment.
Ok, I checked the available attribute forms. The current grammar does not accept '#[coerce_shared_target = Thing<'a, T>], since the right-hand side of an = attribute is parsed as an expression and stored as AttrArgs::Eq { expr: Box<Expr>, .. }, while Thing<'a, T> is type syntax and fails expression parsing.
Also, since the current derive pipeline diagnoses arguments on derive paths and resolves only the trait path, we currently reject derive(CoerceShared(Target))
A structured helper such as #[coerce_shared(target = Thing<'a, T>)] is technically feasible because delimited attributes retain their token stream, allowing the builtin derive to parse target = followed by parse_ty().
|
I don't have enough context to review this, so I will leave this to the compiler champion. r? @oli-obk |
|
Hi @oli-obk, no rush, but I wanted to check that this PR is still on your radar. I know about the usual two-week review window; I’m just making sure it did not get missed after the latest updates. Thanks! |
|
Oli seems to be very busy ATM so thus: Cc. @aapoalas |
|
@dingxiangfei2009 It's been a while, just a reminder to review. No pressure thought! |
There was a problem hiding this comment.
Good thing I didn't unsubscribe, even if it was reassigned from me. I was a bit busy catching up in May as I was not doing much Rust stuff before it.
Generally lgtm, slightly worried about spans without expansion info causing funny diagnostics, but we'll add that when necessary (or testable)
| } | ||
|
|
||
| #[derive(Diagnostic)] | ||
| #[diag("`derive(CoerceShared)` requires exactly one `#[coerce_shared(Target)]` attribute", code = E0802)] |
There was a problem hiding this comment.
those are hard to support in the current derive infra.
| } | ||
|
|
||
| #[derive(Diagnostic)] | ||
| #[diag("`derive(CoerceShared)` requires exactly one `#[coerce_shared(Target)]` attribute", code = E0802)] |
There was a problem hiding this comment.
Should def be an open question on the tracking issue, as #[coerce_shared(Thing<'a, T>)] looks a bit unclear to me, maybe #[coerce_shared_target = Thing<'a, T>] if our attribute syntax permits that?
This comment has been minimized.
This comment has been minimized.
c6af245 to
8d9db78
Compare
|
This PR was rebased onto a different main 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. |
|
@rustbot ready |
This adds built-in derive support for the experimental
ReborrowandCoerceSharedmarker traits behind#![feature(reborrow)].The derives generate the same marker impls users would write manually, while preserving the existing coherence and structural validation path for the experimental reborrow traits.
Please note this impl:
CoerceSharedtarget type through the derive helper attribute.@rustbot label F-reborrow
Tracking: #145612