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

fix: BigInt exponentiation transpiler error (@dfinity/candid) #599

Merged
merged 3 commits into from Jul 28, 2022

Conversation

rvanasa
Copy link
Contributor

@rvanasa rvanasa commented Jul 25, 2022

Description

Fixes a bug encountered in browser environments such as Motoko Playground where BigInt(a) ** BigInt(b) transpiles to Math.pow(BigInt(a), BigInt(b)) for compatibility with older browsers (resulting in a runtime error). This PR adds iexp2 and ilog2 implementations which account for BigInt values and provide clearer error messages for invalid input.

@chenyan-dfinity

How Has This Been Tested?

  • Full repository test suite
  • Additional test cases for the iexp2 and ilog2 functions
  • Confirmed to fix the issue in Motoko Playground

Checklist:

  • My changes follow the guidelines in CONTRIBUTING.md.
  • The title of this PR complies with Conventional Commits.
  • I have edited the CHANGELOG accordingly. (N/A, worked as expected in 0.11.x)
  • I have made corresponding changes to the documentation. (N/A)

@rvanasa rvanasa requested a review from a team as a code owner July 25, 2022 19:33
Copy link
Contributor

@chenyan-dfinity chenyan-dfinity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@krpeacock krpeacock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see - this works because we don't have any mandatory exponentiation other than 2, and don't need to support generalizing inside of the lib. Clever!

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.

None yet

3 participants