improve documentation of field representing types

[BennoLossin]

View all comments

I'm improving the documentation of the existing experimental items for field projections before adding any new ones.

I'm not sure what to put on the Field trait or the field_of! macros as docs, maybe that's just my curse of knowledge. I thought about putting some examples there, maybe that's a sensible thing. Would appreciate a suggestion & some guidance :)

r? @Mark-Simulacrum

[BennoLossin]

@rustbot label +F-field_projections

[Mark-Simulacrum]

Thank you, this is a good improvement. Left some comments, not necessarily all blocking, more questions that I'm interested in.

One aspect that this I think doesn't cover is "what do I as a standard library reviewer do when someone uses field_of!() / FieldRrepsentingType<...> / T: Field in a PR?". I see in the tracking issue there's a large list of things we want to touch, how should standard library code interact with that list? Do we have an example PR perhaps?

View changes since this review

[Mark-Simulacrum]

I typically don't think of traits as storing information. Do you mean in the sense that the information is accessible via <FieldTy as Trait>::ASSOC_CONST or similar?

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.

[BennoLossin]

Yes exactly, you use this information via the associated constants/types.

The reflection features of FRTs are needed because of Pin; we need a way to differentiate structurally pinned fields from normal fields. And since we want to represent subplaces with generic types (since we want to expose the type of the subplace), we naturally went into this direction. We could of course create field-generic operations by having fn op<const OFFSET: usize, T>() where T is the type of the field, but that now makes that operation unsafe, since you could pass any offset. So bundling the data in an unsafe trait was the fix for that. It also allows us to add more information to the trait in the future.

[Mark-Simulacrum]

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?

[BennoLossin]

No, we do not support nested fields. To support enums, we allow field_of!(Enum, Variant.field), but you cannot do field_of!(Struct, nested.field). The reason for disallowing them is that if we did allow them, we would have to answer tough questions about orphan rules: is field_of!(LocalType, foreign_type.other_local_type) considered local, what about field_of!((ForeignType, LocalType), 1.field)?

offset_of! will thus not be definable through field_of!; but this is not the only reason, field_of! also does not record the offset for fields that have dynamic offset. So for example field_of!(Struct<dyn Trait>, tail) doesn't implement the Field trait (given that tail: dyn Trait), since dyn Trait has dynamic alignment and thus dynamic offset if not the first field. We still need to figure out more traits in that hierarchy, since the types of the base and the field are clearly something that should be available there.

The Projection (now named Subplace) trait from the design meeting takes on the role of handling dynamic projections. It allows nesting and much more than field_of!/offset_of!, we probably could expose a proj!($ident: $Type, $place_expr) macro that gives you the corresponding impl Subplace. That could then be used to implement offset_of!, since it also supports dynamic cases. However, the proj! macro itself is most likely very difficult to use correctly/safely, since a Subplace may carry special information for a specific instance of that subplace. So for example proj!(x: [u8], x[42]) can only be used for indexing into a slice with a length of at least 43. We're still figuring out where to do the range-check, if we move it into the actual place operation, then proj! will be fine, if we move it into the creation of Subplace, then an impl Subplace will be tied to a specific place and thus its usage is always unsafe.

[Mark-Simulacrum]

The reason for disallowing them is that if we did allow them, we would have to answer tough questions about orphan rules

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? field_of! expands to a type name (roughly path_to_struct::Struct::field, if that actually existed)?

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?

dynamic projections

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 tail: dyn Trait, though it seems quite reasonable for it to not have const OFFSET if that needs runtime information. Maybe what you want is a mirror of the Sized trait hierarchy (effectively an Offset hierarchy?) for the individual fields?

[Mark-Simulacrum]

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 FieldRepresentingType here though? Maybe that could be a compiler-generated type for every field, which settles the orphan rules and whether users can accidentally create one not via the macro (no)?

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 field_of! (I assume that's right?)

[Mark-Simulacrum]

Suggested change

	//! fields implement the [`Field`] trait, which exposes information such as the offset, base type,
	//! types implement the [`Field`] trait, which exposes information such as the offset, base type,

Right?

[BennoLossin]

No, fields is correct, the sentence without the parenthesis says: The FRTs of certain well-behaved fields implement the [`Field`] trait. So the FRT implements the Field trait if the field is well-behaved. Is there a better way to phrase this? Maybe the parenthesis is preventing from easily parsing the sentence, it could be a footnote instead.

[Mark-Simulacrum]

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?)

[Mark-Simulacrum]

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 FieldRepresentingType::<Vec<u32>, 0, 1>?

Will that be a post-mono error if Vec turns out to not have that field/variant?

[BennoLossin]

The way it is currently implemented, yes, this matters a lot. If you write FieldRepresentingType::<Vec<u32>, 0, 42>, then that type exists (so no error), but does not implement the Field trait. If you try to use that type in a context that expects it to implement the Field trait, the compiler will ICE, since it just uses the variant/field index to index into the ADT data :)

I didn't see the need to make this more accessible, since this developed much later during implementation to avoid having to add another TyKind and we intend it to remain perma-unstable. If you think it's a useful thing to have or you want to make it more robust, then I'm not opposed to changing it.

[Mark-Simulacrum]

Can we add some notes here or in the doc comment about what usage patterns are recommended (or not recommended) in std today?

[BennoLossin]

Sure! Let's draft some docs here, because I don't know what exactly you're looking for.

The usage of FRTs in std vs non-std code should generally be the same, there isn't anything special for std here I think. FRTs have two main use-cases derived from their main properties:

1. Writing field-generic code, and
2. Recording information about fields.

For the first use-case, we could imagine adding the following function to std:

impl<'b, T: ?Sized> RefMut<'b, T> {
    pub fn map_field<F>(mut orig: RefMut<'b, T>, field: F) -> RefMut<'b, F::Type>
    where
        F: Field<Base = T>,
    {
        let value: NonNull<T> = orig.value;
        let value: NonNull<F::Type> = unsafe { orig.value.byte_add(F::OFFSET) }.cast();
        RefMut { value, borrow: orig.borrow, marker: PhantomData }
    }
}

I can just make a PR that adds this function, I think it makes for a good example.

The second use-case is a bit harder to find in std, as I don't think there are constructions in std that would like to know more information about a field. @scottmcm made an interesting suggestion in the design meeting for a new pointer type: RawPtr<T, Align = 4>. Projecting this type benefits from knowing the alignment of fields, as we could project RawPtr<(u16, u8), Align = 2> to RawPtr<u8, Align = 2> (replace the tuple with a repr(linear) struct). That would require us adding the following trait:

pub unsafe trait AlignedField: Field {
    const ALIGNMENT: usize;
}

Along with a derive macro that checked if the type had a repr(linear) attribute & then implemented the trait with the appropriate constant by running the layout algorithm.

Admittedly, this example is something that we could (& probably should just) provide via the compiler instead, as that proc-macro would be very complicated.

(there also is the Pin example, which is even more complicated)

[Mark-Simulacrum]

For RefMut::map_field, why would we want that function vs the existing RefMut::map functions? It should be just as fast at runtime presuming some inlining...

I think my expectation is that there is functionality not really expressible today that this is intended to enable (or at least poorly expressible, e.g., needing to thread offsets through).

Some possible questions/answers that I could see being useful:

1. This is all very experimental and we (libs) should hold off on using it inside std or exposing APIs that deal with it. Experimentation can happen out of tree or in UI tests with no real hazard, and design work is not blocked on anything happening in std/core with this.
2. The public shape of the feature is fairly stable, and design work is interested in seeing how libs-api would apply it to new functionality in std. This is a sufficiently meta feature that I'm not sure what that new functionality would be (see my earlier question about what the value of map_field is :), but maybe you have thoughts.
3. The public shape is not stable, but we're relatively optimistic the internal feature will continue to stabilization in some shape. Usage inside the standard library (if we find a good use case) is reasonable.

I think (3), at least right now, for me runs into it being fairly trivial to write specific-to-one-case custom versions of this without too much work. Open to thoughts on (1) and (2).

It's also possible this is very niche and we don't really expect anyone to propose any APIs in std that deal with this, and we can effectively send PRs reviewing anything in core::field to the folks working on this (let's define an ad-hoc group and route them that way in triagebot then).

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[BennoLossin]

Thanks a lot for taking a look and also for suggesting this in the design meeting. This is really helpful and I appreciate it very much :) I hadn't considered several of your questions and I think they are very important ones to answer.

I also have accumulated lot of "curse of knowledge" about FP and don't know what's easy or hard any more. I've found that the simplest way to get around that problem is to just have me explain this to someone that just asks questions. That way I share all I know without having to realize what is difficult/unintuitive. That way we can arrive good documentation. So if you have any other questions, please ask!

I have put all my answers in the threads so it's easier to reply and not lose track of the different lines of questions we have.

Note that I give some general information in a comment on the first line. I created it last so AFAIK, github will show it last in the email and in the interface, so maybe visit that first before viewing the threads :)

View changes since this review

[BennoLossin]

Yes exactly, you use this information via the associated constants/types.

The reflection features of FRTs are needed because of Pin; we need a way to differentiate structurally pinned fields from normal fields. And since we want to represent subplaces with generic types (since we want to expose the type of the subplace), we naturally went into this direction. We could of course create field-generic operations by having fn op<const OFFSET: usize, T>() where T is the type of the field, but that now makes that operation unsafe, since you could pass any offset. So bundling the data in an unsafe trait was the fix for that. It also allows us to add more information to the trait in the future.

[BennoLossin]

No, we do not support nested fields. To support enums, we allow field_of!(Enum, Variant.field), but you cannot do field_of!(Struct, nested.field). The reason for disallowing them is that if we did allow them, we would have to answer tough questions about orphan rules: is field_of!(LocalType, foreign_type.other_local_type) considered local, what about field_of!((ForeignType, LocalType), 1.field)?

offset_of! will thus not be definable through field_of!; but this is not the only reason, field_of! also does not record the offset for fields that have dynamic offset. So for example field_of!(Struct<dyn Trait>, tail) doesn't implement the Field trait (given that tail: dyn Trait), since dyn Trait has dynamic alignment and thus dynamic offset if not the first field. We still need to figure out more traits in that hierarchy, since the types of the base and the field are clearly something that should be available there.

The Projection (now named Subplace) trait from the design meeting takes on the role of handling dynamic projections. It allows nesting and much more than field_of!/offset_of!, we probably could expose a proj!($ident: $Type, $place_expr) macro that gives you the corresponding impl Subplace. That could then be used to implement offset_of!, since it also supports dynamic cases. However, the proj! macro itself is most likely very difficult to use correctly/safely, since a Subplace may carry special information for a specific instance of that subplace. So for example proj!(x: [u8], x[42]) can only be used for indexing into a slice with a length of at least 43. We're still figuring out where to do the range-check, if we move it into the actual place operation, then proj! will be fine, if we move it into the creation of Subplace, then an impl Subplace will be tied to a specific place and thus its usage is always unsafe.

[BennoLossin]

No, fields is correct, the sentence without the parenthesis says: The FRTs of certain well-behaved fields implement the [`Field`] trait. So the FRT implements the Field trait if the field is well-behaved. Is there a better way to phrase this? Maybe the parenthesis is preventing from easily parsing the sentence, it could be a footnote instead.

[BennoLossin]

The way it is currently implemented, yes, this matters a lot. If you write FieldRepresentingType::<Vec<u32>, 0, 42>, then that type exists (so no error), but does not implement the Field trait. If you try to use that type in a context that expects it to implement the Field trait, the compiler will ICE, since it just uses the variant/field index to index into the ADT data :)

I didn't see the need to make this more accessible, since this developed much later during implementation to avoid having to add another TyKind and we intend it to remain perma-unstable. If you think it's a useful thing to have or you want to make it more robust, then I'm not opposed to changing it.

[BennoLossin]

Sure! Let's draft some docs here, because I don't know what exactly you're looking for.

The usage of FRTs in std vs non-std code should generally be the same, there isn't anything special for std here I think. FRTs have two main use-cases derived from their main properties:

1. Writing field-generic code, and
2. Recording information about fields.

For the first use-case, we could imagine adding the following function to std:

impl<'b, T: ?Sized> RefMut<'b, T> {
    pub fn map_field<F>(mut orig: RefMut<'b, T>, field: F) -> RefMut<'b, F::Type>
    where
        F: Field<Base = T>,
    {
        let value: NonNull<T> = orig.value;
        let value: NonNull<F::Type> = unsafe { orig.value.byte_add(F::OFFSET) }.cast();
        RefMut { value, borrow: orig.borrow, marker: PhantomData }
    }
}

I can just make a PR that adds this function, I think it makes for a good example.

The second use-case is a bit harder to find in std, as I don't think there are constructions in std that would like to know more information about a field. @scottmcm made an interesting suggestion in the design meeting for a new pointer type: RawPtr<T, Align = 4>. Projecting this type benefits from knowing the alignment of fields, as we could project RawPtr<(u16, u8), Align = 2> to RawPtr<u8, Align = 2> (replace the tuple with a repr(linear) struct). That would require us adding the following trait:

pub unsafe trait AlignedField: Field {
    const ALIGNMENT: usize;
}

Along with a derive macro that checked if the type had a repr(linear) attribute & then implemented the trait with the appropriate constant by running the layout algorithm.

Admittedly, this example is something that we could (& probably should just) provide via the compiler instead, as that proc-macro would be very complicated.

(there also is the Pin example, which is even more complicated)

[BennoLossin]

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 :)

[Mark-Simulacrum]

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).

[BennoLossin]

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:

1. They allow one to write code that is field-generic by just using plain generics.
2. One can record information about fields that is then accessible in field-generic code.

The ultimate goal of field projections is to have an operator that allows one to go from Ptr<Struct> to Ptr<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 a PinnableField trait that not only exposes a bool on the structuralness of its pinning, but also a generic associated type that is either the identity function, or the Pin type constructor (so type Bikeshed<P> = P; or type Bikeshed<P> = Pin<P>;). You can already implement this now as an unsafe extension trait of Field that is implemented through a safe derive macro. Since our original plan, we have put some more thought into this and the Move trait got some reviving traction. At the moment, we think the Move trait is a 10x better solution, so we have not really implemented or explored the PinnableField approach 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 generic ID: u64 to record which ListLinks field a list uses; with FRTs, we could change that to a F: Field and then users would write List<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.

[Mark-Simulacrum]

This is useful background, thank you.

"generics are the natural way to write generic code in Rust, so let's just make fields be a special kind of generic"

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?

Pinnable fields

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?

At the moment we use a const generic ID: u64 to record which ListLinks field a list uses; with FRTs, we could change that to a F: Field and then users would write List<field_of!(MyFieldType, foo_list)>, which would be much more meaningful and less error-prone.

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.

[BennoLossin]

I'll mark this as ready, as I want to await your feedback regarding my proposed changes, before I commit them.

@rustbot ready

[rust-bors]

☔ The latest upstream changes (presumably #155851) made this pull request unmergeable. Please resolve the merge conflicts.