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

Support the assignment operator traits like AddAssign #173

Closed
cuviper opened this issue Mar 1, 2016 · 13 comments
Closed

Support the assignment operator traits like AddAssign #173

cuviper opened this issue Mar 1, 2016 · 13 comments

Comments

@cuviper
Copy link
Member

cuviper commented Mar 1, 2016

These have been recently stabilized in rust-lang/rust#28235. We should at least add support for these to our concrete types.

We may also want to require them in our traits where we already require the basic Add etc., although I'm not sure if that's too much to impose on outside types implementing our traits. It could instead be something new like NumAssign: Num + AddAssign + SubAssign + ...

We'll have to raise our baseline Rust version to 1.8 (once it's released) to support assignment operators, but this feature may be useful enough to finally justify moving up. That would also enable a few other changes we've had blocked on Rust 1.0.

@cuviper cuviper added this to the v0.2.0 milestone Mar 1, 2016
@bluss
Copy link
Contributor

bluss commented Mar 2, 2016

Yes supporting the assign operators would be great! Float, mostly used for f32 vs f64 could depend on them?

@cuviper
Copy link
Member Author

cuviper commented Mar 2, 2016

Yeah, I think Float and PrimInt can require this.

@bluss
Copy link
Contributor

bluss commented Mar 7, 2016

Assign traits have a problem we need to solve in libstd first I think: There's no AddAssign<&u32> for u32 implementations (and so on) yet. The by reference implementations are needed so that bigint can share the same interface.

I think it must be weighed if the op assign traits are not so important that maybe they should be added everywhere.

@jimblandy
Copy link

Would this crate accept pull requests implementing compound assignment operators for individual types (say, BigInt), or would you want to wait and land a complete set of implementations for all your concrete types at once, for consistency?

@cuviper
Copy link
Member Author

cuviper commented Apr 25, 2016

@bluss started Complex with #186, but I haven't given it much thought yet. (sorry!) I guess Ratio, BigUint, and BigInt are the only other types to consider, and then decide if we want a NumAssign.

Hiding them behind an "opassign" feature seems like a good start though, so we don't even have to raise the rustc baseline. I think it's fine to deal with each type in separate PRs.

@bluss
Copy link
Contributor

bluss commented Apr 25, 2016

Hearing that makes me cautiously optimistic. 😄

There seems to be two issues in this issue (no order)

  • Supporting OpAssign for Bigint, complex, and the other number types
  • Supporting OpAssign in the numeric traits

I don't think the traits have a nice way to use a crate feature to flip their behavior (would it need adding new traits, like FloatOpAssign or so?)

@cuviper
Copy link
Member Author

cuviper commented Feb 10, 2017

So, #263 bumped us to Rust 1.8 now...

@cuviper cuviper removed this from the v0.2.0 milestone Feb 10, 2017
@Rufflewind
Copy link

Assign traits have a problem we need to solve in libstd first I think: There's no AddAssign<&u32> for u32 implementations (and so on) yet.

Is there an issue upstream tracking this? I just ran into this today writing a generic factorial function :(

@cuviper
Copy link
Member Author

cuviper commented Apr 19, 2017

There's actually an open PR right now, rust-lang/rust#41336.

@gnzlbg
Copy link

gnzlbg commented Sep 17, 2017

Does AddAssign work for num::Float ?

@cuviper
Copy link
Member Author

cuviper commented Sep 17, 2017

Adding that constraint to Float would be a breaking change, but you should be able to use Float + NumAssign.

@gnzlbg
Copy link

gnzlbg commented Sep 17, 2017

Adding that constraint to Float would be a breaking change

I worked around this by implementing my own Float trait.

@cuviper
Copy link
Member Author

cuviper commented Dec 19, 2017

Complex and BigUint are done, and I'll leave the rest in their separate issues:

@cuviper cuviper closed this as completed Dec 19, 2017
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

No branches or pull requests

5 participants