rust-lang / rfcs

RFCs for changes to Rust
https://rust-lang.github.io/rfcs/
Apache License 2.0
5.78k stars 1.55k forks source link

Externally implementable traits #3645

Open Amanieu opened 1 month ago

Amanieu commented 1 month ago

An alternative proposal to #3632.

Rendered

Tracking:

Jules-Bertholet commented 1 month ago

Bikeshed: instead of calling it extern trait, what about extern mod? (Unlike trait, mod doesn't imply an implementing type)

Lokathor commented 1 month ago

I do not know what the good answer is, but extern trait is not good if you can't use it as a trait bound. we don't need to add confusion to the language, just use a separate name.

Jules-Bertholet commented 1 month ago

If we do want to stick with trait (for example, because it's what users are familiar with in terms of "specifying/implementing an interface"), I think we should stay consistent with the type-directed nature of Rust traits and return to a design similar to https://github.com/rust-lang/rfcs/pull/2492. (This reminds me of the traits vs first-class modules debate)

max-niederman commented 1 month ago

Bikeshed: instead of calling it extern trait, what about extern mod? (Unlike trait, mod doesn't imply an implementing type)

I like this a lot more than using "trait." IMO it's much closer to a module since, as the RFC says (emphasis added)

An extern trait may contain functions but not other associated items such as types or constants. Additionally, these functions may not refer to self or Self in their signature. Effectively, these functions follow the same rules as free functions outside an impl block.

I also think it's quite readable if you use impl mod <extern_module_name> like

// core::panic:

extern mod PanicHandler {
    fn panic_handler(_: &PanicInfo) -> !;
}

// user:

impl mod core::panic::PanicHandler {
    fn panic_handler(panic_info: &PanicInfo) -> ! {
        eprintln!("panic: {panic_info:?}");
        loop {}
    }
}
Jules-Bertholet commented 1 month ago

Also, should crates be allowed to partially impl an extern trait/mod, and let other crates impl the rest? And should there be a way to "weakly" impl such an item (permitting downstream crates to partially or fully override)?

Amanieu commented 1 month ago

The problem with #2492 is that it's not something that can be resolved at link-time: it introduces circular dependencies in the type system which can't be resolved without deferring all codegen to the root crate.

Also, should crates be allowed to partially impl an extern trait/mod, and let other crates impl the rest? And should there be a way to "weakly" impl such an item (permitting downstream crates to partially or fully override)?

No, the entire point of grouping functions in a single trait is that they must all be provided together. If you want to only partially implement an extern trait then you should provide your own extern trait for the part that your crate doesn't provide and then forward your extern impl to that.

Bikeshed: instead of calling it extern trait, what about extern mod? (Unlike trait, mod doesn't imply an implementing type)

I think a case can be made for either trait or mod, since it's really something that shares some characteristics of both. Note that there are downsides to extern mod as well, such as the syntax for extern unsafe mod or impl mod looking strange compare to how mod is usually used.

traviscross commented 1 month ago

Setting aside the nits that one could pick on this proposal, the main and interesting question here seems to be whether it should be possible to express that the person who implements an extern item must uphold certain obligations (that the compiler cannot check) in order to prevent undefined behavior.

We recently covered a similar case to this in:

There, we resolved that the person who declares the signatures within an extern block is responsible for those being correct, and that this is a separate obligation from the ones that a caller (or other user) must uphold when calling (or otherwise using) an item not marked safe within an extern.

There is conceptual overlap between that and this RFC (and the alternative proposals). Here, there's a difference between the caller having to uphold certain unchecked obligations when invoking one of these functions and the implementer having to uphold certain unchecked obligations. Given the intended and anticipated use cases for this, I can certainly see how this distinction could matter, and given that we just addressed a similar case in RFC 3484, I could see a lot of reason to be consistent here conceptually.

Due to how central this is to the motivation for this proposal, @Amanieu, I might suggest adding more discussion of this to the RFC.

Jules-Bertholet commented 1 month ago

The problem with https://github.com/rust-lang/rfcs/pull/2492 is that it's not something that can be resolved at link-time

Considering this further, I think the analogy to extern {} blocks is the right way to think about this feature. In today's extern {} (tomorrow's unsafe extern {}), you declare a set of interfaces, such that each interface has exactly one implementation. However, the compiler can't check that said implementation exists and is unique and valid, so the declarer of the extern block must unsafely promise those things.

In this hypothetical feature, one also declares a set of interfaces with exactly one outside implementation; however, now it's the compiler's job to check that this implementation exists and has the correct signature. (There may be additional preconditions on top of that, and the interface definer should be able to specify this by requiring interface implementers to use unsafe.)

Other than the burden of unsafe proof, these two features are extremely similar; therefore their syntaxes should arguably look similar also. Perhaps something like this:

//! crate foo
extern impl {
    // Unsafe to call, safe to implement
    unsafe fn frob();
    static FOO: u32;
}

unsafe extern impl {
    // Safe to call, unsafe to implement
    fn brotzle();
}
//! crate bar
extern crate foo;

impl unsafe fn foo::frob() {
    println!("frobbing");
}

impl static foo::FOO: u32 = 42;

unsafe impl fn foo::brotzle() {
    println!("brot");
}
tmandry commented 1 month ago

@Jules-Bertholet I agree; I just posted a suggestion for how to unify the proposal in the other RFC with extern blocks: https://github.com/rust-lang/rfcs/pull/3632#issuecomment-2125972702. Looks like we are thinking along the same lines.

tmandry commented 1 month ago

For this RFC, I would suggest resolving the issues that have been raised by using a concrete type for the impl. That way it's "just" a trait implementation for a regular concrete type.

// core::panic:

pub trait PanicHandler {
    fn panic_handler(_: &PanicInfo) -> !;
}

pub struct GlobalPanicHandler;
extern impl PanicHandler for GlobalPanicHandler;

// user:

impl core::panic::PanicHandler for core::Panic::GlobalPanicHandler {
    fn panic_handler(panic_info: &PanicInfo) -> ! {
        eprintln!("panic: {panic_info:?}");
        loop {}
    }
}

This shouldn't inherit the problems of the other RFC, but should be compatible with it. Perhaps it can even be made strongly forward-compatible with it in the sense that it would be forward-compatible for the above code in core to switch to using extern type in the future, if the GlobalPanicHandler type contained a PhantomExternSized or similar.

And while we're at it, we could just allow extern impl for inherent impls without needing a trait at all.

joshtriplett commented 1 month ago

Looking at this, I'm going to reiterate the concern I raised in the meeting: this seems confusingly different from other uses of trait in the language, and I think it would cause more confusion than clarity.

I would love to see a real trait-based solution, but not something that's just using the word "trait" with relatively little in common with traits.

joshtriplett commented 1 month ago

The problem with #2492 is that it's not something that can be resolved at link-time: it introduces circular dependencies in the type system which can't be resolved without deferring all codegen to the root crate.

That doesn't seem like a fatal problem. We already defer generation of generics to the point where they're instantiated. If we had to defer codegen of things that depend on an external type, would that be such a substantial problem?

(Note: I am not proposing that we block other efforts while waiting for such a solution. I'm trying to envision the simplest possible implementation of full external types with trait bounds.)

Jules-Bertholet commented 1 month ago

If we had to defer codegen of things that depend on an external type, would that be such a substantial problem?

Even if such deferral is feasible, it would be nice (for compile times) if we don't have to do it unless it's inherently necessary. If the thing being externally implemented is a plain function, deferring codegen should ideally not be forced by the design of the feature.

tmccombs commented 4 weeks ago

Another possibility could be something like:

// core::panic:

pub trait PanicHandler {
    fn panic_handler(&self, _: &PanicInfo) -> !;
}

pub extern static panic_handler: &'static  dyn PanicHandler;

// user:

struct GlobalPanicHandler;
impl core::panic::PanicHandler for GlobalPanicHandler {
    fn panic_handler(&self, panic_info: &PanicInfo) -> ! {
        eprintln!("panic: {panic_info:?}");
        loop {}
    }
}
extern static core::panic::panic_handler = &GlobalPanicHandler;

With some optimization to avoid dynamic dispatch.

max-niederman commented 4 weeks ago

Another possibility could be something like:

// core::panic:

pub trait PanicHandler {
    fn panic_handler(&self, _: &PanicInfo) -> !;
}

pub extern static panic_handler: &'static  dyn PanicHandler;

// user:

struct GlobalPanicHandler;
impl core::panic::PanicHandler for GlobalPanicHandler {
    fn panic_handler(&self, panic_info: &PanicInfo) -> ! {
        eprintln!("panic: {panic_info:?}");
        loop {}
    }
}
extern static core::panic::panic_handler = &GlobalPanicHandler;

With some optimization to avoid dynamic dispatch.

This is proposed in RFC-3635.