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

Create a macro for properly setting builtin member functions #193

Closed
IovoslavIovchev opened this issue Oct 27, 2019 · 10 comments
Closed

Create a macro for properly setting builtin member functions #193

IovoslavIovchev opened this issue Oct 27, 2019 · 10 comments
Assignees
Labels
good first issue Good for newcomers

Comments

@IovoslavIovchev
Copy link
Contributor

IovoslavIovchev commented Oct 27, 2019

When creating a new member function for some builtin, you need to make sure the create_constructor() function contains this:

let some_func = to_value(func as NativeFunctionData);
some_func.set_field_slice("length", to_value(1_i32));

// .. and then
some_prototype.set_field_slice("someFunc", some_func); 

Writing these two lines for each new function can prove tedious and error prone, as the second line can be missed when needed (functions have their length property reflect the number of arguments they accept).

What I propose is the following macro:

make_fn!(some_func, named "someFunc", with length 1, of some_prototype);

which will essentially contain the 3 aforementioned boilerplate lines.

What I am saying should definitely not be set in stone. It is only supposed to start a discussion on the topic.

@jasonwilliams
Copy link
Member

jasonwilliams commented Oct 30, 2019

This is a good idea, although i wonder if it will overlap Function Objects

Would this macro only be used for built-in functions? Or would you see it being used for functions we come across in code?

@IovoslavIovchev
Copy link
Contributor Author

IovoslavIovchev commented Oct 30, 2019

Depends on what you mean by "functions we come across in code".
My intention is for it to only be used for reducing boilerplate in buil-tins.

It is a more humble version of CreateBuiltinFunction (of which I found no implementation, apologies if I missed anything) but for properties, rather than internal slots.
Of course, we could expand the scope a bit here and fully implement CreateBuiltinFunction.

@IovoslavIovchev IovoslavIovchev mentioned this issue Oct 31, 2019
4 tasks
@jasonwilliams
Copy link
Member

I'm happy to push ahead with this, we can always change it as need be.
I think itl prove useful

@IovoslavIovchev
Copy link
Contributor Author

Great. I believe it will be a good first issue.

Will mark it as such, if anyone is interested.

@IovoslavIovchev IovoslavIovchev added the good first issue Good for newcomers label Nov 2, 2019
@Stupremee
Copy link
Contributor

I can take care of this

@Stupremee
Copy link
Contributor

This issue can be closed I guess

@IovoslavIovchev
Copy link
Contributor Author

Done as part of #206

@thiagoarrais
Copy link

I'm getting undefined when I do 'abc'.matchAll.length in tests.js. Even after upgrading to 7698873. Am I doing something wrong?

@Stupremee
Copy link
Contributor

I think a new issue for your issue would be better @thiagoarrais

@thiagoarrais
Copy link

Sure!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants