-
Notifications
You must be signed in to change notification settings - Fork 13.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tracking issue for RFC 3519: arbitrary_self_types
#44874
Comments
Why would you need this?
I'd rather define the trait different. Maybe like this:
In this case, Rc would be a trait type. If every generic type implemented a specific trait (this could be implemented automatically for generic types) this seems more understandable to me. |
This could only be allowed for For inherent methods, I can't |
This is still pending lang team decisions (I hope there will be at least 1 RFC) but I think it will only be allowed for trait method impls. |
You can't implement anything for |
So changes needed:
|
I’ll look into this. |
Note that this is only supported to work with trait methods (and trait impl methods), aka trait Foo {
fn foo(self: Rc<Self>);
}
impl Foo for () {
fn foo(self: Rc<Self>) {}
} and is NOT supposed to work for inherent impl methods: struct Foo;
impl Foo {
fn foo(self: Rc<Self>) {}
} |
I got caught in some more Stylo work that's gonna take a while, so if someone else wants to work on this in the meantime feel free. |
Is this supposed to allow any type as long as it involves trait MyStuff {
fn a(self: Option<Self>);
fn b(self: Result<Self, Self>);
fn c(self: (Self, Self, Self));
fn d(self: Box<Box<Self>>);
}
impl MyStuff for i32 {
...
}
Some(1).a(); // ok?
Ok(2).b(); // ok?
(3, 4, 5).c(); // ok?
(box box 6).d(); // ok? |
…ents, r=nikomatsakis Update comments referring to old check_method_self_type I was browsing the code base, trying to figure out how rust-lang#44874 could be implemented, and noticed some comments that were out of date and a bit misleading (`check_method_self_type` has since been renamed to `check_method_receiver`). Thought it would be an easy first contribution to Rust!
I've started working on this issue. You can see my progress on this branch |
@arielb1 You seem adamant that this should only be allowed for traits and not structs. Aside from method shadowing, are there other concerns? |
inherent impl methods are loaded based on the type. You shouldn't be able to add a method to |
That's it, if you write something like trait Foo {
fn bar(self: Rc<Self>);
} Then it can only be used if the trait If you write an inherent impl, then it can be called without having the trait in-scope, which means we have to be more careful to not allow these sorts of things. |
@arielb1 Can you give an example of what we want to avoid? I'm afraid I don't really see what the issue is. A method you define to take |
I've been trying to figure out how we can support dynamic dispatch with arbitrary self types. Basically we need a way to take a (1) is pretty straightforward: call The tough question is, how do we get the type @arielb1 @nikomatsakis any thoughts? |
Wait, why do you not want it work for inherent impl methods? Because of scoping? I'm confused. =) |
I do want to support that, but I expected it to be out of scope for this first cut. That is, I expected that if a trait uses anything other than |
I know, but I couldn't help looking into it, it's all very interesting to me :) |
We need some sort of "orphan rule" to at least prevent people from doing things like this: struct Foo;
impl Foo {
fn frobnicate<T>(self: Vec<T>, x: Self) { /* ... */ }
} Because then every crate in the world can call Maybe the best way to solve this would be to require I think that if we have the deref-back requirement, there's no problem with allowing inherent methods - we just need to change inherent method search a bit to also look at defids of derefs. So that's probably a better idea than restricting to trait methods only. Note that the struct Foo;
impl Tr for Foo {
fn frobnicate<A: Allocator+?Sized>(self: RcWithAllocator<Self, A>) { /* ... */ }
} Where an |
Are saying is that there would be a "conflicting symbols for architechture x86_64..." linker error?
I'm confused, are you still talking about |
The deref-back requirement is supposed to be for everything, not only object-safety. It prevents the problem when one person does struct MyType;
impl MyType {
fn foo<T>(self: Vec<(MyType, T)>) { /* ... */ }
} While another person does struct OurType;
impl OurType {
fn foo<T>(self: Vec<(T, OurType)>) {/* ... */ }
} And now you have a conflict on |
I'm working on a stabilization report here - hopefully will post it tomorrow - meanwhile a draft PR for stabilization is here. |
@adetaylor we have been discussing the idea of adopting a new stabilization template which is a series of questions: https://hackmd.io/@nikomatsakis/Sy7wJC9Ikx I haven't gotten around to opening a PR for this, but I request that you give it a try and tell me how it works for you. It is meant to help ensure we catch common mistakes. |
OK will do - in that case I withdraw my earlier suggestion that I'd post the stabilization report today - it will take a couple more days to get all that stuff together (I agree it makes sense though) |
Thanks Adrian.
…On Thu, Jan 23, 2025, at 11:40 AM, Adrian Taylor wrote:
OK will do - in that case I withdraw my earlier suggestion that I'd post the stabilization report today - it will take a couple more days to get all that stuff together (I agree it makes sense though)
—
Reply to this email directly, view it on GitHub <#44874 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABF4ZT2THCGPDQ7MGC7J6T2MELQFAVCNFSM4D4TO7FKU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TENRRGAZTKOBXHA2A>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
This comment formerly contained a stabilization report, which on request I've moved to #135881. |
@adetaylor: It would be cool if you broke this out into another issue or ideally put the stabilization onto the stabilization PR itself. Burying into the comment history of a tracking issue seems hard to manage with comments, feedback, concerns, and generally other things here. |
OK - stabilization report moved to #135881. |
This is a pre-existing risk of semver breakage for types implementing Deref<Target=T>. It's made more apparent by the arbitrary self types v2 change, and will now generate errors under some circumstances. This PR documents the existing risk and the new ways it can be triggered. If it's seen as desirable to document the existing risk before Arbitrary Self Types v2 is stabilized, we can split this commit up into two. Part of rust-lang/rust#44874 Stabilization PR rust-lang/rust#135881
This is a pre-existing risk of semver breakage for types implementing Deref<Target=T>. It's made more apparent by the arbitrary self types v2 change, and will now generate errors under some circumstances. This PR documents the existing risk and the new ways it can be triggered. If it's seen as desirable to document the existing risk before Arbitrary Self Types v2 is stabilized, we can split this commit up into two. Part of rust-lang/rust#44874 Stabilization PR rust-lang/rust#135881
This is a pre-existing risk of semver breakage for types implementing Deref<Target=T>. It's made more apparent by the arbitrary self types v2 change, and will now generate errors under some circumstances. This PR documents the existing risk and the new ways it can be triggered. If it's seen as desirable to document the existing risk before Arbitrary Self Types v2 is stabilized, we can split this commit up into two. Part of rust-lang/rust#44874 Stabilization PR rust-lang/rust#135881
This is a pre-existing risk of semver breakage for types implementing Deref<Target=T>. It's made more apparent by the arbitrary self types v2 change, and will now generate errors under some circumstances. This PR documents the existing risk and the new ways it can be triggered. If it's seen as desirable to document the existing risk before Arbitrary Self Types v2 is stabilized, we can split this commit up into two. One of the two code examples here is marked "skip" because it depends on the nightly feature which is under discussion for stabilization. Part of rust-lang/rust#44874 Stabilization PR rust-lang/rust#135881
Bug rust-lang#57276 (and several duplicates) is an ICE which occurs when a generic type is used as a receiver which implements both Receiver and DispatchFromDyn. This change proposes to detect this error condition and turn it into a regular error rather than an ICE. Future changes could liberalise things here. As this same code path currently produces an ICE, this seems to be strictly better, and it seems defensible to inform the user that their excessively generic type is not dyn-safe. This is somewhat related to the stabilization of arbitrary self types in PR rust-lang#135881, tracked in rust-lang#44874.
This is a pre-existing risk of semver breakage for types implementing Deref<Target=T>. It's made more apparent by the arbitrary self types v2 change, and will now generate errors under some circumstances. This PR documents the existing risk and the new ways it can be triggered. If it's seen as desirable to document the existing risk before Arbitrary Self Types v2 is stabilized, we can split this commit up into two. One of the two code examples here is marked "skip" because it depends on the nightly feature which is under discussion for stabilization. Part of rust-lang/rust#44874 Stabilization PR rust-lang/rust#135881
For future traceability: could you link to the various implementations PRs in the top post's "Implementation history" section please? |
…i-obk Arbitrary self types v2: recursion test Add a test for infinite recursion of an arbitrary self type. These diagnostics aren't perfect (especially the repetition of the statement that there's too much recursion) but for now at least let's add a test to confirm that such diagnostics are emitted. As suggested by `@oli-obk` Relates to rust-lang#44874 r? `@wesleywiser`
…i-obk Arbitrary self types v2: recursion test Add a test for infinite recursion of an arbitrary self type. These diagnostics aren't perfect (especially the repetition of the statement that there's too much recursion) but for now at least let's add a test to confirm that such diagnostics are emitted. As suggested by ``@oli-obk`` Relates to rust-lang#44874 r? ``@wesleywiser``
…i-obk Arbitrary self types v2: recursion test Add a test for infinite recursion of an arbitrary self type. These diagnostics aren't perfect (especially the repetition of the statement that there's too much recursion) but for now at least let's add a test to confirm that such diagnostics are emitted. As suggested by ```@oli-obk``` Relates to rust-lang#44874 r? ```@wesleywiser```
Rollup merge of rust-lang#136567 - adetaylor:test-for-recursion, r=oli-obk Arbitrary self types v2: recursion test Add a test for infinite recursion of an arbitrary self type. These diagnostics aren't perfect (especially the repetition of the statement that there's too much recursion) but for now at least let's add a test to confirm that such diagnostics are emitted. As suggested by ```@oli-obk``` Relates to rust-lang#44874 r? ```@wesleywiser```
This comment has been minimized.
This comment has been minimized.
…o_to_object-hard-error, r=<try> Make `ptr_cast_add_auto_to_object` lint into hard error In Rust 1.81, we added a FCW lint (including linting in dependencies) against pointer casts that add an auto trait to dyn bounds. This was part of work making casts of pointers involving trait objects stricter, and was part of the work needed to restabilize trait upcasting. We considered just making this a hard error, but opted against it at that time due to breakage found by crater. This breakage was mostly due to the `anymap` crate which has been a persistent problem for us. It's now a year later, and the fact that this is not yet a hard error is giving us pause about stabilizing arbitrary self types and `derive(CoercePointee)`. So let's see about making a hard error of this. r? ghost cc `@adetaylor` `@Darksonn` `@BoxyUwU` `@RalfJung` `@compiler-errors` `@oli-obk` `@WaffleLapkin` Related: - rust-lang#135881 - rust-lang#136702 Tracking: - rust-lang#127323 - rust-lang#44874 - rust-lang#123430
BugLink: https://bugs.launchpad.net/bugs/2096827 commit c95bbb59a9b22f9b838b15d28319185c1c884329 upstream. The term "receiver" means that a type can be used as the type of `self`, and thus enables method call syntax `foo.bar()` instead of `Foo::bar(foo)`. Stable Rust as of today (1.81) enables a limited selection of types (primitives and types in std, e.g. `Box` and `Arc`) to be used as receivers, while custom types cannot. We want the kernel `Arc` type to have the same functionality as the Rust std `Arc`, so we use the `Receiver` trait (gated behind `receiver_trait` unstable feature) to gain the functionality. The `arbitrary_self_types` RFC [1] (tracking issue [2]) is accepted and it will allow all types that implement a new `Receiver` trait (different from today's unstable trait) to be used as receivers. This trait will be automatically implemented for all `Deref` types, which include our `Arc` type, so we no longer have to opt-in to be used as receiver. To prepare us for the change, remove the `Receiver` implementation and the associated feature. To still allow `Arc` and others to be used as method receivers, turn on `arbitrary_self_types` feature instead. This feature gate is introduced in 1.23.0. It used to enable both `Deref` types and raw pointer types to be used as receivers, but the latter is now split into a different feature gate in Rust 1.83 nightly. We do not need receivers on raw pointers so this change would not affect us and usage of `arbitrary_self_types` feature would work for all Rust versions that we support (>=1.78). Cc: Adrian Taylor <ade@hohum.me.uk> Link: rust-lang/rfcs#3519 [1] Link: rust-lang/rust#44874 [2] Signed-off-by: Gary Guo <gary@garyguo.net> Reviewed-by: Benno Lossin <benno.lossin@proton.me> Reviewed-by: Alice Ryhl <aliceryhl@google.com> Link: https://lore.kernel.org/r/20240915132734.1653004-1-gary@garyguo.net Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> [koichiroden: dropped changes on rust/kernel/list/arc.rs due to missing commit: 6cd341715558 ("rust: list: add ListArc")] Signed-off-by: Koichiro Den <koichiro.den@canonical.com>
This is the tracking issue for RFC 3519: Arbitrary self types v2.
The feature gate for this issue is
#![feature(arbitrary_self_types)]
.About tracking issues
Tracking issues are used to record the overall progress of implementation. They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions. A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature. Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.
Steps
Current plan is:
arbitrary_self_types_pointers
feature gateReceiver
trait -- currently being run through craterReceiver
trait without it doing anything.Receiver
trait, if thearbitrary_self_types
feature is enabled. The main event.!Sized
case and theNonNull
etc. cases.Unresolved Questions
None.
Notable for Stabilization
Related
Implementation history
(Below follows content that predated the accepted Arbitrary Self Types v2 RFC.)
Object Safety
See #27941 (comment)
Handling of inference variables
Calling a method on
*const _
could now pick impls of the formBecause method dispatch wants to be "limited", this won't really work, and as with the existing situation on
&_
we should be emitting an "the type of this value must be known in this context" error.This feels like fairly standard inference breakage, but we need to check the impact of this before proceeding.
Safe virtual raw pointer methods
e.g. this is UB, so we might want to force the call
<dyn Foo as Foo>::bar
to be unsafe somehow - e.g. by not allowingdyn Foo
to be object safe unlessbar
was anunsafe fn
However, even today you could UB in safe code with
mem::size_of_val(foo)
on the above code, so this might not be actually a problem.More information
There's no reason the
self
syntax has to be restricted to&T
,&mut T
andBox<T>
, we should allow for more types there, e.g.The text was updated successfully, but these errors were encountered: