-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[Draft] Supertrait item resolution in subtrait impl
s
#143527
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
base: master
Are you sure you want to change the base?
[Draft] Supertrait item resolution in subtrait impl
s
#143527
Conversation
The job Click to see the possible cause of the failure (guessed by this bot)
|
impl Sub for S { | ||
//~^ ERROR: the trait bound `S: Sup` is not satisfied | ||
type A = (); | ||
//~^ ERROR: the trait bound `S: Sup` is not satisfied |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not the desired end game, yet. This is rather a demonstration that this patch allows name resolution to associate A
with that from trait Sup
correctly.
The patch is also to unblock The next steps are
|
/// The source of this trait ref | ||
/// and whether this trait ref should be regarded as | ||
/// a nominal subtrait-supertrait relation. | ||
pub source: TraitRefSource, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably don't need this in AST.
This flag is needed after HIR->Ty lowering, I guess, and instead of parser it's equally simple to calculate it at any stage before that (e.g. in AST->HIR lowering, or HIR->Ty lowering).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that PolyTraitRef
is used in several AST nodes including dyn
, where
bounds and the trait superbounds. Where would a better place be, to record whether the trait superbound has the auto impl
modifier?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about switching out parse_generic_bounds
parser in fn parse_item_trait
and we record the supertrait modifier, ie. with auto impl
or not, on the trait item level?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The auto impl
modifier is in AST, yes.
Parser generally shouldn't be too context-sensitive and use things like in_supertrait_context
, so it would typically parse auto impl
everywhere and then later passes would reject it semantically in inappropriate positions.
Otherwise we have two separate but similar grammars for bound lists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But for a prototype both ways are ok, and if we need to track in_supertrait_context
in the parser anyway for auto impl
, then we can store it into AST too.
@@ -52,15 +53,15 @@ pub enum BoundKind { | |||
|
|||
/// Super traits of a trait. | |||
/// E.g., `trait A: B` | |||
SuperTraits, | |||
SuperTraits { subtrait: LocalDefId }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LocalDefId
shouldn't be a part of AST or AST visitors.
If it's needed by some specific visitor, it's better to pass it in some other way.
leading_token: &Token, | ||
) -> PResult<'a, GenericBound> { | ||
let auto_impl = if in_supertrait_context && self.eat_keyword(exp!(Auto)) { | ||
self.expect_keyword(exp!(Impl))?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is auto impl Trait
?
I don't see it in tests.
Is it a part of the "supertrait items in subtrait impls" feature, or some other related feature?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it was not included in the the "supertrait items in subtrait impls" feature but it is in the next proposal to handle the marker trait case.
Suppose that we have a marker trait trait Marker {}
which has no associated items. The lang team discussion prefers the option to not automatically fill in an implicit impl Marker
. Rather, we will first not fill it out and let the user to explicitly do impl Marker for ..
, unless the user writes trait Subtrait: auto impl Marker
to allow the auto-impl.
@@ -580,6 +584,9 @@ struct ModuleData<'ra> { | |||
/// Used to memoize the traits in this module for faster searches through all traits in scope. | |||
traits: RefCell<Option<Box<[(Ident, NameBinding<'ra>)]>>>, | |||
|
|||
/// In case supertraits are relevant in the module, the DefIds are collected, too. | |||
supertraits: RefCell<IndexSet<DefId>>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is necessary.
Since use ChildTrait::ItemFromParent;
and similar do not work and are not supposed to work, the whole implementation can be localized to compiler\rustc_resolve\src\late.rs
.
Perhaps as two passes in late resolver.
Since if the first (regular) pass we do not always have trait Parent
late-resolved when late-resolving
trait Child: Parent { ... }
, but we may need trait Parent
late-resolved here.
So maybe the first pass can just keep a hole and some bookkeeping, so that the hole can be filled in a second pass.
r? @ghost
cc @cramertj @petrochenkov
This is pertaining to implementable trait aliases,
Deref
/Receiver
trait migration and supertrait items in subtraitimpl
s.This is a draft for the resolver and crate metadata changes. Through this patch, the resolver will pick up supertrait items, collected from the trait super-bounds and trait alias in a future extension. This information will be encoded so that the resolver can resolve names in the downstream crates.
I would like to post this draft earlier because there are already significant design changes, which may warrant collecting opinions ahead of time.
Next steps would be
hir-analysis
to collect associated items in subtrait items into implied supertraitimpl
s;