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

Tracking Issue for RFC: Supertrait Item Shadowing #89151

Open
5 tasks
pnkfelix opened this issue Sep 21, 2021 · 6 comments
Open
5 tasks

Tracking Issue for RFC: Supertrait Item Shadowing #89151

pnkfelix opened this issue Sep 21, 2021 · 6 comments
Labels
C-tracking-issue T-lang

Comments

@pnkfelix
Copy link
Member

@pnkfelix pnkfelix commented Sep 21, 2021

This is a tracking issue for the RFC "Supertrait Item Shadowing" (rust-lang/rfcs#2845).
The feature gate for the issue is #![feature(supertrait_item_shadowing)].

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Steps

Unresolved Questions

  • Terminology
  • Further fragile base class situations

Implementation history

@pnkfelix pnkfelix added the C-tracking-issue label Sep 21, 2021
@pnkfelix pnkfelix changed the title Tracking Issue for RFC Tracking Issue for RFC: Supertrait Item Shadowing Sep 21, 2021
@dtolnay dtolnay added the T-lang label Oct 27, 2021
@dtolnay
Copy link
Member

@dtolnay dtolnay commented Oct 27, 2021

@rust-lang/libs-api — this change is relevant to our ability to add things like Iterator::intersperse which used to introduce ambiguities against Itertools::intersperse. After this language change it would no longer be ambiguous because trait Itertools: Iterator {...}. Thus downstream uses of .intersperse(...) would resolve to the Itertools one since that's the subtrait.

@the8472
Copy link
Contributor

@the8472 the8472 commented Oct 27, 2021

For the Iterator/Itertools case it would be necessary to shadow based on the explicit import of Itertools vs. the implicit import via the prelude.

Currently the RFC only concerns itself with traits being brought into scope via generics or trait objects, not via the prelude.

To resolve this issue, this RFC proposes the following: If the user does not explicitly bring the supertrait into scope themselves, the subtrait should "shadow" the supertrait, resolving the current ambiguity in favor of the subtrait.
[...]
When using a trait as a bound in generics, or using a trait object, a trait with a supertrait will only bring the supertrait's items into scope if it does not define an item with the same name itself.

So the scope of this issue would have to be widened a bit.

@lcdr
Copy link

@lcdr lcdr commented Oct 28, 2021

This is correct, the RFC explicitly only concerns itself with generics and trait objects.
Note however that glob imports have long been known to have the potential to induce breakage: the Semver and API Evolution RFC points out that glob imports can effectively make any addition of public items a breaking change. It then argues that since a glob import could always have been written as a list of explicit imports, it is considered an acceptable minor change to add new items. Interestingly enough, the RFC suggests glob shadowing as a solution to this issue. However as far as I'm aware there hasn't been a glob shadowing RFC actually submitted.

Similar concerns to what the RFC covers however also apply to the case where both supertrait and subtrait are used, without glob imports being involved at all. Unfortunately in this particular case it's a much more difficult decision whether to shadow or not to shadow - this RFC argued that in the case of generics and trait objects, Rust's implicit bringing into scope of supertrait items was counterintuitive to a user who just imported the subtrait, without even possibly knowing of the supertrait. However, in this case, the user would have consciously imported both traits. It could be argued that the better choice would be to refuse to automatically shadow and force the user to disambiguate, or, on the other hand, that it's better to keep behavior consistent with the subtrait-shadows-supertrait rule in the RFC. It'd be interesting to hear T-lang's opinions on this issue.

@the8472
Copy link
Contributor

@the8472 the8472 commented Oct 28, 2021

Well, the std prelude (which is what causes #88967) isn't an explicit glob import either, it's fully implicit.

@lcdr
Copy link

@lcdr lcdr commented Oct 29, 2021

Ah, I thought you were talking about a prelude in Itertools.

@petrochenkov
Copy link
Contributor

@petrochenkov petrochenkov commented Nov 3, 2021

I remember looking at the method resolution logic (excluding auto(de)ref) in the compiler a couple of years ago, and not liking what I've seen.

Treating methods from generic parameter bounds as inherent (bad idea, IMO), treating Trait methods on dyn Trait as inherent (bad idea, IMO), not considering Trait methods on impl Trait at all (bad idea, IMO, #41221), filtering of private methods (needs to have some priority relative to supertrait item shadowing), filtering of unstable methods (needs to have some priority relative to supertrait item shadowing).

I wish that area got some principled common vision (and corresponding bugfixing) before new features are added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue T-lang
Projects
None yet
Development

No branches or pull requests

5 participants