Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

BREAKING CHANGE: Unify Math & Mathf. Make every method in Math polymorphic#2335

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
MaxGraey wants to merge14 commits intoAssemblyScript:main
base:main
Choose a base branch
Loading
fromMaxGraey:poly-math

Conversation

MaxGraey
Copy link
Member

@MaxGraeyMaxGraey commentedJun 24, 2022
edited
Loading

In this PR we're doing several breaking changes:

  • move all detailed implementations tostd/util/math
  • make all Math's methods polymorphic
  • remove non-standardNativeMath#scalebn
  • remove non-standardNativeMath#rem
  • remove non-standardNativeMath#mod (but not tests)
  • removeNativeMathf completely
  • for now, we have only singleMath.random (forf64 type)

Caveats:

  1. I have observed quite often the script that users from JS for example:
functionfoo(x:i32,y:i32):i32{letres=Math.floor(x/y)asi32;returnres;}

Before this PR this will create unnecessary cast to f64,Math.floor and downcast toi32. With this PR this code now equivalent tox / y without all this unnecessary work due toMath.floor,Math.trunc and etc accept integers and pass through it "as is".

  1. Now following code will generate a compilation error:
constcos3=Math.cos(3);// -> ERROR: Math.cos accept only f32 or f64 types// Solution: it may be fixed as Math.cos(3.0)constx:i32=123;constsqr=Math.sqrt(x);// -> ERROR: Math.sqrt accept only f32 or f64 types// Solution: Math.sqrt(x as f64)

This is the most painful caveat, which can lead to many breaking changes in some code.
Perhaps we could add new generic builtin helper which forces parameter type to float. Something like:

// builtin will be equivalent to this:typeOnlyFloat<T>=Textendsf32|f64 ?T :f64functionsqrt<Textendsi32|i64|f32|f64>(x:T):OnlyFloat<T>{if(isInteger<T>()){returnbuiltin_sqrt<f64>(xasf64)asOnlyFloat<T>;}if(isFloat<T>()){returnbuiltin_sqrt<T>(x)asOnlyFloat<T>;}}// now we can use integer inputs which will forced to f64 for outputs but safely preserve f32constsqrt3=sqrt(3);// infer as sqrt<i32>(x) -> f64

Thoughts?

  • I've read the contributing guidelines
  • I've added my name and email to the NOTICE file

@MaxGraeyMaxGraey marked this pull request as ready for reviewJune 25, 2022 09:01
@MaxGraeyMaxGraey requested a review fromdcodeIOJune 25, 2022 09:01
@dcodeIO
Copy link
Member

dcodeIO commentedJun 25, 2022
edited
Loading

So far,Math is quite intentionally identical to JSMath, also because the implementations can be switched with--use. Making one generic and the other not seems problematic in this regard. Similarly, this might break portable code relying on both being identical. The mentioned caveat that integer literals will be inferred asi32, which sometimes might either not work or produce unexpected results in the absence of a type parameter, seems quite unfortunate as well. Are you sure this is actually viable?

@MaxGraey
Copy link
MemberAuthor

MaxGraey commentedJun 25, 2022
edited
Loading

abs, floor, ceil, round etc will be work properly with integers even in JSMath due toi32 -> f64 -> i32 is invariant. Butu32 -> f64 -> u32 ori64 -> f64 -> i64 is not invariant, so this may be a problem. However, I don't think ppl will use that functions for such types. Also, I'm going to add some compiler warnings for integer paths for according methos.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@dcodeIOdcodeIOAwaiting requested review from dcodeIO

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@MaxGraey@dcodeIO

[8]ページ先頭

©2009-2025 Movatter.jp