Skip to content
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

Fixed stdlib builtins not working with typedoc #1894

Merged
merged 3 commits into from
Jun 9, 2021

Conversation

torch2424
Copy link
Contributor

Hello!

This is just something I noticed when working on the Fastly AS SDK.

It seems like in version 0.18.13, we introduced a change in which caused a type error, as we try to assign void to an i32, and i31ref:

Screenshot from 2021-06-08 17-19-44

I got the error above when running typedoc on version 0.19.1, but it works again on 0.18.12. So, I assume unreachable is the instruction that causes things to crash. So instead of returning void, I still call the function, but return a "void number" (0), which I assume is the expected behavior?

With this fix, things start working again in the type system:

Screenshot from 2021-06-08 17-26-09

Thanks to whoever picks up this review! 😄 🎉

cc @dcodeIO @MaxGraey 😄

@torch2424 torch2424 self-assigned this Jun 9, 2021
Copy link
Member

@MaxGraey MaxGraey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@dcodeIO
Copy link
Member

dcodeIO commented Jun 9, 2021

Hmm, interesting. Unreachable is currently typed as

declare function unreachable(): never;

and in AS itself is an expression of any type. I guess the following doesn't work either?

static new(value: i32): i31ref { return <i31ref>unreachable(); }

static get(i31expr: i31ref): i32 { return <i32>unreachable(); }

Ah, I see now. In this particular file, it picks up the AS-internal builtin definition of unreachable, which is

// @ts-ignore: decorator
@unsafe @builtin
export declare function unreachable(): void;

Changing the void there to auto gets rid of one of the errors, and the other can be worked around as well:

static new(value: i32): i31ref { return changetype<i31ref>(unreachable()); }

static get(i31expr: i31ref): i32 { return unreachable(); }

Wondering if that would be a preferable fix?

(other than that it's kinda hilarious where just naming an instruction i31.new (new is a keyword) leads here, since if it would be named i31.create instead, we could just use a proper namespace, heh - we could propose renaming the instruction, as I would assume similar workarounds, if possible at all, are necessary in other languages as well)

@torch2424
Copy link
Contributor Author

Thank you for the review @MaxGraey ! 😄

And I went ahead and made your requested changes @dcodeIO ! 😄 It totally worked for me, thank you! 😄

Screenshot from 2021-06-09 10-42-51

@torch2424
Copy link
Contributor Author

Thanks for the review y'all @dcodeIO @MaxGraey ! Merging! 😄

@torch2424 torch2424 merged commit 7ada77c into main Jun 9, 2021
@torch2424 torch2424 deleted the torch2424/builtin-types-with-typedoc branch June 9, 2021 22:04
torch2424 added a commit that referenced this pull request Jun 16, 2021
torch2424 added a commit that referenced this pull request Jun 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants