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 TryFrom impls #123

Merged
merged 8 commits into from
Mar 14, 2020
Merged

Add TryFrom impls #123

merged 8 commits into from
Mar 14, 2020

Conversation

milesand
Copy link
Contributor

@milesand milesand commented Jan 2, 2020

Closes #120

Currently contains:

  • TryFrom translations of existing conversion traits.
  • A minimal Error type used for all TryFrom implementations.

Additional implemetations (e.g. TryFrom<BigInt> for BigUint which skips clone()) could be beneficial. Also, no new tests have been added since all new impls are basically thin adapters around existing, tested methods. It may be beneficial to add them regardless.

@cuviper
Copy link
Member

cuviper commented Mar 9, 2020

FWIW, I rebased this to resolve the conflicts. It looks good!

Additional implementations (e.g. TryFrom<BigInt> for BigUint which skips clone()) could be beneficial.

I'm still thinking about this before merging, because it would be a minor breaking change to add it later. For example, given x: BigInt, x.try_into::<BigUint>() will auto-ref right now, but will consume x if we add the additional implementations later. This could easily break code that still needed x.

I'm somewhat inclined to add by-value TryFrom<BigInt> and TryFrom<BigUint> in all cases, even going into primitive integers, so they are useful in generic contexts like T: TryInto<u32> without forcing references.

src/lib.rs Outdated Show resolved Hide resolved
@milesand
Copy link
Contributor Author

milesand commented Mar 10, 2020

I'm somewhat inclined to add by-value TryFrom<BigInt> and TryFrom<BigUint> in all cases

I pushed a relevant commit; This should cover most, if not all, by-value fallible conversion between primitives, BigInt and BigUint.

@cuviper
Copy link
Member

cuviper commented Mar 13, 2020

I added a couple tweaks for simplification, but otherwise it looks good, thanks!

bors r+

@bors
Copy link
Contributor

bors bot commented Mar 14, 2020

Build succeeded

@bors bors bot merged commit c076bb8 into rust-num:master Mar 14, 2020
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.

Implement TryFrom/TryInto
2 participants