From b32c37ef870e3b0eab2eadfc7d4279204ace628b Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Sat, 4 May 2024 00:31:31 +0100 Subject: [PATCH 01/11] Supertrait item shadowing v2 --- text/0000-supertrait-item-shadowing-v2.md | 77 +++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 text/0000-supertrait-item-shadowing-v2.md diff --git a/text/0000-supertrait-item-shadowing-v2.md b/text/0000-supertrait-item-shadowing-v2.md new file mode 100644 index 00000000000..78e1e941e82 --- /dev/null +++ b/text/0000-supertrait-item-shadowing-v2.md @@ -0,0 +1,77 @@ +- Feature Name: `supertrait_item_shadowing` +- Start Date: 2024-05-04 +- RFC PR: [rust-lang/rfcs#0000](https://github.com/rust-lang/rfcs/pull/0000) +- Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000) + +# Summary +[summary]: #summary + +When name resolution encounters an ambiguity between 2 trait methods, if one trait is a sub-trait of the other then select that method instead of reporting an ambiguity error. + +# Motivation +[motivation]: #motivation + + +The libs-api team would like to stabilize `Iterator::intersperse` but has a problem. The `itertools` crate already has: + +```rust +// itertools +trait Itertools: Iterator { + fn intersperse(self, element: Self::Item) -> Intersperse; +} +``` + +This method is used in crates with code similar to the following: + +```rust +use core::iter::Iterator; // Implicit import from prelude + +use itertools::Itertools as _; + +fn foo() -> impl Iterator { + "1,2,3".split(",").intersperse("|") + // ^ This is ambiguious: it could refer to Iterator::intersperse or Itertools::intersperse +} +``` + +This code actually works today because `intersperse` is an unstable API, which works because the compiler already has [logic](https://github.com/rust-lang/rust/pull/48552) to prefer stable methods over unstable methods when an amiguity occurs. + +Attempts to stabilize `intersperse` have failed with a large number of regressions [reported by crater](https://github.com/rust-lang/rust/issues/88967) which affect many popular crates. Even if these were to be manually corrected (since ambiguity is considered allowed breakage) we would have to go through this whole process again every time a method from `itertools` is uplifted to the standard library. + +# Proposed solution +[proposed-solution]: #proposed-solution + +This RFC proposes to change name resolution to resolve the ambiguity in the following specific circumstances: +- All method candidates are trait methods. (Inherent methods are already prioritized over trait methods) +- One trait is transitively a sub-trait of all other traits in the candidate list. + +When this happens, the sub-trait method is selected instead of reporting an ambiguity error. + +# Drawbacks +[drawbacks]: #drawbacks + +This behavior can be surprising: adding a method to a sub-trait can change which function is called in unrelated code. A lint could be emitted to warn users about the potential ambiguity. + +# Rationale and alternatives +[rationale-and-alternatives]: #rationale-and-alternatives + +If we choose not to accept this RFC then there doesn't seem to be a reasonable path for adding new methods to the `Iterator` trait if such methods are already provided by `itertools` without a lot of ecosystem churn. + +# Prior art +[prior-art]: #prior-art + +### RFC 2845 + +RFC 2845 was a previous attempt to address this problem, but it has several drawbacks: +- It doesn't fully address the problem since it only changes name resolution when trait methods are resolved due to generic bounds. In practice, most of the amiguity from stabilizing `intersperse` comes from non-generic code. +- It adds a lot of complexity because name resolution depends on the specific trait bounds that have been brought into scope. + +# Unresolved questions +[unresolved-questions]: #unresolved-questions + +None + +# Future possibilities +[future-possibilities]: #future-possibilities + +None From de6a4cfdc83edd41c0e412b2924e782cb6b75055 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Mon, 6 May 2024 11:09:27 +0200 Subject: [PATCH 02/11] Clarify that this only triggers when both traits are in scope --- text/0000-supertrait-item-shadowing-v2.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/text/0000-supertrait-item-shadowing-v2.md b/text/0000-supertrait-item-shadowing-v2.md index 78e1e941e82..7a4a0bb6a91 100644 --- a/text/0000-supertrait-item-shadowing-v2.md +++ b/text/0000-supertrait-item-shadowing-v2.md @@ -6,7 +6,7 @@ # Summary [summary]: #summary -When name resolution encounters an ambiguity between 2 trait methods, if one trait is a sub-trait of the other then select that method instead of reporting an ambiguity error. +When name resolution encounters an ambiguity between 2 trait methods when both traits are in scope, if one trait is a sub-trait of the other then select that method instead of reporting an ambiguity error. # Motivation [motivation]: #motivation @@ -47,6 +47,8 @@ This RFC proposes to change name resolution to resolve the ambiguity in the foll When this happens, the sub-trait method is selected instead of reporting an ambiguity error. +Note that this only happens when *both* traits are in scope since this is required for the ambiguity to occur in the first place. + # Drawbacks [drawbacks]: #drawbacks From 95594f76884586ab578a691eef26d008f7cbbba3 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Tue, 28 May 2024 23:21:03 +0100 Subject: [PATCH 03/11] Add more alternatives --- text/0000-supertrait-item-shadowing-v2.md | 52 +++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/text/0000-supertrait-item-shadowing-v2.md b/text/0000-supertrait-item-shadowing-v2.md index 7a4a0bb6a91..51a911f5c44 100644 --- a/text/0000-supertrait-item-shadowing-v2.md +++ b/text/0000-supertrait-item-shadowing-v2.md @@ -59,6 +59,58 @@ This behavior can be surprising: adding a method to a sub-trait can change which If we choose not to accept this RFC then there doesn't seem to be a reasonable path for adding new methods to the `Iterator` trait if such methods are already provided by `itertools` without a lot of ecosystem churn. +## Only doing this for specific traits + +One possible alternative to a general change to the name resolution rules would be to only do so on a case-by-case basis for specific methods in standard library traits. This could be done by using a perma-unstable `#[shadowable]` attribute specifically on methods like `Iterator::intersperse`. + +There are both advantages and inconvenients to this approach. While it allows most Rust users to avoid having to think about this issue for most traits, it does make the `Iterator` trait more "magical" in that it doesn't follow the same rules as the rest of the language. Having a consistent rule for how name resolution works is easier to teach people. + +## Preferring the supertrait method instead + +In cases of ambiguity between a subtrait method and a supertrait method, there are 2 ways of resolving the ambiguity. This RFC proposes to resolve in favor of the subtrait since this is most likely to avoid breaking changes in practice. + +Consider this situation: + +- library A has trait `Foo` +- crate B, depending on A, has trait `FooExt` with `Foo` as a supertrait +- A adds a new method to `Foo`, but it has a default implementation so it's not breaking. B has a pre-existing method with the same name. + +In this general case, the reason this cannot be resolved in favor of the supertrait is that the method signatures are not necessarily compatible. + +[In code](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=b3919f7a8480c445d40b18a240936a07): + +```rust +#![allow(unused)] + +mod a { + pub trait Int { + // fn call(&self) -> u32 { + // 0 + // } + } + impl Int for () {} +} + +mod b { + pub trait Int: super::a::Int { + fn call(&self) -> u8 { + 0 + } + } + impl Int for () {} +} + +use a::Int as _; +use b::Int as _; + +fn main() { + let val = ().call(); + println!("{}", std::any::type_name_of_val(&val)); +} +``` + +Resolving in favor of `a` is a breaking change; in favor of `b` is not. The only other option is the status quo: not compiling. `a` simply cannot happen lest we violate backwards compatibility and the status quo is not ideal. + # Prior art [prior-art]: #prior-art From 57892c5dc91bde694e5da496e9ce5cc15e1afd7d Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Tue, 23 Jul 2024 19:40:02 +0100 Subject: [PATCH 04/11] Add lint and add type inference example --- text/0000-supertrait-item-shadowing-v2.md | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/text/0000-supertrait-item-shadowing-v2.md b/text/0000-supertrait-item-shadowing-v2.md index 51a911f5c44..f171188b819 100644 --- a/text/0000-supertrait-item-shadowing-v2.md +++ b/text/0000-supertrait-item-shadowing-v2.md @@ -49,10 +49,31 @@ When this happens, the sub-trait method is selected instead of reporting an ambi Note that this only happens when *both* traits are in scope since this is required for the ambiguity to occur in the first place. +When an ambiguity is resolved in this way, a lint warning is also emitted to warn the user about the potential ambiguity. The aim of this lint is to discourage reliance on this mechanism in normal code usage: it should only be used for backwards-compatibilty and the lint can be silenced by having users change their code. We can always later change this lint to be allowed by default if we consider that there are valid use cases for this feature other than backwards-compatiblity. + +### Type inference + +This change happens during name resolution and specifically doesn't interact with type inference. Consider this example: + +```rust +trait Foo { fn method(&self) {} } +trait Bar: Foo { fn method(&self) {} } +impl Foo for Vec { } +impl Bar for Vec { } + +fn main() { + let x = vec![]; + x.method(); // which to call? + x.push(Box::new(22)); // oh, looks like `Foo` +} +``` + +Today that example will give an ambiguity error because `method` is provided by multiple traits in scope. With this RFC, it will instead always resolve to the sub-trait method and then compilation will fail because `Vec` does not implement the `Copy` trait required by `Bar::method`. + # Drawbacks [drawbacks]: #drawbacks -This behavior can be surprising: adding a method to a sub-trait can change which function is called in unrelated code. A lint could be emitted to warn users about the potential ambiguity. +This behavior can be surprising: adding a method to a sub-trait can change which function is called in unrelated code. This is mitigated by the which warns users about the potential ambiguity. # Rationale and alternatives [rationale-and-alternatives]: #rationale-and-alternatives From c4cc0a8f8fc323c3004aa66430fa9c0a24be6d9e Mon Sep 17 00:00:00 2001 From: Travis Cross Date: Fri, 3 Jan 2025 19:37:35 +0000 Subject: [PATCH 05/11] Fix some typographical errors Thanks to zachs18 and andresovela for some of these. --- text/0000-supertrait-item-shadowing-v2.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/text/0000-supertrait-item-shadowing-v2.md b/text/0000-supertrait-item-shadowing-v2.md index f171188b819..0aa5e073e5a 100644 --- a/text/0000-supertrait-item-shadowing-v2.md +++ b/text/0000-supertrait-item-shadowing-v2.md @@ -30,11 +30,11 @@ use itertools::Itertools as _; fn foo() -> impl Iterator { "1,2,3".split(",").intersperse("|") - // ^ This is ambiguious: it could refer to Iterator::intersperse or Itertools::intersperse + // ^ This is ambiguous: it could refer to Iterator::intersperse or Itertools::intersperse } ``` -This code actually works today because `intersperse` is an unstable API, which works because the compiler already has [logic](https://github.com/rust-lang/rust/pull/48552) to prefer stable methods over unstable methods when an amiguity occurs. +This code actually works today because `intersperse` is an unstable API, which works because the compiler already has [logic](https://github.com/rust-lang/rust/pull/48552) to prefer stable methods over unstable methods when an ambiguity occurs. Attempts to stabilize `intersperse` have failed with a large number of regressions [reported by crater](https://github.com/rust-lang/rust/issues/88967) which affect many popular crates. Even if these were to be manually corrected (since ambiguity is considered allowed breakage) we would have to go through this whole process again every time a method from `itertools` is uplifted to the standard library. @@ -73,7 +73,7 @@ Today that example will give an ambiguity error because `method` is provided by # Drawbacks [drawbacks]: #drawbacks -This behavior can be surprising: adding a method to a sub-trait can change which function is called in unrelated code. This is mitigated by the which warns users about the potential ambiguity. +This behavior can be surprising: adding a method to a sub-trait can change which function is called in unrelated code. This is mitigated by the lint which warns users about the potential ambiguity. # Rationale and alternatives [rationale-and-alternatives]: #rationale-and-alternatives @@ -138,7 +138,7 @@ Resolving in favor of `a` is a breaking change; in favor of `b` is not. The only ### RFC 2845 RFC 2845 was a previous attempt to address this problem, but it has several drawbacks: -- It doesn't fully address the problem since it only changes name resolution when trait methods are resolved due to generic bounds. In practice, most of the amiguity from stabilizing `intersperse` comes from non-generic code. +- It doesn't fully address the problem since it only changes name resolution when trait methods are resolved due to generic bounds. In practice, most of the ambiguity from stabilizing `intersperse` comes from non-generic code. - It adds a lot of complexity because name resolution depends on the specific trait bounds that have been brought into scope. # Unresolved questions From f54263a0c8112abadf31115e679b6017818b04d9 Mon Sep 17 00:00:00 2001 From: Travis Cross Date: Fri, 3 Jan 2025 19:39:09 +0000 Subject: [PATCH 06/11] Adjust according to 2024-10-30 lang feedback Co-authored-by: zachs18 --- text/0000-supertrait-item-shadowing-v2.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/text/0000-supertrait-item-shadowing-v2.md b/text/0000-supertrait-item-shadowing-v2.md index 0aa5e073e5a..738f097de91 100644 --- a/text/0000-supertrait-item-shadowing-v2.md +++ b/text/0000-supertrait-item-shadowing-v2.md @@ -49,7 +49,7 @@ When this happens, the sub-trait method is selected instead of reporting an ambi Note that this only happens when *both* traits are in scope since this is required for the ambiguity to occur in the first place. -When an ambiguity is resolved in this way, a lint warning is also emitted to warn the user about the potential ambiguity. The aim of this lint is to discourage reliance on this mechanism in normal code usage: it should only be used for backwards-compatibilty and the lint can be silenced by having users change their code. We can always later change this lint to be allowed by default if we consider that there are valid use cases for this feature other than backwards-compatiblity. +We will provide an allow-by-default lint to let users opt in to being notified when an ambiguity is resolved in this way. ### Type inference @@ -73,7 +73,7 @@ Today that example will give an ambiguity error because `method` is provided by # Drawbacks [drawbacks]: #drawbacks -This behavior can be surprising: adding a method to a sub-trait can change which function is called in unrelated code. This is mitigated by the lint which warns users about the potential ambiguity. +This behavior might be surprising as adding a method to a subtrait can change which function is called in unrelated code. This is somewhat mitigated by the opt-in lint which, when enabled, warns users about the potential ambiguity. # Rationale and alternatives [rationale-and-alternatives]: #rationale-and-alternatives @@ -144,7 +144,7 @@ RFC 2845 was a previous attempt to address this problem, but it has several draw # Unresolved questions [unresolved-questions]: #unresolved-questions -None +- Should we have a warn-by-default lint that fires at the definition-site of a subtrait that shadows a supertrait item? # Future possibilities [future-possibilities]: #future-possibilities From 0872f23aa953344eb8a10e28efeeb5b79bb970bb Mon Sep 17 00:00:00 2001 From: Travis Cross Date: Fri, 3 Jan 2025 20:22:58 +0000 Subject: [PATCH 07/11] Make some editorial adjustments --- text/0000-supertrait-item-shadowing-v2.md | 32 ++++++++++------------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/text/0000-supertrait-item-shadowing-v2.md b/text/0000-supertrait-item-shadowing-v2.md index 738f097de91..3d8a295f454 100644 --- a/text/0000-supertrait-item-shadowing-v2.md +++ b/text/0000-supertrait-item-shadowing-v2.md @@ -6,12 +6,11 @@ # Summary [summary]: #summary -When name resolution encounters an ambiguity between 2 trait methods when both traits are in scope, if one trait is a sub-trait of the other then select that method instead of reporting an ambiguity error. +When name resolution encounters an ambiguity between two trait methods when both traits are in scope, if one trait is a subtrait of the other then select that method instead of reporting an ambiguity error. # Motivation [motivation]: #motivation - The libs-api team would like to stabilize `Iterator::intersperse` but has a problem. The `itertools` crate already has: ```rust @@ -34,7 +33,7 @@ fn foo() -> impl Iterator { } ``` -This code actually works today because `intersperse` is an unstable API, which works because the compiler already has [logic](https://github.com/rust-lang/rust/pull/48552) to prefer stable methods over unstable methods when an ambiguity occurs. +This code actually works today because `intersperse` is an unstable API, and the compiler already has [logic](https://github.com/rust-lang/rust/pull/48552) to prefer stable methods over unstable methods when an ambiguity occurs. Attempts to stabilize `intersperse` have failed with a large number of regressions [reported by crater](https://github.com/rust-lang/rust/issues/88967) which affect many popular crates. Even if these were to be manually corrected (since ambiguity is considered allowed breakage) we would have to go through this whole process again every time a method from `itertools` is uplifted to the standard library. @@ -42,10 +41,11 @@ Attempts to stabilize `intersperse` have failed with a large number of regressio [proposed-solution]: #proposed-solution This RFC proposes to change name resolution to resolve the ambiguity in the following specific circumstances: -- All method candidates are trait methods. (Inherent methods are already prioritized over trait methods) -- One trait is transitively a sub-trait of all other traits in the candidate list. -When this happens, the sub-trait method is selected instead of reporting an ambiguity error. +- All method candidates are trait methods (inherent methods are already prioritized over trait methods). +- One trait is transitively a subtrait of all other traits in the candidate list. + +When this happens, the subtrait method is selected instead of reporting an ambiguity error. Note that this only happens when *both* traits are in scope since this is required for the ambiguity to occur in the first place. @@ -68,7 +68,7 @@ fn main() { } ``` -Today that example will give an ambiguity error because `method` is provided by multiple traits in scope. With this RFC, it will instead always resolve to the sub-trait method and then compilation will fail because `Vec` does not implement the `Copy` trait required by `Bar::method`. +Today that example will give an ambiguity error because `method` is provided by multiple traits in scope. With this RFC, it will instead always resolve to the subtrait method and then compilation will fail because `Vec` does not implement the `Copy` trait required by `Bar::method`. # Drawbacks [drawbacks]: #drawbacks @@ -84,17 +84,17 @@ If we choose not to accept this RFC then there doesn't seem to be a reasonable p One possible alternative to a general change to the name resolution rules would be to only do so on a case-by-case basis for specific methods in standard library traits. This could be done by using a perma-unstable `#[shadowable]` attribute specifically on methods like `Iterator::intersperse`. -There are both advantages and inconvenients to this approach. While it allows most Rust users to avoid having to think about this issue for most traits, it does make the `Iterator` trait more "magical" in that it doesn't follow the same rules as the rest of the language. Having a consistent rule for how name resolution works is easier to teach people. +There are both advantages and inconveniences to this approach. While it allows most Rust users to avoid having to think about this issue for most traits, it does make the `Iterator` trait more "magical" in that it doesn't follow the same rules as the rest of the language. Having a consistent rule for how name resolution works is easier to teach people. ## Preferring the supertrait method instead -In cases of ambiguity between a subtrait method and a supertrait method, there are 2 ways of resolving the ambiguity. This RFC proposes to resolve in favor of the subtrait since this is most likely to avoid breaking changes in practice. +In cases of ambiguity between a subtrait method and a supertrait method, there are two ways of resolving the ambiguity. This RFC proposes to resolve in favor of the subtrait since this is most likely to avoid breaking changes in practice. Consider this situation: -- library A has trait `Foo` -- crate B, depending on A, has trait `FooExt` with `Foo` as a supertrait -- A adds a new method to `Foo`, but it has a default implementation so it's not breaking. B has a pre-existing method with the same name. +- Library A has trait `Foo`. +- Crate B, depending on A, has trait `FooExt` with `Foo` as a supertrait. +- A adds a new method to `Foo`, but it has a default implementation so it's not breaking. B has a preexisting method with the same name. In this general case, the reason this cannot be resolved in favor of the supertrait is that the method signatures are not necessarily compatible. @@ -130,7 +130,7 @@ fn main() { } ``` -Resolving in favor of `a` is a breaking change; in favor of `b` is not. The only other option is the status quo: not compiling. `a` simply cannot happen lest we violate backwards compatibility and the status quo is not ideal. +Resolving in favor of `a` is a breaking change; resolving in favor of `b` is not. The only other option is the status quo -- not compiling. Resolving to `a` simply cannot happen lest we violate backwards compatibility, and the status quo is not ideal. # Prior art [prior-art]: #prior-art @@ -138,6 +138,7 @@ Resolving in favor of `a` is a breaking change; in favor of `b` is not. The only ### RFC 2845 RFC 2845 was a previous attempt to address this problem, but it has several drawbacks: + - It doesn't fully address the problem since it only changes name resolution when trait methods are resolved due to generic bounds. In practice, most of the ambiguity from stabilizing `intersperse` comes from non-generic code. - It adds a lot of complexity because name resolution depends on the specific trait bounds that have been brought into scope. @@ -145,8 +146,3 @@ RFC 2845 was a previous attempt to address this problem, but it has several draw [unresolved-questions]: #unresolved-questions - Should we have a warn-by-default lint that fires at the definition-site of a subtrait that shadows a supertrait item? - -# Future possibilities -[future-possibilities]: #future-possibilities - -None From 566bbc956ce64e309f4ab4bd0a2369d53e83f664 Mon Sep 17 00:00:00 2001 From: Travis Cross Date: Fri, 3 Jan 2025 20:25:20 +0000 Subject: [PATCH 08/11] Rename RFC 3624 file and populate metadata --- ...m-shadowing-v2.md => 3624-supertrait-item-shadowing-v2.md} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename text/{0000-supertrait-item-shadowing-v2.md => 3624-supertrait-item-shadowing-v2.md} (97%) diff --git a/text/0000-supertrait-item-shadowing-v2.md b/text/3624-supertrait-item-shadowing-v2.md similarity index 97% rename from text/0000-supertrait-item-shadowing-v2.md rename to text/3624-supertrait-item-shadowing-v2.md index 3d8a295f454..0a9b30b56e8 100644 --- a/text/0000-supertrait-item-shadowing-v2.md +++ b/text/3624-supertrait-item-shadowing-v2.md @@ -1,7 +1,7 @@ - Feature Name: `supertrait_item_shadowing` - Start Date: 2024-05-04 -- RFC PR: [rust-lang/rfcs#0000](https://github.com/rust-lang/rfcs/pull/0000) -- Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000) +- RFC PR: [rust-lang/rfcs#3624](https://github.com/rust-lang/rfcs/pull/3624) +- Tracking Issue: [rust-lang/rust#89151](https://github.com/rust-lang/rust/issues/89151) # Summary [summary]: #summary From 8df733cbf376ab25be20904cbeb59f31a931abc0 Mon Sep 17 00:00:00 2001 From: Travis Cross Date: Sat, 4 Jan 2025 18:09:20 +0000 Subject: [PATCH 09/11] Remove section on type inference As was pointed out by compiler-errors, some of the details in this section are wrong. More broadly, though, specifying these details goes beyond the purpose of this RFC. These can be worked out later. --- text/3624-supertrait-item-shadowing-v2.md | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/text/3624-supertrait-item-shadowing-v2.md b/text/3624-supertrait-item-shadowing-v2.md index 0a9b30b56e8..5ad9ee972bc 100644 --- a/text/3624-supertrait-item-shadowing-v2.md +++ b/text/3624-supertrait-item-shadowing-v2.md @@ -51,25 +51,6 @@ Note that this only happens when *both* traits are in scope since this is requir We will provide an allow-by-default lint to let users opt in to being notified when an ambiguity is resolved in this way. -### Type inference - -This change happens during name resolution and specifically doesn't interact with type inference. Consider this example: - -```rust -trait Foo { fn method(&self) {} } -trait Bar: Foo { fn method(&self) {} } -impl Foo for Vec { } -impl Bar for Vec { } - -fn main() { - let x = vec![]; - x.method(); // which to call? - x.push(Box::new(22)); // oh, looks like `Foo` -} -``` - -Today that example will give an ambiguity error because `method` is provided by multiple traits in scope. With this RFC, it will instead always resolve to the subtrait method and then compilation will fail because `Vec` does not implement the `Copy` trait required by `Bar::method`. - # Drawbacks [drawbacks]: #drawbacks From dc3e2fd2e6b298cec24ec9b7b3fb78c25e246739 Mon Sep 17 00:00:00 2001 From: Travis Cross Date: Sat, 4 Jan 2025 18:12:10 +0000 Subject: [PATCH 10/11] Replace "name resolution" with "method selection" What this RFC had referred to as "name resolution" is actually method selection, as pointed out by compiler-errors, so let's fix this everywhere. In doing this, we remove one of the purported drawbacks of RFC 2845, as it makes less sense as stated after this fix. --- text/3624-supertrait-item-shadowing-v2.md | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/text/3624-supertrait-item-shadowing-v2.md b/text/3624-supertrait-item-shadowing-v2.md index 5ad9ee972bc..ad9d6b67c45 100644 --- a/text/3624-supertrait-item-shadowing-v2.md +++ b/text/3624-supertrait-item-shadowing-v2.md @@ -6,7 +6,7 @@ # Summary [summary]: #summary -When name resolution encounters an ambiguity between two trait methods when both traits are in scope, if one trait is a subtrait of the other then select that method instead of reporting an ambiguity error. +When method selection encounters an ambiguity between two trait methods when both traits are in scope, if one trait is a subtrait of the other then select that method instead of reporting an ambiguity error. # Motivation [motivation]: #motivation @@ -40,7 +40,7 @@ Attempts to stabilize `intersperse` have failed with a large number of regressio # Proposed solution [proposed-solution]: #proposed-solution -This RFC proposes to change name resolution to resolve the ambiguity in the following specific circumstances: +This RFC proposes to change method selection to resolve the ambiguity in the following specific circumstances: - All method candidates are trait methods (inherent methods are already prioritized over trait methods). - One trait is transitively a subtrait of all other traits in the candidate list. @@ -63,9 +63,9 @@ If we choose not to accept this RFC then there doesn't seem to be a reasonable p ## Only doing this for specific traits -One possible alternative to a general change to the name resolution rules would be to only do so on a case-by-case basis for specific methods in standard library traits. This could be done by using a perma-unstable `#[shadowable]` attribute specifically on methods like `Iterator::intersperse`. +One possible alternative to a general change to the method selection rules would be to only do so on a case-by-case basis for specific methods in standard library traits. This could be done by using a perma-unstable `#[shadowable]` attribute specifically on methods like `Iterator::intersperse`. -There are both advantages and inconveniences to this approach. While it allows most Rust users to avoid having to think about this issue for most traits, it does make the `Iterator` trait more "magical" in that it doesn't follow the same rules as the rest of the language. Having a consistent rule for how name resolution works is easier to teach people. +There are both advantages and inconveniences to this approach. While it allows most Rust users to avoid having to think about this issue for most traits, it does make the `Iterator` trait more "magical" in that it doesn't follow the same rules as the rest of the language. Having a consistent rule for how method selection works is easier to teach people. ## Preferring the supertrait method instead @@ -118,10 +118,7 @@ Resolving in favor of `a` is a breaking change; resolving in favor of `b` is not ### RFC 2845 -RFC 2845 was a previous attempt to address this problem, but it has several drawbacks: - -- It doesn't fully address the problem since it only changes name resolution when trait methods are resolved due to generic bounds. In practice, most of the ambiguity from stabilizing `intersperse` comes from non-generic code. -- It adds a lot of complexity because name resolution depends on the specific trait bounds that have been brought into scope. +RFC 2845 was a previous attempt, but it did not fully address the problem since it only changes method selection when trait methods are resolved due to generic bounds. In practice, most of the ambiguity from stabilizing `intersperse` comes from non-generic code. # Unresolved questions [unresolved-questions]: #unresolved-questions From 9d34dc028e9ef7a6b98e78f0b1bcd087e714af51 Mon Sep 17 00:00:00 2001 From: Travis Cross Date: Sat, 4 Jan 2025 18:54:28 +0000 Subject: [PATCH 11/11] Clarify which method we're selecting Thanks to fbstj for pointing out this potential ambiguity. --- text/3624-supertrait-item-shadowing-v2.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/3624-supertrait-item-shadowing-v2.md b/text/3624-supertrait-item-shadowing-v2.md index ad9d6b67c45..b3b8cda3a16 100644 --- a/text/3624-supertrait-item-shadowing-v2.md +++ b/text/3624-supertrait-item-shadowing-v2.md @@ -6,7 +6,7 @@ # Summary [summary]: #summary -When method selection encounters an ambiguity between two trait methods when both traits are in scope, if one trait is a subtrait of the other then select that method instead of reporting an ambiguity error. +When method selection encounters an ambiguity between two trait methods when both traits are in scope, if one trait is a subtrait of the other then select the method from the subtrait instead of reporting an ambiguity error. # Motivation [motivation]: #motivation