-
-
Notifications
You must be signed in to change notification settings - Fork 669
[Implement] nameof builtin #731
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
Conversation
Sorry. It looks like I committed dist files again. My ide is not being cooperative. |
Looking good with the comments addressed :) |
src/builtins.ts
Outdated
return module.unreachable(); | ||
} | ||
|
||
if (resultType.classReference !== null) { |
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.
Checking for is(TypeFlags.REFERENCE)
first can harden this a bit, for example if i32
, that is not a reference, gets a classReference
of I32
in the future.
nit: Also seems that let value: string
above can be moved above here, and doesn't have to be nullable / require an initializer anymore due to adding the default case below.
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.
Ah! That makes perfect sense. It might make the code clearer too. It would return I32
instead of i32
as well. Great catch.
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.
if (resultType.is(TypeFlags.REFERENCE)) {
if (resultType.classReference !== null) {
value = resultType.classReference.name;
} else if (resultType.signatureReference !== null) {
value = "Function";
} else {
assert(false);
value = "";
}
} else {
switch (resultType.kind) {
Edit: I've submitted a commit to provide the full context of what I mean. I'm happy to change it per your review.
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.
Can be simplified to
if (resultType.classReference !== null) {
value = resultType.classReference.name;
} else {
assert(resultType.signatureReference);
value = "Function";
}
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.
Perfect. Will change right now.
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.
All set.
closed in favor of #739 |
This looks like it's all set. It has generic type inference, class name inference, and basic type inference to the best of my ability. Comments/feedback is welcome please!
Ready for review.