Implement macro for setting builtin functions #206

Stupremee · 2019-11-09T10:41:54Z

Description

This pull request introduces a new macro called make_fn in the builtins module,
which can be used to simplify setting a builtin function and replaces every "old" way with the new macro.

Example

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

becomes

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

and

some_prototype.set_field_slice("someFunc", (some_func as NativeFunctionData));

becomes

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

This change was requested #193

jasonwilliams · 2019-11-09T11:24:59Z

Do we want to call this macro make_builtin_fn so it’s explicit that’s what it’s for?
As we can also make dynamic functions (js code) later on.

Stupremee · 2019-11-09T11:49:16Z

Why does some functions, e.g. findIndexor find, in the array primitive doesn't set the length field?

…, number, object, regexp and string

IovoslavIovchev · 2019-11-09T14:43:00Z

@Stupremee basically, that is where the idea for this macro came from. Some functions do not set the length when they should simply because it is something that is easy to be missed during implementation.

Stupremee · 2019-11-09T14:44:16Z

Ah okay. So I should set the length field for every method that doesn't set it?

IovoslavIovchev · 2019-11-09T14:46:33Z

src/lib/builtins/mod.rs

+        $p.set_field_slice($name, $fn);
+    };
+    ($fn:ident, named $name:expr, of $p:ident) => {
+        $p.set_field_slice($name, to_value($fn as NativeFunctionData));


Are there any functions that do not have their length set (e.g. it's undefined)? If so, this line is fine.

But if not, I think it would be better if you set the length to 0 by default.

IovoslavIovchev · 2019-11-09T14:47:45Z

@Stupremee yep

IovoslavIovchev · 2019-11-10T06:37:10Z

src/lib/builtins/math.rs

+    make_builtin_fn!(acos, named "acos", with length 1, of math);
+    make_builtin_fn!(asin, named "asin", with length 1, of math);
+    make_builtin_fn!(atan, named "atan", with length 1, of math);
+    make_builtin_fn!(atan2, named "atan2", with length 1, of math);


The length should be 2.

IovoslavIovchev · 2019-11-10T06:37:52Z

src/lib/builtins/math.rs

+    make_builtin_fn!(acos, named "acos", with length 1, of math);
+    make_builtin_fn!(asin, named "asin", with length 1, of math);
+    make_builtin_fn!(atan, named "atan", with length 1, of math);
+    make_builtin_fn!(atan2, named "atan2", with length 1, of math);


Suggested change

make_builtin_fn!(atan2, named "atan2", with length 1, of math);

make_builtin_fn!(atan2, named "atan2", with length 2, of math);

IovoslavIovchev · 2019-11-10T06:46:24Z

src/lib/builtins/number.rs

-    number_prototype.set_field_slice("toPrecision", to_value(to_precision as NativeFunctionData));
-    number_prototype.set_field_slice("toString", to_value(to_string as NativeFunctionData));
-    number_prototype.set_field_slice("valueOf", to_value(value_of as NativeFunctionData));
+    make_builtin_fn!(to_exponential, named "toExponential", of number_prototype);


Suggested change

make_builtin_fn!(to_exponential, named "toExponential", of number_prototype);

make_builtin_fn!(to_exponential, named "toExponential", with length 1, of number_prototype);

IovoslavIovchev · 2019-11-10T06:48:24Z

src/lib/builtins/number.rs

+    make_builtin_fn!(to_fixed, named "toFixed", with length 1, of number_prototype);
+    make_builtin_fn!(to_locale_string, named "toLocaleString", of number_prototype);
+    make_builtin_fn!(to_precision, named "toPrecision", with length 1, of number_prototype);
+    make_builtin_fn!(to_string, named "toString", of number_prototype);


Suggested change

make_builtin_fn!(to_string, named "toString", of number_prototype);

make_builtin_fn!(to_string, named "toString", with length 1, of number_prototype);

IovoslavIovchev · 2019-11-10T07:01:54Z

src/lib/builtins/string.rs

+    make_builtin_fn!(index_of, named "indexOf", with length 1, of proto);
+    make_builtin_fn!(last_index_of, named "lastIndexOf", with length 1, of proto);
+    make_builtin_fn!(r#match, named "match", with length 1, of proto);
+    make_builtin_fn!(pad_end, named "padEnd", with length 2, of proto);


Suggested change

make_builtin_fn!(pad_end, named "padEnd", with length 2, of proto);

make_builtin_fn!(pad_end, named "padEnd", with length 1, of proto);

IovoslavIovchev · 2019-11-10T07:02:26Z

src/lib/builtins/string.rs

+    make_builtin_fn!(last_index_of, named "lastIndexOf", with length 1, of proto);
+    make_builtin_fn!(r#match, named "match", with length 1, of proto);
+    make_builtin_fn!(pad_end, named "padEnd", with length 2, of proto);
+    make_builtin_fn!(pad_start, named "padStart", with length 2, of proto);


Suggested change

make_builtin_fn!(pad_start, named "padStart", with length 2, of proto);

make_builtin_fn!(pad_start, named "padStart", with length 1, of proto);

src/lib/builtins/mod.rs

Add make_fn macro in builtins module

353f8a3

Stupremee added 3 commits November 9, 2019 13:25

Replace all array methods with the make_fn macro

b776dab

Set builtins method with make_fn for bool, console, error, json, math…

f3c0363

…, number, object, regexp and string

Rename make_fn macro to make_builtin_fn

132961b

Stupremee marked this pull request as ready for review November 9, 2019 13:04

Rename the actual macro rule to make_builtin_fn

a4bd434

IovoslavIovchev reviewed Nov 9, 2019

View reviewed changes

Stupremee added 3 commits November 9, 2019 22:08

make_builtin_fn macro without length will result to length 0

1aa80d0

Add length property for every builtin method

6554b51

Remove duplicate definition of toString method in regexp

c2d6442

IovoslavIovchev reviewed Nov 10, 2019

View reviewed changes

Add missing length attributes to builtins methods

8f64309

IovoslavIovchev reviewed Nov 11, 2019

View reviewed changes

src/lib/builtins/mod.rs Show resolved Hide resolved

IovoslavIovchev merged commit 7698873 into boa-dev:master Nov 12, 2019

IovoslavIovchev mentioned this pull request Nov 13, 2019

Create a macro for properly setting builtin member functions #193

Closed

This was referenced Nov 20, 2019

Add String.replace #190

Closed

builtinfun.length undefined #210

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement macro for setting builtin functions #206

Implement macro for setting builtin functions #206

Stupremee commented Nov 9, 2019 •

edited

Loading

jasonwilliams commented Nov 9, 2019

Stupremee commented Nov 9, 2019

IovoslavIovchev commented Nov 9, 2019

Stupremee commented Nov 9, 2019

IovoslavIovchev Nov 9, 2019

IovoslavIovchev commented Nov 9, 2019

IovoslavIovchev Nov 10, 2019

IovoslavIovchev Nov 10, 2019

IovoslavIovchev Nov 10, 2019

IovoslavIovchev Nov 10, 2019

IovoslavIovchev Nov 10, 2019

IovoslavIovchev Nov 10, 2019

	make_builtin_fn!(atan2, named "atan2", with length 1, of math);
	make_builtin_fn!(atan2, named "atan2", with length 2, of math);

	make_builtin_fn!(to_exponential, named "toExponential", of number_prototype);
	make_builtin_fn!(to_exponential, named "toExponential", with length 1, of number_prototype);

	make_builtin_fn!(to_string, named "toString", of number_prototype);
	make_builtin_fn!(to_string, named "toString", with length 1, of number_prototype);

	make_builtin_fn!(pad_end, named "padEnd", with length 2, of proto);
	make_builtin_fn!(pad_end, named "padEnd", with length 1, of proto);

	make_builtin_fn!(pad_start, named "padStart", with length 2, of proto);
	make_builtin_fn!(pad_start, named "padStart", with length 1, of proto);

Implement macro for setting builtin functions #206

Implement macro for setting builtin functions #206

Conversation

Stupremee commented Nov 9, 2019 • edited Loading

Description

Example

jasonwilliams commented Nov 9, 2019

Stupremee commented Nov 9, 2019

IovoslavIovchev commented Nov 9, 2019

Stupremee commented Nov 9, 2019

IovoslavIovchev Nov 9, 2019

Choose a reason for hiding this comment

IovoslavIovchev commented Nov 9, 2019

IovoslavIovchev Nov 10, 2019

Choose a reason for hiding this comment

IovoslavIovchev Nov 10, 2019

Choose a reason for hiding this comment

IovoslavIovchev Nov 10, 2019

Choose a reason for hiding this comment

IovoslavIovchev Nov 10, 2019

Choose a reason for hiding this comment

IovoslavIovchev Nov 10, 2019

Choose a reason for hiding this comment

IovoslavIovchev Nov 10, 2019

Choose a reason for hiding this comment

Stupremee commented Nov 9, 2019 •

edited

Loading