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

Implement the rest of Math methods #524

Closed
6 tasks done
HalidOdat opened this issue Jun 24, 2020 · 9 comments · Fixed by #525
Closed
6 tasks done

Implement the rest of Math methods #524

HalidOdat opened this issue Jun 24, 2020 · 9 comments · Fixed by #525
Assignees
Labels
builtins PRs and Issues related to builtins/intrinsics enhancement New feature or request good first issue Good for newcomers
Milestone

Comments

@HalidOdat
Copy link
Member

HalidOdat commented Jun 24, 2020

Implement Math methods:

  • Math.clz32()
  • Math.expm1()
  • Math.fround()
  • Math.hypot()
  • Math.log1p()
  • Math.imul()
@HalidOdat HalidOdat added enhancement New feature or request good first issue Good for newcomers builtins PRs and Issues related to builtins/intrinsics labels Jun 24, 2020
@mr-rodgers
Copy link
Contributor

Hey, I'm new, but I would like to take these.

@HalidOdat
Copy link
Member Author

HalidOdat commented Jun 24, 2020

Hey, I'm new, but I would like to take these.

Sure. go for it! If you have any questions or need any assistance just ask :)

@HalidOdat
Copy link
Member Author

@mr-rodgers a good example of a Math method is abs: https://github.com/boa-dev/boa/blob/master/boa/src/builtins/math/mod.rs#L43-L45

after we create the methods we need to register them here like abs: https://github.com/boa-dev/boa/blob/master/boa/src/builtins/math/mod.rs#L478

@pedropaulosuzuki
Copy link
Contributor

I think we missed Math.imul(a: f64, b:f64) -> i32;

@HalidOdat
Copy link
Member Author

I think we missed Math.imul(a: f64, b:f64) -> i32;

Good catch! I'll add it to the list :)

@mr-rodgers
Copy link
Contributor

Do we have a guide for writing tests?

@HalidOdat
Copy link
Member Author

HalidOdat commented Jun 24, 2020

Do we have a guide for writing tests?

Not really. we really should have one, but for now here is an example of how we test abs: https://github.com/boa-dev/boa/blob/master/boa/src/builtins/math/tests.rs#L6-L22

Also the Math tests should go in this file

Hope this helps.

mr-rodgers added a commit to mr-rodgers/boa that referenced this issue Jun 24, 2020
@Razican Razican linked a pull request Jun 25, 2020 that will close this issue
6 tasks
@jasonwilliams jasonwilliams added this to the v0.10.0 milestone Jun 25, 2020
mr-rodgers added a commit to mr-rodgers/boa that referenced this issue Jun 25, 2020
mr-rodgers added a commit to mr-rodgers/boa that referenced this issue Jun 25, 2020
mr-rodgers added a commit to mr-rodgers/boa that referenced this issue Jun 25, 2020
mr-rodgers added a commit to mr-rodgers/boa that referenced this issue Jun 25, 2020
mr-rodgers added a commit to mr-rodgers/boa that referenced this issue Jun 25, 2020
mr-rodgers added a commit to mr-rodgers/boa that referenced this issue Jun 25, 2020
mr-rodgers added a commit to mr-rodgers/boa that referenced this issue Jun 26, 2020
@mr-rodgers
Copy link
Contributor

Do we have a guide for writing tests?

Not really. we really should have one [...]

Maybe this is something I can help with?

@HalidOdat
Copy link
Member Author

Maybe this is something I can help with?

Yes. It would be awesome to have it for newcomers to the project :)

HalidOdat pushed a commit that referenced this issue Jul 2, 2020
* add `Math.clz32` method (#524)

* fix doc urls for clz32

* [#524] optimize impl for `Math.clz32`

* [#524] add implementation for `Math.expm1()`

* [#524] add implementation for `Math.fround()`

* [#524] implement `Math.hypot()`

* [#524] implement `Math.log1p()`

* [#524] implement `Math.imul()`

* improve `Math.clz32()` implementation

* [#524] add tests for more states
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builtins PRs and Issues related to builtins/intrinsics enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants