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

Add `quoted.Liftable[BigInt]` and `quoted.Liftable[BigDecimal]` to the stdlib #6944

Merged
merged 5 commits into from Jul 27, 2019

Conversation

@nicolasstucki
Copy link
Contributor

commented Jul 27, 2019

No description provided.

@nicolasstucki nicolasstucki changed the title Add `quoted.Liftable[BigInt]` to the stdlib Add `quoted.Liftable[BigInt]` and `quoted.Liftable[BigDecimal]` to the stdlib Jul 27, 2019

@nicolasstucki nicolasstucki requested a review from liufengyun Jul 27, 2019

@liufengyun
Copy link
Contributor

left a comment

Otherwise, LGTM

library/src/scala/quoted/Liftable.scala Show resolved Hide resolved
@plokhotnyuk

This comment has been minimized.

Copy link

commented Jul 27, 2019

@nicolasstucki is it can be done only through string representation?

Have you considered to use Array[Byte] for BigInt and (Array[Byte] /* mantissa */, Int /* scale */, java.math.MathContext /* mc */) for BigDecimal instead?

Conversion through the string can be inefficient for really big numbers or lead for some unexpected rounding...

@nicolasstucki

This comment has been minimized.

Copy link
Contributor Author

commented Jul 27, 2019

I did consider the Array[Byte] variant, but I left it as an optimization for later as I still do not have the liftable for Array[Byte]. But I can add the generic version and add the specialised one afterwards.

@nicolasstucki nicolasstucki force-pushed the dotty-staging:add-bignumbers-liftable branch from 5151604 to 89ad39b Jul 27, 2019

@nicolasstucki nicolasstucki requested a review from liufengyun Jul 27, 2019

Implementation changed

@liufengyun
Copy link
Contributor

left a comment

LGTM

@nicolasstucki nicolasstucki merged commit 3181374 into lampepfl:master Jul 27, 2019

2 checks passed

CLA User signed CLA
Details
continuous-integration/drone/pr Build is passing
Details

@nicolasstucki nicolasstucki deleted the dotty-staging:add-bignumbers-liftable branch Jul 27, 2019

@anatoliykmetyuk anatoliykmetyuk added this to the 0.18 Tech Preview milestone Aug 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.