Fix support for recursive constraints in iftype conditions. #1961
Conversation
src/libponyc/ast/token.h
Outdated
| @@ -147,6 +147,7 @@ typedef enum token_id | |||
| TK_IF, | |||
| TK_IFDEF, | |||
| TK_IFTYPE, | |||
| TK_IFTYPE_CLAUSE, | |||
There was a problem hiding this comment.
How would you feel about calling this TK_IFTYPE_COND instead of TK_IFTYPE_CLAUSE?
At least In the ponyc source code, we often use clause to refer to the body of a conditional, and cond to refer to the condition itself. Here's at least one example of this precedent.
There was a problem hiding this comment.
I don't really like the name TK_IFTYPE_CLAUSE either, but I don't think TK_IFTYPE_COND is much better. This node contains both the condition and the clause.
I also had TK_IFTYPE_BRANCH in mind, which might be better. I'm open to other suggestions
There was a problem hiding this comment.
In that case, I think I might go the other way with it, and consider this construct to be the TK_IFTYPE, and consider the outer construct to be a group of them; maybe a TK_IFTYPE_SET.
There was a problem hiding this comment.
Yeah, that makes sense.
|
This is marked "DO NOT MERGE" until the aforementioned name change is done, which @plietar will get to after he wraps up some other responsibilities. |
iftype is implemented by shadowing the type parameter with a new definition and a new constraint. Storing the pointer of the original definition allows us to compare identity of type parameters easily. This replaces the more complicated subtyping-based check to determine whether a type parameter should be reified.
This creates a new scope with both the constraint and the body of the iftype, but without the else clause.
By defining the shadowing TK_TYPEPARAM on the entire iftype clause, which contains both the constraint and the body, the appropriate definition of the type variable is used when it appears in the constraint. Fixes ponylang#1960
|
This should be good now @jemc |
jemc
added
the
changelog - fixed
|
Thanks! |
By defining the shadowing TK_TYPEPARAM on the entire iftype clause, which
contains both the constraint and the body, the appropriate definition
of the type variable is used when it appears in the constraint.
Fixes #1960
Iftype is implemented by creating a new definition of the typeparameter with a constraint which combines both the original constraint and the RHS of the iftype. This TK_TYPEPARAM shadows the original definition inside the iftype body.
The third commit moves the shadowing constraint from the body to the entire iftype, so it gets picked up by the constraint as well. This allows recursive constraints such as the examples in #1960.
However we don't want it to get used by the else clause of the iftype. The second commit creates a new AST node, TK_IFTYPE_CLAUSE, containing the constraint and the body. This isolates the various branches of the iftypes in individual scopes.
Finally the reification used subtyping of constraints to compare if a typeparam was shadowing the other one. I ran into issues with this since the two constraints are defined in separate scopes now.
Instead, the first commit changes it to have the ast_data always point to the original typeparam.
This way, even if a typeparam is an iftype shadowing of another, their data pointers will be equal.
I believe this matches @sylvanc's original intention in cb5c611.
cc @jemc for #1960. This should allow my examples with recursive constraints in that issue
cc @Praetonus who implemented iftypes