Skip to content

Commit b32c37e

Browse files
committed
Supertrait item shadowing v2
1 parent 0f62578 commit b32c37e

File tree

1 file changed

+77
-0
lines changed

1 file changed

+77
-0
lines changed
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
- Feature Name: `supertrait_item_shadowing`
2+
- Start Date: 2024-05-04
3+
- RFC PR: [rust-lang/rfcs#0000](https://github.com/rust-lang/rfcs/pull/0000)
4+
- Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000)
5+
6+
# Summary
7+
[summary]: #summary
8+
9+
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.
10+
11+
# Motivation
12+
[motivation]: #motivation
13+
14+
15+
The libs-api team would like to stabilize `Iterator::intersperse` but has a problem. The `itertools` crate already has:
16+
17+
```rust
18+
// itertools
19+
trait Itertools: Iterator {
20+
fn intersperse(self, element: Self::Item) -> Intersperse<Self>;
21+
}
22+
```
23+
24+
This method is used in crates with code similar to the following:
25+
26+
```rust
27+
use core::iter::Iterator; // Implicit import from prelude
28+
29+
use itertools::Itertools as _;
30+
31+
fn foo() -> impl Iterator<Item = &'static str> {
32+
"1,2,3".split(",").intersperse("|")
33+
// ^ This is ambiguious: it could refer to Iterator::intersperse or Itertools::intersperse
34+
}
35+
```
36+
37+
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.
38+
39+
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.
40+
41+
# Proposed solution
42+
[proposed-solution]: #proposed-solution
43+
44+
This RFC proposes to change name resolution to resolve the ambiguity in the following specific circumstances:
45+
- All method candidates are trait methods. (Inherent methods are already prioritized over trait methods)
46+
- One trait is transitively a sub-trait of all other traits in the candidate list.
47+
48+
When this happens, the sub-trait method is selected instead of reporting an ambiguity error.
49+
50+
# Drawbacks
51+
[drawbacks]: #drawbacks
52+
53+
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.
54+
55+
# Rationale and alternatives
56+
[rationale-and-alternatives]: #rationale-and-alternatives
57+
58+
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.
59+
60+
# Prior art
61+
[prior-art]: #prior-art
62+
63+
### RFC 2845
64+
65+
RFC 2845 was a previous attempt to address this problem, but it has several drawbacks:
66+
- 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.
67+
- It adds a lot of complexity because name resolution depends on the specific trait bounds that have been brought into scope.
68+
69+
# Unresolved questions
70+
[unresolved-questions]: #unresolved-questions
71+
72+
None
73+
74+
# Future possibilities
75+
[future-possibilities]: #future-possibilities
76+
77+
None

0 commit comments

Comments
 (0)