comex commented on Jul 11

Bikeshed: I would prefer a syntax that could naturally be extended to support multiple stable vtable formats in the future. Perhaps something like #[vtable_repr(rust_v1)]?
Why would we ever need multiple stable formats? Well,

• We might want to add a new version someday, if backwards-incompatible changes are necessary to support new functionality (e.g. adding more built-in functions in addition to drop_in_place)
• We might want to add a COM-compatible option, assuming the initial version isn't. The problem here is that COM, like C++, stores the vtable pointer in the object itself rather than using fat pointers. But there have been proposals in the past for thin Rust trait objects; if the language gains support for those someday, going further and adding COM compatibility would be a more natural extension.

chorman0773 commented on Jul 11 •

Presently I left the attribute to be without an argument, as the point is to come up with a single multi-language vtable layout.
As mentioned in the rfc, I'm open to extensions that follow from the work to add custom abis (I do not presently know where that is, beyond the discussion on rust-lang internals). However, I'm hesitent to add an argument as its stands.
Certainly rust_v1 as the argument would seem to go against the purpose of the RFC, as its name implies its specifically for rust, not generally for systems that subscribe to the proposed abi.
If others think that the attribute should be there by default I'd be willing to add it for sure, but I'm not sure its necessary as it stands.

In reference to the COM-compatible option, that was one of the options I was exploring as an alternative.
That may be a use for providing the argument early. As for the problem of storing the object inside the pointer, that may cause some problems indeed. You could specify that dyn Trait for such traits has the same layout as

#[repr(C)]
struct COMObject{
     vtable: *const VTable,
     obj: ()-but-really-an-arbitrary-unsized-type-implementing-Trait
};

however creating references to such objects wouldn't be easy as simply, materializing a pointer to the object, and materializing a pointer to the vtable.

comex commented on Jul 11

Certainly rust_v1 as the argument would seem to go against the purpose of the RFC, as its name implies its specifically for rust, not generally for systems that subscribe to the proposed abi.

Fair enough, but I still think there should be some sort of argument.

burdges commented on Jul 11 •

If I understand, you want first to organize where extern "C" fns live inside trait object vtables? Is there really any need for an an attribute here? Could extern "C" fns just always come first? If you do need attributes, then maybe attributes should give explicit positions:

pub trait Baz {
    // Rust methods have no stable vtable position.
    fn foo(&self) -> &Self;
    fn bar(&self) -> &Self;

    /// But extern "C" methods can be assigned unique vtable positions
    #[vtable_position=1]
    extern "C" fn c_foo(self: *mut Self) -> *mut Self;
    #[vtable_position=2]
    extern "C" fn c_bar(self: *mut Self) -> *mut Self;
}

You've this separate issue about Rust stabilizing some fat pointers, which sounds harder:

We cannot do struct Foo([A],[B]) currently but maybe one day we'd allow this, meaning &Foo requires three usizes. We cannot cast &[T] to &dyn Borrow<[T]> either, again because this extra fat pointer needs three usizes, which sounds harder to do than Foo, but maybe not impossible. You must express constraints upon the pointer types that reference this trait object somehow.

Are attributes sufficient here? You might require an actual type dyn(C) Trait perhaps? We might later support stuff like &dyn(unsized=2) Borrow<([A],[B])> where the 2 records the number of dynamic sizes, so one for each slice, and this type occupies 4 usizes.

As an aside related to fat pointers, rust should add small box optimizations eventually, probably via some SmallBox<dyn Trait> type that encodes Sized types smaller no larger than usize into the pointer's space. We need this because SmallBox<dyn Trait> could work without even alloc: Any call SmallBox::new::<T>(..) succeeds if size_of_val::<&T>() < size_of::<usize>() and align_of_val::<&T>() < align_of::<usize>(), even without alloc, but without alloc then such calls panic if the size and alignment checks fail. After this, SmallBox: Deref/DerefMut determine how they handle the pointer usizes by reinvoking these size and alignment checks, which simply read from the vtable without processing the pointer. We need this so that some SmallBox<dyn Error> works without alloc, and hence that io-like traits work.

chorman0773 commented on Jul 11

burdges commented on Jul 12

I neglected to commit an edit I made right after that comment, sorry. Anyways you're right this always requires annotation because going from stable layout to unstable layout is a breaking change, so then my main question is if you want anything more explicit or just want to say the order in the trait is the order in the vtable, which is I guess how C does everything so hey..

And my final question was if you want some type like dyn(C) Trait that captures layout of the pointer itself.

chorman0773 commented on Jul 12

The RFC defines the layout order to be the declaration order of the trait.
As for dyn(C) Trait. imo it seems like this would be better as a property of the trait, rather than a property of the object type.

chorman0773 commented on Jul 14 •

Re: a mandatory argument in the attribute
I'm not sure what such an argument would be, particularily. Originally, this was overloading the meaning of repr(C), but I don't really want to use C as the argument as this isn't necessarily a "C" thing to do. I am certainly open to suggestions.

bjorn3 commented on Jul 16

How should where Self: Sized be handled? Currently it leaves a gap filled with 0, instead of completely omitting the method. This makes it very easy to go from the nth method to the position in the vtable for it: (3 + n) * size_of::<usize>()

chorman0773 commented on Jul 16

That's an omission, I'll correct it. My intention was that Self: Sized methods be omitted from the vtable. The idea is that a plugin written in a foreign language could declare the equivalent of a trait using the equivalent variation of this rfc, and pass it to an application running rust, or vice-versa. For some things, a Self: Sized -equivalent bound wouldn't make sense or even exist.

chorman0773 commented on Jul 23

Note the following rules:
The behaviour is undefined if any of the following is violated for any stable-layout-pointer. The implementation shall not cause any of these constraints to be violated:

• size shall be a multiple of align.
• align shall be a power of two.

These are imposing a limitation on the implementation as well as programs. Specifically, this means that any vtable the implementation creates must not violate those constraints. This is for compatibility with other specifications providing similar constructs. If an implementation violates them, it would be unsound on the merits that if handed to a different implementation, it violates those UB causing constraints.

cramertj commented on Oct 23

We discussed this in the last language team meeting and the consensus was that now isn't the right time for the right time for this feature. For one, our typical vtable representation isn't yet settled, so it's conceivable we'd wind up with several iterations of in-language vtable representations as a result of making promises here. Additionally, it seems like the functionality proposed here could be emulated at relatively minor ergonomic cost by using a procedural macro to generate a vtable struct and constructor function. This would also allow more flexibility, quick iteration, and evolution than an in-language tool, as well as potentially access to custom features like associated constants and static methods in vtables. With that in mind,

@rfcbot fcp postpone

rfcbot commented on Oct 23 •

Team member @cramertj has proposed to postpone this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

nikomatsakis commented on Nov 2

@rfcbot reviewed

rfcbot commented 12 days ago

This is now entering its final comment period, as per the review above.

rfcbot commented 2 days ago

The final comment period, with a disposition to postpone, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC is now postponed.