-
Notifications
You must be signed in to change notification settings - Fork 182
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
Implement GATs, step 1 #118
Conversation
chalk-parse/src/parser.lalrpop
Outdated
parameter_kinds: p | ||
"type" <name:Id> <p:Angle<ParameterKind>> <b:(":" <Id> <Angle<Parameter>>)?> | ||
<w:QuantifiedWhereClauses> ";" => | ||
{ |
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.
Ok so currently we do not handle at all the +
separator which expresses multiple bounds as in where T: Clone + Sized
. This is not a problem for e.g. where clauses on traits because you can still write where T: Clone, T: Sized
but for associated types since there is no other way to express these multiple bounds, we need to be able to parse them with the +
acting as a separator.
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.
done
chalk-parse/src/ast.rs
Outdated
@@ -54,6 +54,8 @@ pub struct TraitFlags { | |||
pub struct AssocTyDefn { | |||
pub name: Identifier, | |||
pub parameter_kinds: Vec<ParameterKind>, | |||
pub bound: Option<TraitRef>, |
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 would need multiple bounds there (see below), and using TraitRef
is too rigid as we cannot parse things like type Foo<T>: Iterator<Item = T>
.
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'm thinking this would maybe need some slight refactoring for how we handle where clauses in the grammar. Something which may work would be to split existing where clauses like TraitRef
into two parts: a left part (which would be T
in T: Foo<K>
) and a right part (which would be Foo<K>
), separated by the :
symbol. This would prevent code duplication I think because in the case of associated types, you would only ask for the right part and then build the whole TraitRef
by hand as you did. It would be especially useful when we add lifetime requirements on types as well.
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.
Yep, what do you think of something like TraitBound
and ProjectionEqBound
that I added to ast? (Currently they're only being used for GATs.)
These can contain methods to convert them to TraitRef
, WhereClause::ProjectionEq
, etc. if the proper information is obtained.
chalk-parse/src/ast.rs
Outdated
@@ -177,6 +177,11 @@ impl PolarizedTraitRef { | |||
} | |||
} | |||
|
|||
pub struct TraitBound { |
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 think it would be useful to add a comment here explaining what this represents and how it is used.
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.
done!
chalk-parse/src/ast.rs
Outdated
@@ -68,6 +68,23 @@ pub enum Parameter { | |||
Lifetime(Lifetime), | |||
} | |||
|
|||
pub struct TraitBound { |
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.
Maybe this should not be called TraitBound
? I would think something like TypeParameterBound
This adds parsing rules for bounds and where clauses on associated type definitions for #116.
It also removes where clauses on associated type values.
The way I handled bounds directly in the parser felt a bit messy, open to suggestions. Not sure if using the associated type name directly in the
TraitRef
is going to work downstream.