-
-
Notifications
You must be signed in to change notification settings - Fork 14.9k
improve documentation of field representing types #154927
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,23 +1,60 @@ | ||||||
| //! Field Reflection | ||||||
| //! Field Reflection for field projections. | ||||||
| //! | ||||||
| //! # Overview | ||||||
| //! | ||||||
| //! This module is part of the field projection compiler experiment; see the [tracking | ||||||
| //! issue](https://github.com/rust-lang/rust/issues/145383) for more information. | ||||||
|
Comment on lines
+5
to
+6
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also had another question: Have we accidentally added anything that is observable on stable? We still want to be able to change all aspects of FRTs (including removing them wholesale). To the best of my ability, we have not, but I'd like to get an experts' seal of approval :)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see anything here that looks exposed to stable. That question is often incorrectly answered, but I think you're operating in relatively safe territory right now. I'm much more worried about the other PR on projection traits, especially once/if they become something driving compiler behavior (rather than just being traits for library code). |
||||||
| //! | ||||||
| //! The current approach of field projections makes use of *field representing types*. These are | ||||||
| //! compiler-generated types that identify a field of a struct, union, enum or tuple. Users can | ||||||
| //! implement traits for them to store additional information about the *field*. | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I typically don't think of traits as storing information. Do you mean in the sense that the information is accessible via I guess in principle we could expose reflection-like information through the type (e.g., attributes on the field), but I presume that is not currently part of this proposal, right? And even if it was, not sure why we'd do that via traits vs. just having properties (fields or associated consts or methods) on the field type itself.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes exactly, you use this information via the associated constants/types. The reflection features of FRTs are needed because of |
||||||
| //! | ||||||
| //! # API Surface | ||||||
| //! | ||||||
| //! At its core, this module provides the [`field_of!`] macro, which takes a type and the name of a | ||||||
| //! field of that type and returns the field representing type (FRT) of that field. | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we support the same nested syntax that offset_of! does? I see that the macro definitions supports it, but the docs sort of imply it doesn't (https://doc.rust-lang.org/nightly/std/field/macro.field_of.html). Is it accurate to say that in some sense, offset_of! should eventually be user-definable as field_of!(...).offset() or similar?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, we do not support nested fields. To support enums, we allow
The
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Hm, this doesn't quite make sense to me. Isn't the expansion of a nested expression there 'just' querying the type of the field and then calling field_of! on that type? I am assuming that for the purposes of orphan rules the FRT is owned by the parent ADT crate and referenced conceptually from there with regards to the trait system, in all cases. Is that an incorrect assumption? That does imply that such types and impls on them are conceptually generated for all crates, even those that don't opt-in to the feature gate... but that seems fine?
It feels a little odd to me to have two separate categories here. I would definitely expect to be able to get an FRT for
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, actually, my mental model was wrong with expansions to a type name. I'm wondering if there's a reason we want to define That does mean there's not one FRT type but I'm not sure how much it buys us to have the name if you're always supposed to go via |
||||||
| //! | ||||||
| //! The FRTs of certain well-behaved (at the moment `repr(packed)` and `?Sized` are not supported) | ||||||
| //! fields implement the [`Field`] trait, which exposes information such as the offset, base type, | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Right?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No,
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you trying to say that Field is not implemented if the field is repr(packed) or if the parent type is repr(packed)? That's the confusion I was trying to clear up here. (I think repr(packed) influencing this is a bit confusing. That shouldn't prevent offsets being statically known, right?) |
||||||
| //! and type of the field. | ||||||
| //! | ||||||
| //! # Implementation Details | ||||||
| //! | ||||||
| //! FRTs are a mix of compiler and library implemented. The perma-unstable `FieldRepresentingType` | ||||||
| //! struct is used to ultimately represent a field by the variant and field index; while the | ||||||
| //! compiler performs existence and coherence checks before allowing access to it. | ||||||
| //! | ||||||
| //! FRTs are intended to contain only *static* information about a field and as such they are | ||||||
| //! inhabited ZSTs. For this reason, they also implement [`Copy`], [`Send`], and [`Sync`] regardless | ||||||
| //! of the properties of the base type. | ||||||
|
|
||||||
| use crate::marker::PhantomData; | ||||||
|
|
||||||
| /// Field Representing Type | ||||||
| /// A type representing a field of a struct, union, enum or tuple. | ||||||
| /// | ||||||
| /// This perma-unstable type is part of the implementation details of the [`field_of!`] macro. This | ||||||
| /// type is only instantiated by the compiler when the type `T` has a variant with the index | ||||||
| /// `VARIANT` and that variant has a field with index `FIELD`. This type will then represent that | ||||||
| /// specific field. | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it matter when the compiler chooses to instantiate this? Can't user code choose to instantiate it at different types/constants, e.g., with Will that be a post-mono error if Vec turns out to not have that field/variant?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The way it is currently implemented, yes, this matters a lot. If you write I didn't see the need to make this more accessible, since this developed much later during implementation to avoid having to add another |
||||||
| /// | ||||||
| /// Since this type is marked as `#[fundamental]`, downstream crates may implement types as long as | ||||||
| /// `T` is local to that crate. | ||||||
| #[unstable(feature = "field_representing_type_raw", issue = "none")] | ||||||
| #[lang = "field_representing_type"] | ||||||
| #[expect(missing_debug_implementations)] | ||||||
| #[fundamental] | ||||||
| #[doc(hidden)] | ||||||
| pub struct FieldRepresentingType<T: ?Sized, const VARIANT: u32, const FIELD: u32> { | ||||||
| _phantom: PhantomData<T>, | ||||||
| } | ||||||
|
|
||||||
| // SAFETY: `FieldRepresentingType` doesn't contain any `T` | ||||||
| // SAFETY: `FieldRepresentingType` doesn't contain any `T`. | ||||||
|
BennoLossin marked this conversation as resolved.
|
||||||
| unsafe impl<T: ?Sized, const VARIANT: u32, const FIELD: u32> Send | ||||||
| for FieldRepresentingType<T, VARIANT, FIELD> | ||||||
| { | ||||||
| } | ||||||
|
|
||||||
| // SAFETY: `FieldRepresentingType` doesn't contain any `T` | ||||||
| // SAFETY: `FieldRepresentingType` doesn't contain any `T`. | ||||||
| unsafe impl<T: ?Sized, const VARIANT: u32, const FIELD: u32> Sync | ||||||
| for FieldRepresentingType<T, VARIANT, FIELD> | ||||||
| { | ||||||
|
|
@@ -36,7 +73,7 @@ impl<T: ?Sized, const VARIANT: u32, const FIELD: u32> Clone | |||||
| } | ||||||
| } | ||||||
|
BennoLossin marked this conversation as resolved.
|
||||||
|
|
||||||
| /// Expands to the field representing type of the given field. | ||||||
| /// The field representing type of the given field. | ||||||
| /// | ||||||
| /// The container type may be a tuple, `struct`, `union` or `enum`. In the case of an enum, the | ||||||
| /// variant must also be specified. Only a single field is supported. | ||||||
|
|
||||||
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.
Some more general information on FRTs (let me know if it makes sense to put this in the docs somewhere):
FRTs have two main properties that we need for field projections:
The ultimate goal of field projections is to have an operator that allows one to go from
Ptr<Struct>toPtr<Field>. Since all customizable operators in Rust are backed by traits, we thought about how we could translate the syntactic information. We ended up by deducing that "generics are the natural way to write generic code in Rust, so let's just make fields be a special kind of generic". This explains why we need & how we arrived at the first property.The second property is required to support
Pin; we need some way to record the structural pinning information of fields that can be accessed from generic code. Our initial plan was to have aPinnableFieldtrait that not only exposes aboolon the structuralness of its pinning, but also a generic associated type that is either the identity function, or thePintype constructor (sotype Bikeshed<P> = P;ortype Bikeshed<P> = Pin<P>;). You can already implement this now as anunsafeextension trait ofFieldthat is implemented through a safe derive macro. Since our original plan, we have put some more thought into this and theMovetrait got some reviving traction. At the moment, we think theMovetrait is a 10x better solution, so we have not really implemented or explored thePinnableFieldapproach further.The proposal has changed a fair bit; FRTs were designed and implemented in September last year and then reimplemented in January & then again in February. Their internal implementation changed a lot.
We're not sure if we need FRTs in the end. At the moment I'm leaning towards that we do not need them. However, they are still extremely useful for other purposes, so we might want to separate them into their own feature. For example, RfL wants them to better handle intrusive data structures: a struct can contain multiple instances of
ListLinks, which allows it to be in multiple different lists at the same time. At the moment we use a const genericID: u64to record whichListLinksfield a list uses; with FRTs, we could change that to aF: Fieldand then users would writeList<field_of!(MyFieldType, foo_list)>, which would be much more meaningful and less error-prone.For this reason, we might want to split it off as a standalone feature. It also explains the purpose of FRTs as being a tool of reflection a bit more, we want to be able to talk about properties of fields in a generic context. More on that in the thread about usage patterns in std. I'll give more information about the feature that supersedes FRTs in the
offset_of!thread.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 is useful background, thank you.
My mental model is that we are very much not introducing a new kind of generic here (that would be impossible in library code), just a trait and some compiler-generated impls of it. Am I missing something, or does that sound accurate?
I think that all sounds pretty reasonable, though I think Pin is not the only structural thing... for example, the discussion we had around Option/Result mapping feels like it's ultimately the same kind of operation?
I think this makes sense, but it (at least right now) also feels like it may not really belong in std -- or at least not be on a clear path to stabilization. Is it accurate that the primary benefit of this being in std is that we can get away without a derive-macro and needing to derive it on every type to synthesize the types/impls for 'field types'? That is definitely a large benefit for the users of the feature but it does seem like it fits under the reflection umbrella than projections.