Skip to content
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

Scoped impl Trait for Type #3634

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

Tamschi
Copy link

@Tamschi Tamschi commented May 12, 2024

This may be a bit long. I thought it would be best to go over the interactions with other language features and tooling as well as potential impacts thoroughly in advance, since while the addition to the language itself can be explained quickly, it does interact with a lot of other Rust features and aspects of the crate ecosystem.

In (very) short this is a version of orphan-rule avoidance (i.e. non-unique trait implementations) that does not suffer from either version of "the hashtable problem", does not obscure imports, can be safely ignored unless needed (but is discoverable), is seamlessly compatible with specialisation and tries to strike a good balance between usability and complexity, so that developers are automatically funneled towards correct and unsurprising but also highly compatible code.

I'm aware that there are often rash proposals in this direction, but I believe I've covered all previously-discussed issues.
As the overall RFC is a fairly dense read, I've added cross-references and less-formal comments in form of blockquotes to it.


Thanks

  • to @teliosdev for some very early syntax feedback that helped put me on track,
  • to @cofinite for pointing out how scoped implementations allow syntax traits to be used as extension traits,
  • to @thefakeplace and to SkiFire13 in the draft discussion for suggestions on how to make this RFC more approachable and easier to understand.

Rendered

- Defined rules for the observed `TypeId` of generic type parameters' opaque types, rewrote sections on type identity and layout-compatibility
- Added section on sealed traits
- Corrected the use of 'blanket implementation' vs. 'generic implementation' vs. 'implementation on a generic type'
- Sketched two more warnings and one more error
- Added a section Behaviour change/Warning: `TypeId` of generic discretised using generic type parameters
- Removed the Trait binding site section but kept Coercion to trait objects and slightly expanded it
- Added section Unexpected behaviour of `TypeId::of::<Self>()` in implementations on generics in the consumer-side presence of scoped implementations and `transmute`
- Near-completely rewrote the Rationale and alternatives section with subheadings and before/after-style examples, added more positive effects of the feature
- Rewrote Alternatives section
- Removed some Unresolved questions that are now tentatively resolved
- Added top-level syntax and a field example to Explicit binding, elaborated a bit more
- Added Future possibilities:
  - Conversions where a generic only cares about specific bounds' consistency
  - Scoped bounds as contextual alternative to sealed traits
  - Glue crate suggestions
- Various small fixes, adjustments and clarifications
- Added a list of bullet points to the Summary and revised it slightly
- Coined the term *implementation environment* to refer to the set of all implementations applicable (to a given type) in a given place
- Near-completely rewrote the Logical consistency subsection to add subheadings and examples
- Small fixes and adjustments
- Distinguish between implementation-aware generics and implementation-invariant generics
- Minor edits and fixes
- Added section "(Pending changes to this draft)".
- Revised "`TypeId` of generic type parameters' opaque types" as tuples are implementation-invariant.
- Replaced section "Contextual monomorphisation of generic implementations and generic functions" with sections "Binding choice by implementations' bounds" and "Binding and generics".
- Added sections "Marking a generic as implementation-invariant is a breaking change" and "Efficient compilation".
- Renamed future possibilities section "Scoped bounds as contextual alternative to sealed traits" to "Sealed trait bounds".
- Added future possibilities section "Reusable limited-access APIs".
- a range of smaller adjustments to wording and formatting
- Implemented the former Future Possibilities section "Explicit binding" into the main text as "inline implementation environments", mainly in form of grammar extensions.
- Specified that call expressions capture the implementation environment of their function operand, acting as host for implementation-invariant generics there.
- Miscellaneous wording clarifications and fixes. (Turns out you can't call a tuple struct's implicit constructor through a type alias, whoops.)
- Added more guide-level documentation changes
- Reordered warnings and errors to the end of the reference-level explanation
- Some small adustments to wording and formatting
@Tamschi
Copy link
Author

Tamschi commented May 12, 2024

I wasn't sure what to use as "Start Date", so I used today's date.

The main question here is probably whether the feature is easy enough to understand and work with for developers. As far as possible, I tried to do this by not adding any original algorithms and by making sure that the most simple/concise implementation is most likely the ideal one in each situation, or at least can always be refactored without breaing changes, but especially the split between implementation-aware and implementation-invariant generics not quite along which ones are built-ins with special syntax adds a bit of complexity for better ergonomics.

Some of the changes are modular, for example the syntax for using a scoped implementation directly, without bringing it into scope, could be added separately from that for importing it into and later reexporting it from another scope, and referring to global implementations in place of scoped ones could be another separate addition, but anything related to type identity would have to be implemented first in order for the feature to be backwards-compatible and not require an edition change.

@Tamschi
Copy link
Author

Tamschi commented May 12, 2024

This RFC covers the use-cases of the following postponed and draft RFCs:

It would likely help with the following RFC by providing a mechanism for access control:

@ehuss ehuss added the T-lang Relevant to the language team, which will review and decide on the RFC. label May 12, 2024
@compiler-errors compiler-errors added the T-types Relevant to the types team, which will review and decide on the RFC. label May 12, 2024
Copy link

@kvverti kvverti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments for clarification and some questions.

text/3634-scoped-impl-trait-for-type.md Show resolved Hide resolved
Comment on lines +239 to +245
This also affects trait-bounded generic type parameters: If `T` is bounded on `Hash`, then
`TypeId::of::<T>()` results in distinct `TypeId`s in that context depending on the
captured implementation.

However, note that `TypeId::of::<T>()` and `TypeId::of::<T as Hash in module>()` are
always equivalent for one definition of `T`, as `TypeId::of`'s implementation does **not**
have a `T: Hash` bound!
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this was cleared up elsewhere, but as someone looking at this documentation the first time I'm confused by this explanation. On its face it seems like it's saying that Type and Type as Trait in module are both the same type and different types.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type as Trait in module is technically not a type at all, as implementations never "stick" to the outside of a type.
It's a type argument consisting of a type and an inline implementation environment.

This combination, along with any scoped implementations inherited from the surrounding scope, is "baked" into the effective (opaque) type of the generic type parameter as part of monomorphisation.
These opaque types are special-cased in terms of how the baked implementation environment influences the observed TypeId:

  • If the type parameter is wrapped in an implementation-aware generic (even indirectly), e.g. as TypeId::of::<HashSet<T>>(), then the TypeId identifies the combination of the discrete type given as part of the type argument and the full set of baked implementations.
    (Normally this would happen because the e.g. HashSet<T> is used as type argument and then has its TypeId taken inside another layer of generic function call.)

    This is necessary to ensure existing unsafe code doesn't accidentally transmute hashtables in violation of their coherence expectations, i.e. part of version 1 of "the hashtable problem".

    (Writing it out explicitly like above, without indirection through another generic call, also produces the Warning: TypeId of implementation-aware generic discretised using generic type parameters, since this is almost never what you actually want to observe when making type-keyed collections.)

  • In all other cases, the observed TypeId identifies the combination of the type given as type argument and any bounds-relevant captured implementations.

    This makes type-keyed and type-erased collections work as expected by consumer code, without over-distinguishing entries based on scoped implementations that are irrelevant to their function (which is what I call 'version 2 of "the hashtable problem"').

    You can find more on the reasoning here in the section on Logical consistency of type-erased collections.

Please let me know if this works as clarification.
I should add a cross-reference to the Logical consistency explanations here in any case, though.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is my understanding:

  • When a concrete type is used as a type argument, the observed type-identity of the corresponding type parameter in the generic item changes depending on the where-bounds present on the type parameter.

This seems strange, to say the least. As an example:

trait TypeContainer {
  type Type;
}

struct Generic<T>(T) /* where T: Marker */;
impl<T> TypeContainer for Generic<T> {
  type Type= T;
}

trait Marker {}

mod one {
  use impl super::Marker for () {}
  pub type Alias = super::Generic<()>;
}

mod two {
  use impl super::Marker for () {}
  pub type Alias = super::Generic<()>;
}

type X = ();
type Y = <one::Alias as TypeContainer>::Type;
type Z = <two::Alias as TypeContainer>::Type;

// or, equivalently
// type Y = <Generic<() as Marker in one> as TypeContainer>::Type;
// type Z = <Generic<() as Marker in two> as TypeContainer>::Type;

Is it then the case that X, Y, and Z are the same if there is no T: Marker bound, but distinct if there is one (despite all three types being known to be ())?

Copy link

@kvverti kvverti May 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or is it the case that; no, all three types are known to be identical in concrete code, but generic code defining these three types are not only not known to be identical, but are in fact known to be distinct (as witnessed by runtime type comparison).

If it is intended that the same type can have different type IDs depending on the surrounding code, have you made sure this does not cause a regression of 97156 (perhaps by allowing generic code similar to qcell to create two unique tokens for the same underlying type)?

Copy link
Author

@Tamschi Tamschi May 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or is it the case that; no, all three types are known to be identical in concrete code, but generic code defining these three types are not only not known to be identical, but are in fact known to be distinct (as witnessed by runtime type comparison).

[…]

This was my intention, but I failed to consider that generic type parameters can flow back into concrete code transparently like this. I'll have to go over this (and the issue) carefully and see how my ideas hold up and where I need to specify more thoroughly or change something.

(Tentatively, I think that the implementations can/should 'fall off' in concrete code so that X, Y, Z and () are statically known to be identical and would/could¹ yield identical TypeIds, but that this would not regress the issue because the different concrete Generic<…>s still are not at all assignable to each other (regardless of whether the bound is present or not).)

I can't look into this properly quite right now unfortunately, but hopefully I'll manage to do it at some point this week.


¹ I have to read the issue a bit more thoroughly to be sure what the current status is on this.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tentatively, I think that the implementations can/should 'fall off' in concrete code so that X, Y, Z and () are statically known to be identical and would/could¹ yield identical TypeIds

In effect, implicitly making the type of a generic type parameters a sort of Wrapper<T> with partial implementation-awareness.

On a related note, I wonder if you can automate implementation-awareness based on impl blocks (e.g. HashMap relies on K: Eq + Hash in one of its impl blocks, so functions from that impl block carry implementation-awareness, rather than the type itself?

Copy link
Author

@Tamschi Tamschi May 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tentatively, I think that the implementations can/should 'fall off' in concrete code so that X, Y, Z and () are statically known to be identical and would/could¹ yield identical TypeIds

In effect, implicitly making the type of a generic type parameters a sort of Wrapper<T> with partial implementation-awareness.

In a way, yes. I'll think about whether I can explain it that way to some extent, though it's possible it's too not-quite-the-same (for me) to fit it into the text well.

On a related note, I wonder if you can automate implementation-awareness based on impl blocks (e.g. HashMap relies on K: Eq + Hash in one of its impl blocks, so functions from that impl block carry implementation-awareness, rather than the type itself?

Implementation-awareness must be shared between distinct impl blocks.

For example, the std's HashMap has inherent, Extend, From, FromIterator, Index, PartialEq and Eq impls that expect K: Hash to be coherent. The category likely also should include external implementations in at least some cases, which for simplicity would mean all cases.

Having that information "flow backwards" from impls to overall types' identities then would introduce a lot of tricky behaviour impacts at a distance, and most likely wouldn't be compatible with early code generation for concrete code.


See also [scoped-implementation-of-external-sealed-trait].

## Type parameters capture their *implementation environment*
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sections talking about interaction of scoped implementations with generic code rely on precise terminology that is not defined and not universal within the Rust programming language community, So, I think there should be a section defining terms relating to generics as used in this RFC. I recommend using the following terminology:

Type parameter: a name that acts as a placeholder for a type, introduced by generic type and trait definitions (enum Option<T>), type alias definitions and generic associated type declarations (type Result<T> = ...;), generic function declarations (fn collect<T>(...) -> ...), generic inherent and trait implementations (impl<T> ...), and (with this RFC) imports of generic scoped implementations; collectively "generic items". The Self type is a type variable within a trait definition.

Type argument: a type (which can be a concrete type, type parameter, or partially-applied generic type) that takes the place of a type parameter in a use of a generic item. Examples include the i32 in let x: Vec<i32>, and the [T] in impl<T> Default for Box<[T]>`.

Concrete type: a type that contains no type variables.

Using this terminology, type arguments can have implementation environments.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just edited this section (and some related wording that I caught) a bit to effectively use this terminology in ef35245 ¹, but adding a section that defines the terms formally is a great idea.

I used 'concrete' and (mainly) 'discrete' interchangeably in this RFC so far, but it's also a good idea to be consistent about that. (I personally would tend to use 'discrete' for types and 'concrete' for values, but that's purely based on vibes so I'll do a bit of research tomorrow to see if there's any precedent in that regard.)

(I should be able to add the definition properly in the next one or two days, depending on how busy I am.)

¹ I'm not sure it was you who pointed this out on Discord. If not: Thank you for the review! If yes: Thanks again!

Comment on lines +523 to +526
## Type identity of discrete types
[type-identity-of-discrete-types]: #type-identity-of-discrete-types

The type identity and `TypeId::of::<…>()` of discrete types, including discretised generics, are not affected by scoped implementations *on* them.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is basically "The identity of a concrete type, including a fully-instantiated generic type, is not affected by any scoped implementations for that type." Perhaps an example would do here as well, e.g. the identity of HasSet<String> is not affected by any scoped implementations for HashSet<String>.

text/3634-scoped-impl-trait-for-type.md Show resolved Hide resolved
Comment on lines +1092 to +1093
`where`-clauses without generics or `Self` type, like `where (): Debug`, **do not** affect binding of implementations within an `impl` or `fn`, as the non-type-parameter-type `()` is unable to receive an *implementation environment* from the discretisation site.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that a scoped implementation of use impl Debug for () { ... } does not affect the behavior of the function? That is, trivial bounds are still trivial with this RFC, because implementation environments are only carried by type arguments. This could be unexpected behavior.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, exactly that. (More precisely, such a scoped implementation at the caller does not affect the behaviour of the function, but at its definition it does, and the trivial bound is satisfied according to the function definition's implementation environment too.)

I think that, in this case, it's overall better to keep monomorphisation in check and not turn discrete-as-written implementations into monomorphised ones. Rustdoc does display trivial bounds though 🤔
(Another practical issue is inability to write implementation environments inline for anything that's not a generic type argument… but that could just be a case of me personally coming up blank if I think about the syntax for it.)

Changing this doesn't sit well with me because it seems unintuitive (it would make where-clauses a form of first-class implicit-only parameter) and there may be macros that emit spurious trivial bounds. It would also break that trivial bounds can be used as assertions, at least if allowed without global implementation… though the static_assertions crate notably does not currently do this.

In general, I'm very hesitant to add anything that reinterprets currently valid Rust code, even if I'm pretty sure this wouldn't be a compilation-breaking change. Maybe it would be a better idea to hide trivial bounds from the docs instead, going for consistency in the other direction?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me, but you might consider explicitly calling out that trivial bounds are not affected by scoped implementations.

text/3634-scoped-impl-trait-for-type.md Outdated Show resolved Hide resolved
Comment on lines +1383 to +1404
The following functions do not compile, as the implicitly returned `()` is not stated *inside* the scope where the implementation is available:

```rust
trait Trait {}

fn function() -> impl Trait {
^^^^^^^^^^
use impl Trait for () {}
---------------------

// Cannot bind on implicit `()` returned by function body without trailing *Expression*.
}

fn function2() -> impl Trait {
^^^^^^^^^^
use impl Trait for () {}
---------------------

return; // Cannot bind on `return` without expression.
-------
}
```
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would the implicit return not pick up scoped implementations?

Copy link
Author

@Tamschi Tamschi May 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory it could be made to, but I (subectively) think that would be hard to follow, since for example other items nested in function bodies aren't available to their signature (if I'm not completely mistaken).

The function body block isn't visually located inside itself, so to me it appears one or a half level of scoping higher than a scoped impl Trait for Type placed inside it.

Copy link

@kvverti kvverti May 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like too subtle of a distinction to make. I don't think most rust programmers conceptualize the implicit return value to be any different than an explicit return value. While, yes, there are already subtle differences with regards to implicit drop and lifetimes, I don't think this RFC should add to those pain points by distinguishing between implicit return, return with expression, and return without expression.

However, I admit that the surface area of this particular change is rather small, as it only affects functions that return ().

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a strong preference in this regard. return without expression at least could definitely be defined to pick up the environment on the return token instead without it feeling odd in the long run, so I'll change that here when I can.

If I had totally free reign, I would probably put off the matter with the fully implicit return until the very last minute and then run a survey, but I don't know if that's a thing 😅
It's really hard for me to tell whether reaching the end of the function body should observe the implementation environment just outside or just inside the function, though. From a purely practical/convenience point of view, I think I would say 'inside' too, and I now suspect it may also be closer to developer intent in mixed scenarios, so I'll change it to that.

(Sorry, today I'm having some trouble with the wider reason it took me so long to submit this in the first place, so I'll be a bit slow to implement changes and respond to the larger points you brought up.)

text/3634-scoped-impl-trait-for-type.md Outdated Show resolved Hide resolved
Comment on lines +2876 to +2895
```rust
struct Type;
struct Generic<T>(T);
trait Trait {}

mod a {
use super::{Type, Generic, Trait};
pub use impl Trait for Type {}
pub type Alias = Generic<T>;
}

mod b {
use super::{Type, Generic, Trait};
pub use impl Trait for Type {}
pub type Alias = Generic<T>;
}

use impl Trait for a::Alias {}
use impl Trait for b::Alias {}
```
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to be able to have distinct impl Trait for Generic<Type as X in one> and impl Trait for Generic<Type as X in two>? This seems unexpected.

Copy link
Author

@Tamschi Tamschi May 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's required for backwards-compatibility, or at least having a distinct impl Trait for Generic<Type> that isn't available for Generic<Type as X in one> or Generic<Type as X in two> is. Existing code can make safety-critical assumptions about the exact behaviour of the former even if X is not sealed.

(Forbidding implementations for concrete types that contain captured scoped implementations outright is technically also possible, but I don't think that would helpful.)

Fitting these distinct implementation targets into Rustdoc could be interesting, though, just in terms of information arrangement.

…*" to "## Type arguments capture their *implementation environment*" and clarified the role of type arguments vs. type parameters

This was pointed out in the Rust Programming Language Community Discord.
Fixes https://github.com/rust-lang/rfcs/pull/3634/files#r1598697056 . An inline implementation environment can only refer to implementations that it can actually see.
@Tamschi
Copy link
Author

Tamschi commented May 14, 2024

(In fairness I should say that a few of those 🚀 reactions on the OP are from friends who have been following along a bit and seem excited about this. I did not ask them to add those, didn't even give them a link directly to this PR.)

@kvverti
Copy link

kvverti commented May 14, 2024

I think the consequences for type identity should be discussed more.

  • Currently, if T and U are type-equal, then Generic<T> and Generic<U> are type-equal. This RFC changes that.
  • Currently, a type T has a consistent type ID regardless of the context in which the type ID is requested. This RFC changes that.

@burdges
Copy link

burdges commented May 14, 2024

I'd think scoped traits wind up unsound..

crate x

pub trait SomeTrait { .. }

put unsafe trait SomeTraitConforms : SomeTrait { }

pub fn bar<T: SomeTrait>(..) {
}

pub fn baz<T: SomeTraitConforms>(..) {
    unsafe { .. }
}

crate y

pub struct Foo { .. }
impl SomeTrait for Foo { .. }
impl SomeTraitConforms for Foo { .. }

crate z

use impl SomeTrait for Foo { .. }

// This must use the local SomeTrait 
bar::<Foo>(..)

// This must use the remote SomeTrait, because obviously the local is unsound here.
baz::<Foo>(..)

// Yet, baz could assume that bar invocations used the same SomeTrait, thereby creating unsoundness.

I've not yet explored solutions, but maybe traits should opt into this using an attribute like #[unsafe_forbit_unsafe_subtraits], but Rust has many marker traits which get used as supertraits, so then many traits cannot have local definitions. Also, we've no idea if that's a compelte fix, so really soundness should be formally proven here, ala the rustbelt work.

@Tamschi
Copy link
Author

Tamschi commented May 14, 2024

I'd think scoped traits wind up unsound..

[…]

This code wouldn't compile under this RFC. Subtrait impls are shadowed along with any supertrait impls they require, and can only be imported into scopes where all their matching supertrait impls are also present.

@burdges
Copy link

burdges commented May 14, 2024

Alright fair enough, but SomeTraitConforms need not be a subtrait of SomeTrait, so then pub fn baz<T: SomeTrait+SomeTraitConforms>(..) creates problems.

Also, you could disable seemingly unrelated impls unexpectedly, right? You cannot roll back to an un-specialized impl just because the specilized one depends upon some unsafe subtrait, so presumably some expected generic impls disapear when doing this. That's not necessarily a problem, just surprising.

@lcnr lcnr added T-lang Relevant to the language team, which will review and decide on the RFC. and removed T-lang Relevant to the language team, which will review and decide on the RFC. labels May 15, 2024
@lcnr
Copy link
Contributor

lcnr commented May 15, 2024

I am reaching out here as a member of the Types Team, even if I do not speak for the team as a whole. It is obvious that a lot of work went into this RFC and the design. From what I can tell by looking over it, you've been very thorough and it looks like you've spent a significant effort to make sure learn and apply existing terminology and concepts. This makes it even more disheartening to shut this down without providing any technical feedback.

I expect this RFC to end up getting stale without ever receiving feedback or reasoning from T-lang or T-types, regardless of whether the proposed changes are desirable as described.

This is a very big change to the core type system and we do not currently have to capacity to appropriately review and implement this. From a quick skim, I can tell that it allows some sort of specializing impls (which I expect to require some sort of "always applicable impls" rust-lang/rust#48538), it adds an additional concept similar to variance, and impacts TypeId depending on location in the source. While none if these additions are necessarily unsound, they require a significant amount of care and a proper understanding of the underlying interactions. This holds not only for the author, but also for the team members approving this change.

While we don't officially communicate this anywhere from what I can tell1, such big changes should already have the affected teams in the loop far before you've fleshed it out this much. Both the language and the types team have the MCP process (similar to #2904) and the more or less official concept of a 'liason': someone from the team who is involved in the design and acts as an advisor and a bridge to the rest of the team. This should also catch cases where a proposed solution is not feasible due to a lack of capacity without first requiring the contributor to spend dozens of hours on it.

I do not have any useful recommendations for what you should do here. While you can definitely reach of on the relevant zulip streams (types lang) to get a team member to liason for this work and to try and push this forward, I don't expect that there is a types team member with the necessary capacity. It may be possible to split it in multiple smaller proposals and make progress on these, though I would also only recommend that once you have a team member on board.

I am very sorry that you ended up at this point without getting such feedback earlier. If there's anything you want to respond here or talk about, feel free to do so here or on zulip, either via pm or in a public stream.

Footnotes

  1. which is incredibly bad and something we have to change. I made a note to formalize our suggested workflows when proposing changes affecting the types team

@Tamschi
Copy link
Author

Tamschi commented May 15, 2024

That's totally fine! Well, maybe not entirely in the general sense, but to be honest I somewhat expected that to be the case.
(I think I would have written this up anyway. It has been an interesting exercise in many ways and it's something I wanted to be "out there". I'm sorry if this is causing any work for you, though.)

I'm going to reach out and see if anyone would be interested in reading this, even if it's just out of personal curiosity and without commitment to a thorough review or anything like that. It's very difficult for me to get feedback on this otherwise, so I'd be grateful about anything, really. Also to anyone on this issue who'd still entertain me without expectation that this RFC ends up making a difference, of course.
(It's painfully obvious from my writing style, but just to be clear, I'm not part of academia and don't have the means of submitting this for review anywhere.)

You're mistaken about one thing though: I didn't put much specific effort into the research for this. Terminology and concepts happen to be something I pick up passively for the most part, and I do enough systems programming (as a hobby) that I'd ran into most of them beforehand already. The main avoidable difficulty was that I didn't come across much of the earlier discussion and with that prior art until I was far along with the concept, so I ended up re-deriving most of it with stable Rust as a base.

Thanks for the considerate response and pointers!

@Tamschi
Copy link
Author

Tamschi commented May 15, 2024

Alright fair enough, but SomeTraitConforms need not be a subtrait of SomeTrait, so then pub fn baz<T: SomeTrait+SomeTraitConforms>(..) creates problems.

That's true. I don't expect this to be a problem for standard library traits (the Trusted… traits are subtraits of those they describe and as far as I can tell right now it's not an issue for existing supertraits beyond what's already clearly unsound), but a rule against the linking of (including indirectly as supertrait) an unsafe trait and an unrelated scoped implementation like that is necessary at least for existing unsafe traits.

This would also affect cases where the type with attached scoped implementation appears as type parameter or associated type in the unsafe trait. This would have to be measured, but it could turn out to be a relatively small amount of friction.

It would be possible to add an #[asserts_at_most_supertrait_implementations] attribute to be applied on the unsafe trait's declaration to allow such combinations, and/or make that the default in an edition change and add an attribute to opt-out of the requirement.

Also, you could disable seemingly unrelated impls unexpectedly, right? You cannot roll back to an un-specialized impl just because the specilized one depends upon some unsafe subtrait, so presumably some expected generic impls disapear when doing this. That's not necessarily a problem, just surprising.

(I'm not entirely sure I understand you correctly, so some of this may be irrelevant.)

Blanket-impls aren't automatically shadowed if some of their bounds can't be fulfilled in a scope, so impls where the trait itself isn't dependent won't disappear automatically. To my knowledge, specialisations can't have different supertraits from any other implementation of that trait, so nothing would become "de-specialised" directly.

A global blanket impl that becomes reapplied could unexpectedly shadow a more specific implementation with the current proposal though, for example impl<T> Trait for T where T: Other { … } would shadow impl Trait for Type { … } in scopes with use impl Other for Type { … }.

(This RFC doesn't allow any mixing of overlapping implementations at all by itself, except through strict lexical shadowing and pointing out how it would interact with existing overlaps from specialisation-proper. That's part of why I don't think it necessarily requires "always applicable impls". The other part is that generic code also can't mis-recognise a specific type with accessible caller-side scoped implementation to instead have the global implementation, due to the distinct TypeId that such a bounds-relevant capture causes in generic code. cc @lcnr)

@burdges
Copy link

burdges commented May 17, 2024

As an aside, it's plausible a soundness proof for some flavor of this winds up being an interesting paper for some academics.

@Tamschi
Copy link
Author

Tamschi commented May 19, 2024

As an aside, it's plausible a soundness proof for some flavor of this winds up being an interesting paper for some academics.

Maybe! I don't think I have the mathematical toolkit to do that rigorously, but it does seem like a cleanly defined problem.

Though, a few of Rust's implicit properties that cause issues here, like 'unsafe impl Trait can guarantee unrelated impl Traits generically', are "grown" and not inherent to a bounds-on-impls programming language with scoped implementations, and also not necessarily useful, so I don't think such a programming language/paper should allow that if it has the choice.

Fortunately that particular one seems to be self-contained enough that a few extra rules can patch it up.
(I'm intermittently very busy, so it could take a little while for me to make changes here and will likely happen in bursts.)

@nikomatsakis
Copy link
Contributor

nikomatsakis commented May 21, 2024 via email

@Tamschi
Copy link
Author

Tamschi commented May 22, 2024

I'll fix the direct issues with the RFC that came up here before doing so, but thanks, I'll look into it. (There isn't really a lot that is arbitrary, aside from syntax specifics, so I'd like to make sure the idea is plausible as presented then.)

@Tamschi
Copy link
Author

Tamschi commented May 26, 2024

Alright fair enough, but SomeTraitConforms need not be a subtrait of SomeTrait, so then pub fn baz<T: SomeTrait+SomeTraitConforms>(..) creates problems.

Fixed in 360a594 (Forbidden implementation combinations, permalink).

This could cause some friction since the restriction wouldn't be desirable if this feature was drawn up in a vacuum, but hopefully nothing all too serious. It's also something that could potentially eventually be cleaned up for future unsafe traits in an edition change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-lang Relevant to the language team, which will review and decide on the RFC. T-types Relevant to the types team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants