RFC: Trait Implementation Privacy with permits

[HackingRepo]

Rendered

[kennytm]

how does the upstream trait declaration knows the name of the downstream crate name provide the impls? any crate can call themselves mycrate_extra, so this does not prevent "malicious impls" mentioned on line 21 at all

[Lokathor]

It is generally very simple to work out how the Orphan Rule applies. The fundamental types work a little different, but there's only a handful of such types.

[teor2345]

If a malicious impl can cause memory safety issues, then the trait must be marked unsafe. Or rewritten to defend against malicious impls.

If it can't, then API design, documentation, assertions, and tests are usually the way this gets resolved. Are there specific scenarios where these aren't enough?

[workingjubilee]

To be charitable to the idea for a moment, a malicious impl could be internally unsound, technically, even if the functions are safe.

I am charitable only because that more thoroughly demonstrates the problem: one is always gambling that all the code you are calling is not internally unsound, and in that case e.g. a Debug or PartialEq impl is no different than any other trait's impls.

[teor2345]

In practice, malicious (or buggy) PartialEq is worse than a local trait, because it's called by a lot of library code, and by language operators. So reviewing / auditing all implementations and uses of PartialEq for bugs is tricky.

But at least it's a safe trait, so callers have to code defensively, because they have to accept incorrect eq() return values without any unsoundness.

Would an unsafe trait achieve some of the goals of this RFC?
Then maintaining invariants would be the trait implementer's responsibility - and callers would have clear safety preconditions.

It feels like that's roughly what's happening here, you're shifting responsibility for implementing the trait correctly on to the permitted implementers.

[teor2345]

Some thoughts on next steps and alternatives

[teor2345]

In practice, malicious (or buggy) PartialEq is worse than a local trait, because it's called by a lot of library code, and by language operators. So reviewing / auditing all implementations and uses of PartialEq for bugs is tricky.

But at least it's a safe trait, so callers have to code defensively, because they have to accept incorrect eq() return values without any unsoundness.

Would an unsafe trait achieve some of the goals of this RFC?
Then maintaining invariants would be the trait implementer's responsibility - and callers would have clear safety preconditions.

It feels like that's roughly what's happening here, you're shifting responsibility for implementing the trait correctly on to the permitted implementers.

[teor2345]

Is there an incremental way of introducing the changes in this RFC?

For example, standardising sealed traits would reduce the boilerplate, and make them more discoverable (and easier to teach).

Could we start with something like:

pub trait SealedTrait sealed {
    …
}

Then add syntax for additional permitted crates once there's a demonstrated need?

If that's the first step, can it be done outside the compiler by a macro?
(And then if the macro becomes popular, that's an argument for standardisation.)

[ChayimFriedman2]

There is already an accepted RFC for syntax sugar for sealed traits. It is less flexible than this RFC by not allowing putting external crates (which IMO is a bad idea), but more flexible by allowing specific paths within the current crate.

[ChayimFriedman2]

It is worth noting than crates have no names in Rust currently. I'm not sure Cargo is even telling rustc the crate name, and I know rust-analyzer does not require a crate name. Only dependencies have names. So this is a new concept.