-
-
Notifications
You must be signed in to change notification settings - Fork 418
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
Fix type parameters not being visible to a lambda type in a type alias #1633
Conversation
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.
Great work! This seems right to me.
I just added a few small style consistency comments.
test/libponyc/verify.cc
Outdated
|
||
TEST_F(VerifyTest, LambdaTypeGenericAliased) | ||
{ | ||
const char *good = |
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.
To be consistent with the other tests, could you use src
as the name of the variable that holds the string for this source code, rather than good
?
test/libponyc/verify.cc
Outdated
TEST_F(VerifyTest, LambdaTypeGenericAliased) | ||
{ | ||
const char *good = | ||
"type Foo[T] is { (T): 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.
To be consistent with the lambda type style used in other tests, could you remove the "extra" spaces inside the curly braces. That is, it would be consistent if it looked like: {(T): 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.
Ok, I have updated the commit.
Nice! |
Currently the following code does not compile:
The compiler gives the error:
This PR allows the lambda type to find type parameters from an enclosing type alias, as well as class, actor, etc.
I don't think this should affect anything else, since the only other use of the
collect_type_params()
function is for sugaring object literals, which aren't allowed in a type alias. Please let me know if this change will have wider implications.