Address comments

text/0000-subtyping-coercion-inferred-target.md

@@ -0,0 +1,124 @@
+- Title: Subtyping coercion with inferred target
+- Feature Name: `subtyping_coercion_inferred_target`
+- Start Date: (fill me in with today's date, YYYY-MM-DD)
+- RFC PR: [#0000](https://github.com/rust-lang/rfcs/pull/0000)
+- Tracking Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000)
+- Team: lang
+- Keywords: coherence, pattern types, subtyping, traits, variance
+- Previous discussion: [Internals comment](https://internals.rust-lang.org/t/thoughts-on-pattern-types-and-subtyping/17675/18), [`coherence_leak_check` tracking issue](https://github.com/rust-lang/rust/issues/56105), [introduction to pattern types](https://cohost.org/oli-obk/post/165584-ranged-integers-via)
+
+This RFC uses the proposed template of [RFC 3339](https://github.com/rust-lang/rfcs/pull/3339).
+
+# Summary
+
+This RFC expands subtyping coercion, allowing it to occur in certain contexts where the target type must be inferred. As a necessary prerequisite for accomplishing this, the RFC proposes to make some patterns that are currently future-compatibility warnings under [`coherence_leak_check`](https://github.com/rust-lang/rust/issues/56105) into hard errors.
+
+The proposal smooths the path forward for future extensions to subtyping in Rust, e.g. pattern types, by ensuring that those extensions will play well with type inference.
+
+# Motivation and background
+
+## Type inference failure
+
+Consider the following code example [(playground)](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=3c43c647ec032a7909740d7d9ed081ce).
+
+```rust
+fn takes_concrete_supertype(_: fn(&'static u8)) {}
+
+trait Trait {}
+impl Trait for fn(&'static u8) {}
+
+fn takes_impl_trait(_: impl Trait) {}
+
+fn main() {
+    let fn_ptr: for<'a> fn(&'a u8) = |_| {};
+
+    // This works today. Rust realizes it needs to convert `fn_ptr`
+    // from a `for<'a> fn(&'a u8)` to its supertype `fn(&'static u8)`.
+    takes_concrete_supertype(fn_ptr);
+
+    // This doesn't compile today :(
+    // Rust doesn't infer that it must coerce
+    // the `for<'a> fn(&'a u8)` to its supertype `fn(&'static u8)`.
+    takes_impl_trait(fn_ptr);
+}
+```
+
+`for<'a> fn(&'a u8) <: fn(&'static u8)`. (`<:` means "is a subtype of"). Rust allows coerecing subtype values to supertypes in many different contexts. But, as shown as the example above, Rust can't perform these coercions when the target supertype must be inferred.
+
+In current Rust, this issue only arises with higher-ranked lifetimes on function pointers, so it's not a major problem. However, features like pattern types would expose this type inference failure in many more situations, by introducing new instances of subtyping.
+
+For code like the above to compile, Rust must be able to determine a single "best" supertype to coerce to, even though multiple options could potentially satisfy the trait bounds. This RFC's proposed changes ensure that this is possible.
+
+## `coherence_leak_check` today
+
+For any type `T` and trait `Trait`, Rust's coherence rules allow at most one  `impl Trait for T`. For any type `U` such that `U <: T` xor `T <: U`, you can write `impl Trait for T` and `impl Trait for U` together, but this triggers the `coherence_leak_check` future-compatibility lint. `coherence_leak_check` has not been made a hard error, because such impls occasionally come up in real-world code.
+
+# Explanation
+
+## New coherence rules
+
+This RFC proposes the following new coherence rules.
+
+> For any type `T` and trait `Trait`, one of the following two cases must hold:
+>
+> 1. `T` implements `Trait`.
+>
+>    1. In this case, if any of `T`'s supertypes or subtypes also implement `Trait`, those impls must come from the same `impl` block as the impl of `Trait` for `T`.
+>    2. None of `T`'s supertypes or subtypes may implement `!Trait`. (Similarly, if `T` actually implements `!Trait`, none of its subtypes or supertypes may implement `Trait`.)
+>
+> 2. `T` does not implement `Trait`. In this case, consider the set `{U}` of all supertypes of `T` that do implement `Trait`. If `{U}` is non-empty, then all of the elements of `{U}` must share a common subtype `S`, that itself implements `Trait`, and is a member of `{U}`.
+
+As far as I am aware, it is impossible to write a trait impl in current Rust that violates these rules without triggering the `coherence_leak_check` lint. Rules 1.2 and 2 are currently impossible to violate even with the lint `allow`ed.
+
+## Inferred supertype coercions
+
+These rules allow selecting a single "best" target type to coerce to in cases like the [above example](#type-inference-failure). In general, when a value of type `T` is provided but a value of a type implementing `Trait` is expected, the value of type `T` is coerced to `S`. In the specific case of the above example, `T` is `for<'a> fn(&'a u8)` and `S` is `fn(&'static u8)`.
+
+## Why rule 1.1?
+
+Rule 1.1 is not strictly necessary for this scheme to work; `S` could be inferred without it. However, the rule defuses a major footgun. Both subtyping coercion and type inference happen implicitly; the programmer may not be aware of the exact types being selected. In addition, the details of type inference are not part of Rust's stability guarantees, and can change with compiler upgrades. The "one `impl` block" rule makes it difficult (though not impossible) to write code that breaks or silently changes behavior depending on type inference details.
+
+## Why rule 1.2?
+
+This rule exists to prevent conflicts with potential future features that add variance to traits. It may be relaxed or removed in the future.
+
+## New hard errors
+
+As part of ensuring that the rules above are upheld, some patterns that currently trigger the `coherence_leak_check` lint become hard errors. Specifically, for any pair of type `T` and `U` such that `T <: U`, `impl Trait for T` and `impl Trait for U` cannot both be present at the same time. (Impls that currently trigger the lint but don't violate the above rules are unaffected; they stay future-compatibility warnings).
+
+## Migration strategy for projects relying on the old behavior
+
+Real-world code that trips `coherence_leak_check` today is rare, but not unheard-of. Searching all Rust code on GitHub, I found two projects using `#![allow(coherence_leak_check)]`:
+
+- `wasm-bindgen` could remove its `#![allow(coherence_leak_check)]` once negative impls integrated into coherence are stabilized, by adding `impl<'a, T: ?Sized> !FromWasmAbi for &'a T {}` to its source code. This RFC should not be stabilized until negative impls have been stabilized for some time.
+- The [`gcmodule`](https://github.com/quark-zju/gcmodule) project defines some impls that trigger the lint, as described [here](https://github.com/rust-lang/rust/issues/56105#issuecomment-606379619). (Old versions of Servo would also be affected by this issue, though the latest version is not.) This RFC proposes to support `gcmodule`'s use-case by ensuring that `impl<T> Trait for fn(T)` subsumes/implies `impl<T> Trait for fn(&T)`. This is to be accomplished by treating `for<'a> fn(&'a T)` as essentially equivalent to `fn(&'empty T)`,where `'empty` is a lifetime that all lifetimes outlive. More generally, any type with a higher-ranked lifetime in contravariant positions only can be treated as essentially equivalent to the same type, but with the lifetime replaced with `'empty`. The full details of how this works are explained in [this GitHub comment](https://github.com/rust-lang/rust/issues/56105#issuecomment-465634661).
+
+# Drawbacks
+
+- The proposal contains some breaking changes. Though very few projects appear to be affected, one of them is the widely-used crate `wasm-bindgen`.
+- The proposal makes type inference more complex.
+- The proposal makes the coherence rules more complex.
+- The proposal is primarily motivated by future language extensions, which might never happen.
+
+# Rationale and alternatives
+
+- Compared to doing nothing: not addressing the inference issue at all would make it impossible for libraries to start using features like pattern types in existing APIs without breaking their consumers.
+- Compared to having different subtyping coercion inference rules for higher-ranked lifetime subtyping and other forms of subtyping: this alternative would needlessly complicate the language and specification.
+- Compared to not having rule 1.1: without this restriction, Rust code would become more brittle in the face of extensions to subtyping, which is exactly counter to the motivation behind this RFC.
+
+# Prior art
+
+None that I am aware of.
+
+# Unresolved questions
+
+- The rules involving `'empty` probably need more experimentation and thought to establish with certainty they are correct.
+
+# Future possibilities
+
+- This proposal is primarily intended to support future extensions to subtyping.
+- This proposal leaves the future of some patterns that `coherence_leak_check` lints against unresolved. In the future, these could be either permitted without lint, or forbidden with or without replacement.
+- Future extensions to variance/subtyping could address more of the issues that features like pattern types face.
+- An "escape hatch" annotation of some sort could be added to allow violating proposed coherence rule 1.1 if the programmer is "really sure." (This would restrict potential future features that add variance to traits.)
+- Proposed coherence rule 1.2 could be relaxed in the future. (Ditto.)
+- Proposed coherence rule 2 could be relaxed in the future, at the price of making type inference worse when it is violated.

text/3319-aligned-trait.md

@@ -37,6 +37,8 @@ Relaxing `Self: Sized` bounds to `Self: Aligned` allows implementing those metho
 
 - `core::mem::align_of<T>()` for slices could be implemented with a library. However, a library would be unable to support records that contain a slice as the last field. Also, relaxing the trait dyn safety requirements can only be done with a language feature.
 - `?Aligned` could be accepted as new syntax, equivalent to `?Sized`. However, I don't think it's worth it to have two ways to spell the exact same concept in the same edition.
+- There may be a use-case for types that are `Sized` but not `Aligned`. However, I don't know of such, and allowing it would likely cause backward-compatibility issues.
+- Traits with methods bounded by `where Self: Aligned` could be made fully object unsafe, instead of making it impossible just to call that specific method from a trait object. However, this would make the language strictly less useful, without gaining any simplicity (as `Sized` would remain special-cased).
 
 # Prior art
 
@@ -54,3 +56,4 @@ None that I am aware of.
 - Certain `Self: Sized` bounds in the standard library could be relaxed to `Self: Aligned`. However, this might cause backward-compatibility issues.
   - [IRLO topic](https://internals.rust-lang.org/t/removing-self-sized-and-backward-compatibility/17456) on how the issues could be addressed.
 - [RFC 3308: `core::mem::offset_of`](https://github.com/rust-lang/rfcs/pull/3308) could, if accepted, be made to work on any `Aligned` type. One caveat is that if Rust ever gets structs with multiple unsized fields, those could be `Aligned` but not support `offset_of` for every field.
+- There has been discussion about adding other traits into the `Sized` hierarchy, like `DynSized`. If both `Aligned` and these other traits are integrated into Rust, their relative positions in the trait hierarchy will need to be determined.