-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
RFC: Field projection #3318
base: master
Are you sure you want to change the base?
RFC: Field projection #3318
Conversation
- Removed `Ref[Mut]` - clarified Pin and pin example - added attribute implementation details - projecting arc section - specified projectable pointers - elaborated on design rationale - updated unresolved questions
also made small corrections
In ece8b0d I fixed the soundness hole found by pitaj in the internals thread. Sadly this is to the detriment of ergonomics, but I think keeping existing behavior the same is very important. |
That might be nice to have for ergonomics but there is already a, mind you stable, way to do this through |
addr_of! only works on raw pointers. It does nothing for Pin, MaybeUninit, Cell, UnsafeCell, ... |
I feel like it's missing a drawback: the implicit conversions. While supporting this for Rust generally prefers explicit over implicit, and this is adding implicit conversions on the very common dot operator (unlike, say, I'd also like to see a bit on "how do we teach this?" to users. How come I can access the inner field of some wrapper types but not others? The field was an |
To me there is no hidden conversion. If I have match struct {
Some(struct) => Some(&mut struct.field),
None => None,
} To me this conversion is fully expressed by the member access syntax (I think the argument for different syntax can be made). It also is functionally very similar to (I do not know if it is clear from the RFC, but |
True, yet can't you call |
yes you can do struct Foo {
bar: Bar,
}
struct Bar {
a: usize
}
let data = UnsafeCell::new(...);
let a: &UnsafeCell<usize> = unsafe { &*addr_of_mut!((*data.get()).bar.a).cast::<UnsafeCell<usize>>() } But
|
The trait suggested here isn't actually implementable: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=23e3e48fbc2816cf3f6460bceeba6dd2 Probably it should have an extra lifetime param: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=23e3e48fbc2816cf3f6460bceeba6dd2 pub trait FieldProjecting<'a, T> where Self: 'a {
type Inner<U: 'a>;
unsafe fn project<U>(self, proj: impl FnOnce(*mut T) -> *mut U) -> Self::Inner<U>;
} |
The trait must also be |
As later pointed I missed the detail where this only applies to pointers, my bad! (I read about it but my brain chose to ignore that for this example.) |
There are no conversions from
Note that all of these types have the same layout as |
My point there was that: (1) with the example of UnsafeCell, you might as well use unsafe for the rest too and (2) while it's very verbose in this plain form, it's not quite that hard to abstract it away in a trait from a crate. There are already crates providing lens as a functionality, this is no different. This would be a great crate, but I personally don't think it has a place in the standard library, or at least not like this. |
To me, field projection feels different compared to lenses. It also is something native to Rust. We have these different wrapper types ( This is something general that many Rust programs will need to do, so in my opinion it is something the language should provide. Most of the impact will be by allowing this for |
struct Foo {
a: i32,
b: dyn Send,
} |
I think for the beginning we are not going to support those. But conversion can only happen in one way, right? (so from fat => thin) So it might be alright to allow that at some point (it would be weird to not support pin projection for |
|
There is also the big question of how we want pin projection to look like. Here is a zulip post about that |
I do like the idea of field projection in general, but I am not at all convinced that making You reference RFC 2708, and I cannot help but feel that re-ifying field access is a direction that should be explored more: it is useful on its own, and it seems like it could power field projection. From a pure syntax perspective, it would certain be more verbose, especially in the absence of aliasing, yet:
The latter is indubitably more verbose, but at the same time it's within a factor of x2 and has zero magic. Is Field Projection frequent enough to warrant special treatment? I believe not. It is used, certainly, but Contrast with It's clear why Should Field Projection be implementable safely? It's a laudable goal, yet at the same time all the types you wish to implement for already have an So I'll propose an alternative, lightweight design:
That's it. No complex attributes for the compiler to implement, no trait. Any additional implementation cost, any additional language or library change, should be justified as providing sufficient added value above what this lightweight alternative offers. |
|
||
Importantly, the `Field` trait should only be implemented on fields of non-`packed` types, since otherwise the above code would not be sound. | ||
|
||
Another restriction is that unsized fields have dynamic offsets and thus cannot be statically known. So these fields types do not implement the `Field` trait, but the compiler generated type for the field still exists. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would not apply to slices and other Aligned
types (#3319), though, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not so sure how knowing alignment will change this. For example if I have a &dyn Any
how do I even know that there is a field of some type present?
I am coming here from a separate discussion on a potential It would be great to have some kind of field projection on a volatile type that is correct and ergonomic. An ergonomic, completely separate approach from this RFC to this would be allowing the following: let foo: &mut VolatileCell<Foo> = magic!();
let bar = &mut foo.bar; // &mut VolatileCell<Bar> This should verify borrows as usual: mutable references must be exclusive, and all references must satisfy any lifetime requirements. Specifically, projecting to different fields should work and non-mutable borrowing are allowed to overlap. Going by the Would this reference magic be part of the "general custom field projection" that is referenced in the motivation, or would this be similar to "the very first design [that] only supported transparent wrapper types" design referenced in the RFC History? |
Benno has been involved with the Rust for Linux project for the better part of a year now. He has been working on solving the safe pinned initialization problem [1], which resulted in the pin-init API patch series [2] that allows to reduce the need for `unsafe` code in the kernel. He is also working on the field projection RFC for Rust [3] to bring pin-init as a language feature. His expertise with the language will be very useful to have around in the future if Rust grows within the kernel, thus add him to the `RUST` entry as reviewer. Link: https://rust-for-linux.com/the-safe-pinned-initialization-problem [1] Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [2] Link: rust-lang/rfcs#3318 [3] Cc: Benno Lossin <[email protected]> Reviewed-by: Alex Gaynor <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]>
Benno has been involved with the Rust for Linux project for the better part of a year now. He has been working on solving the safe pinned initialization problem [1], which resulted in the pin-init API patch series [2] that allows to reduce the need for `unsafe` code in the kernel. He is also working on the field projection RFC for Rust [3] to bring pin-init as a language feature. His expertise with the language will be very useful to have around in the future if Rust grows within the kernel, thus add him to the `RUST` entry as reviewer. Link: https://rust-for-linux.com/the-safe-pinned-initialization-problem [1] Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [2] Link: rust-lang/rfcs#3318 [3] Cc: Benno Lossin <[email protected]> Reviewed-by: Alex Gaynor <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]>
It is not completely separate :)
Note that
For your impl<T> VolatileCell<T> {
pub fn map<F: Field<Base = T>>(&mut self) -> &mut VolatileCell<F::Type> {
todo!()
}
} Then you could do: let foo: &mut VolatileCell<Foo> = magic!();
let bar: &mut VolatileCell<Bar> = foo.map::<field_of!(Foo, bar)>(); An ergonomics problem that this has is that you cannot borrow multiple fields at the same time: let foo: &mut VolatileCell<Foo> = magic!();
let bar: &mut VolatileCell<Bar> = foo.map::<field_of!(Foo, bar)>();
let baz: &mut VolatileCell<Baz> = foo.map::<field_of!(Foo, baz)>();
// cannot use `bar` here. If we would get some better custom references support, this could be fixed orthogonally. |
Benno has been involved with the Rust for Linux project for the better part of a year now. He has been working on solving the safe pinned initialization problem [1], which resulted in the pin-init API patch series [2] that allows to reduce the need for `unsafe` code in the kernel. He is also working on the field projection RFC for Rust [3] to bring pin-init as a language feature. His expertise with the language will be very useful to have around in the future if Rust grows within the kernel, thus add him to the `RUST` entry as reviewer. Link: https://rust-for-linux.com/the-safe-pinned-initialization-problem [1] Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [2] Link: rust-lang/rfcs#3318 [3] Cc: Benno Lossin <[email protected]> Reviewed-by: Alex Gaynor <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]>
Benno has been involved with the Rust for Linux project for the better part of a year now. He has been working on solving the safe pinned initialization problem [1], which resulted in the pin-init API patch series [2] that allows to reduce the need for `unsafe` code in the kernel. He is also working on the field projection RFC for Rust [3] to bring pin-init as a language feature. His expertise with the language will be very useful to have around in the future if Rust grows within the kernel, thus add him to the `RUST` entry as reviewer. Link: https://rust-for-linux.com/the-safe-pinned-initialization-problem [1] Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [2] Link: rust-lang/rfcs#3318 [3] Cc: Benno Lossin <[email protected]> Reviewed-by: Alex Gaynor <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]>
Benno has been involved with the Rust for Linux project for the better part of a year now. He has been working on solving the safe pinned initialization problem [1], which resulted in the pin-init API patch series [2] that allows to reduce the need for `unsafe` code in the kernel. He is also working on the field projection RFC for Rust [3] to bring pin-init as a language feature. His expertise with the language will be very useful to have around in the future if Rust grows within the kernel, thus add him to the `RUST` entry as reviewer. Link: https://rust-for-linux.com/the-safe-pinned-initialization-problem [1] Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [2] Link: rust-lang/rfcs#3318 [3] Cc: Benno Lossin <[email protected]> Reviewed-by: Alex Gaynor <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]>
Benno has been involved with the Rust for Linux project for the better part of a year now. He has been working on solving the safe pinned initialization problem [1], which resulted in the pin-init API patch series [2] that allows to reduce the need for `unsafe` code in the kernel. He is also working on the field projection RFC for Rust [3] to bring pin-init as a language feature. His expertise with the language will be very useful to have around in the future if Rust grows within the kernel, thus add him to the `RUST` entry as reviewer. Link: https://rust-for-linux.com/the-safe-pinned-initialization-problem [1] Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [2] Link: rust-lang/rfcs#3318 [3] Cc: Benno Lossin <[email protected]> Reviewed-by: Alex Gaynor <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]>
Benno has been involved with the Rust for Linux project for the better part of a year now. He has been working on solving the safe pinned initialization problem [1], which resulted in the pin-init API patch series [2] that allows to reduce the need for `unsafe` code in the kernel. He is also working on the field projection RFC for Rust [3] to bring pin-init as a language feature. His expertise with the language will be very useful to have around in the future if Rust grows within the kernel, thus add him to the `RUST` entry as reviewer. Link: https://rust-for-linux.com/the-safe-pinned-initialization-problem [1] Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [2] Link: rust-lang/rfcs#3318 [3] Cc: Benno Lossin <[email protected]> Reviewed-by: Alex Gaynor <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]>
I think we may not need This came up during discussion of (Needless to say, this would definitely not block an initial implementation using |
I notice that the current proposal doesn't really need special compiler support (playground). The easiest way to help move this RFC forward would be to define a crate with some version of |
That implementation is not sound, I can decide to call The real problem with the implementation is that it is not actually useful in the sense that it does not allow projections where we need to store some information on the field. For example in the case of So yes you can implement it like this, but I would say that the implementation that I linked in the crates section by @nbdd0121 is much more mature. |
Ah, I did not realize this was a need; is it load-bearing? As in, is it required for meaningful experimentation? AFAIU for pin projections this macro+trait would get us a safe The current proposal is so clean and minimal, it gave me hope that a crate that implements it even partially could solve many problems long before we get compiler support. |
You can experiment with that API, but it does not offer you the same amount of freedom in what you can achieve with it.
I agree, this simpler version already seems useful for the basic types that have constant projections (so the projection does not depend on the field type). Rust for linux has some more applications of non-constant projections, for example we might want to use it to make the RCU API more ergonomic. And I can imagine this being useful for memory mapped registers too. |
I have taken some time to dig through the compiler source code and I have actually managed to implement the |
Excellent! You could submit this as a PR. Adding a feature gate requires a lot less procedure than getting an RFC accepted. They'll tell you if this is acceptable (you'll definitely have to add tests!) and we'll all get to experiment with it |
To expand on this: I see this proposal as a first step toward a more general compile-time reflection mechanism, in the style of "A Mirror for Rust". That more general feature will require a mechanism for performing an operation with all a struct's fields. If the way to refer to "a field of type |
This isn't anything like a concrete proposal, but I wanted to note the opposite direction of what @Jules-Bertholet just mentioned: arguably the only thing that projections need is "this value has some data of type |
I do not have the bandwidth of changing this proposal to be part of a bigger system of compile-time reflection. If you (or anyone else) wants to tackle the problem, feel free to reach out to me, I can take a look if what you are proposing will cover the use-cases of this RFC. I also think that the general compile-time reflection feature is going to take a long time to develop. RFL needs these projection as soon as we can get them. I do not want that trying to find a "perfect" general compile-time reflection implementation prevents us from having field projections in a couple of months (of course I am talking about it being an unstable feature). |
As I said before, our projections need more than just the offset and the type of the field. We also need to store information on each field, be it for pin projections, RCU projections or memory mapped registers. For enums I do not see any issue with the current proposal. I haven't implemented them yet, but they could definitely work. Unsized types where we know the layout (i.e. structs where the last field is unsized) should also be no problem. |
I have no objection to having an imperfect API as an experimental feature gate, especially if it gives feedback that improves the final result. However, the current lang team policy seems to be that RFCs should present a polished and complete design. |
Sure that is fine. When I opened this RFC, I thought that making an RFC is the only way to introduce these changes (note that at the beginning the RFC had many more things compared to now). Also at the time I had not yet implemented anything and tinkering on the compiler seemed too daunting. I have no idea what you are trying to convey by this link. Can you please point me to a specific comment? |
This comment was marked as resolved.
This comment was marked as resolved.
Could I suggest updating this so the second argument is parsed identically to that of Enum support would be useful in e.g. |
I am picking up this work again. See this zulip topic for more info. I am starting out by first collecting the use cases, as I feel that last time, the motivation for this feature was not sufficient enough. Here is the document presenting all current use cases for field projections (also linked from the zulip topic, please leave your feedback there). |
Rendered
Also see the pre-RFC discussion.
Unresolved questions