Add anti-fundamental attribute - #160391
Conversation
|
Thanks for the pull request, and welcome! The Rust Project is excited to review your changes, and you should hear from @dingxiangfei2009 (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
This comment has been minimized.
This comment has been minimized.
96b570b to
eec2923
Compare
This comment has been minimized.
This comment has been minimized.
eec2923 to
8bc0866
Compare
This comment has been minimized.
This comment has been minimized.
This adds a new compiler attribute that prevents non-local fundamental types from receiving implementations of marked traits. This allows addressing soundness problems with traits like DerefMut on Pin and similar wrapper types. The attribute is checked during the orphan check in coherence.
Mark Deref, DerefMut, DispatchFromDyn, CoerceUnsized, and Receiver with #[rustc_anti_fundamental] to prevent downstream crates from implementing these traits on #[fundamental] types like Box and Pin.
8bc0866 to
1663605
Compare
|
@rustbot label +llm-assisted |
|
@bors try |
This comment has been minimized.
This comment has been minimized.
Add anti-fundamental attribute
There was a problem hiding this comment.
I'd update the wording here to talk about anti-fundamental instead of saying "special hack".
There was a problem hiding this comment.
Good catch, thanks! Adjusted the wording and changed the link to point to this PR instead
|
@craterbot check |
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
We marked DerefMut anti-fundamental, so the PinHelper indirection is no longer needed to prevent downstream users from implementing DerefMut on Pin.
1663605 to
b6158f7
Compare
| /// it wraps a local type as in (2)). | ||
| /// - e.g., `Box<LocalType>` or `&Pin<LocalType>` is rejected if the trait | ||
| /// is `#[rustc_anti_fundamental]`. | ||
| /// - This lets the standard library reserve control over traits like `Deref` |
There was a problem hiding this comment.
I feel that we don't need bullet points to structure the motivation, it would be more readable if this is just a prose.
| loop { | ||
| ty = infcx.shallow_resolve(ty); | ||
| let norm_ty = match lazily_normalize_ty(ty)? { | ||
| norm if norm.is_ty_var() => ty, |
There was a problem hiding this comment.
Huh, if norm is a type var, ty can't be normalized to the slightest bit I suppose. So it reads a bit off to me if we call it norm_ty... Wdyt?
| /// Checks the head of the Self type for a non-local fundamental type. | ||
| /// | ||
| /// If the head is a reference (`&` / `&mut`), we unwrap it and inspect the pointee type. | ||
| /// This ensures that wrapping a fundamental type in a reference (such as `&Pin<LocalType>`) |
There was a problem hiding this comment.
| /// This ensures that wrapping a fundamental type in a reference (such as `&Pin<LocalType>`) | |
| /// This ensures that wrapping a fundamental type in a reference, say a `&Pin<LocalType>`, |
|
@craterbot cancel See #162233 |
|
🗑️ Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
This PR introduces the internal
#[rustc_anti_fundamental]trait attribute to prevent downstream crates from implementing traits on non-local#[fundamental]types (such asPinandBox).We extend the orphan rule to reject implementations for anti-fundamental traits if the head of its
Selftype is a non-local fundamental type.We mark
Deref,DerefMut,Receiver,CoerceUnsized, andDispatchFromDynwith#[rustc_anti_fundamental], which cleanly resolves orphan rule conflicts and removes the internalPinDerefMutHelperworkaround trait incore::pin.See also #85099
Fixes #148727, #147998 and #148899
Removes hack needed by #145608
AI disclosure
This PR was created with AI assistance. I don't think I have permission to add the
llm-assistedlabel.It touches soundness, but I created it before the new LLM usage policy came into effect, and I only noticed once I fixed some issues I came across and rebased and wanted to update the PR.
Xiang has agreed to review this PR.
We sketched out the implementation logic by hand together and I then gave instructions to the LLM to implement it. I then iterated it and expanded testing through discussing edge cases and AI review until we were fairly confident in the current implementation.
r? @dingxiangfei2009