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.
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.
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.
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.
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.
Perfect. Will change right now.
|
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.