Skip to content

[Implement] Lots of useful Builtins #739

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

Merged
merged 30 commits into from
Aug 13, 2019
Merged

Conversation

jtenner
Copy link
Contributor

@jtenner jtenner commented Jul 30, 2019

Instead of a bunch of tiny builtin requests, I decided this was a nice subset of the proposed ones to merge immediately pending review. I'm sure there's lots of little things we could make better, but it's my intention to get this done by the end of the week so I can get back to work on EventEmitter.

Features:

  • isVoid<T>()
  • ParameterCount<T>(func?: T): i32 lengthof<T extends Function>(func?: T)
  • nameof<T>(value?: T): string
  • idof<T>(): i32
  • ReturnType<T>
  • Signature#equals(value: Signature): bool

@dcodeIO
Copy link
Member

dcodeIO commented Jul 30, 2019

Continuing after #735 (comment)

Going with our own naming scheme and aliasing those types TS has as well sounds good. So, when we go down that route, the uppercase namings would be aliases while our naming scheme would be all lower case to match what we have already (e.g. indexof<T>, sizeof<T>). As such, the following would be lowercase

  • parameterCount<T?>(fn?: T) (maybe lengthof<T>?)
  • returnof<T>, but aliased as ReturnType<T>

so there's no arbitrary mixing in the original implementations, with just the aliases deviating.

@MaxGraey
Copy link
Member

parameterCount -> argumentsCount wdyt? Because JS has arguments.lenght

@jtenner
Copy link
Contributor Author

jtenner commented Jul 30, 2019

@MaxGraey I would love to figure out how to implement arguments.length and add a backing class Function to get func.call() working. We can probably do this on a later date in a separate pull request that will break a lot of things.

@dcodeIO I disagree with parameterCount() naming. We should remove it in favor of lengthof<T>() completely as an exclusive assemblyscript builtin. Shorter and sweeter.

I doctored the EventEmitter pull request to showcase how they might be used here: https://github.com/assemblyscript/node

Check out the pull request and use this as context.

I'll also get around to implementing the aliases soon. Thanks for the input.

@jtenner
Copy link
Contributor Author

jtenner commented Jul 31, 2019

@dcodeIO Should be good now for lengthof<T extends Function>(func?: T)

Please help me clean up this pull request and get it sorted soon! Thanks.

@jtenner
Copy link
Contributor Author

jtenner commented Aug 1, 2019

Can you help me enumerate the rest of the aliases too? I'm pretty sure I'm missing a few.

@jtenner
Copy link
Contributor Author

jtenner commented Aug 2, 2019

Alright. I've tried my best to cover the latest round of nitpicks. Anything else?

@jtenner
Copy link
Contributor Author

jtenner commented Aug 5, 2019

@dcodeIO A few people I talked to might like a deref(ref: any): usize and ref<T>(ptr: usize): T built-in, including myself. Typing changetype<usize>(value) all the time doesn't feel really ergonomic :). Could we sneak those functions in?

@dcodeIO
Copy link
Member

dcodeIO commented Aug 10, 2019

Seems tests/compiler/ReturnType-error.untouched.wat became created somehow, even though the test is guaranteed to error hence should not yield a fixture. Looking good to me otherwise!

Regarding ref/deref: I remember vaguely that I had been thinking about different names when introducing changetype, ultimately picking changetype because everything else can be implemented using it, like ref/deref in this case.

@dcodeIO dcodeIO merged commit a7b9244 into AssemblyScript:master Aug 13, 2019
@dcodeIO
Copy link
Member

dcodeIO commented Aug 13, 2019

Thanks! :)

@jtenner
Copy link
Contributor Author

jtenner commented Aug 13, 2019

Keep in mind that the dist files need to be updated before the builtins can be used.

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