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

Arithmetic operations with reference semantics #35

Open
cohomology opened this issue Oct 12, 2018 · 2 comments
Open

Arithmetic operations with reference semantics #35

cohomology opened this issue Oct 12, 2018 · 2 comments

Comments

@cohomology
Copy link

cohomology commented Oct 12, 2018

In the implementation of complex, the Add, Sub, etc. for references are forwarded via clone() for values. This is very bad for types which are expensive to copy. In my example, I use Complex<Ratio<BigInt>>.

Can the code be refactored in such a way, that no clone() is necessary? I guess the different arithmetic operations have to be coded out and not forwarded.

I could try that, if there is no objection against that ...

@cuviper
Copy link
Member

cuviper commented Oct 12, 2018

The main problem is that we added all of those implementations with just T: Clone + Num, and it's a breaking change to add additional constraints. To implement ops directly on references, we would probably want to use NumRef, NumAssignRef, and RefNum.

@cuviper
Copy link
Member

cuviper commented Feb 18, 2020

In my example, I use Complex<Ratio<BigInt>>.

Note, Ratio also implements reference ops by cloning, which means that changing Complex alone wouldn't really help you.

I started looking at this for 0.3 (#70), but I'm skeptical. It requires a lot of code duplication on our part to directly implement each op with/without clones, and the additional T: NumRef + ... requirements end up propagating widely. The latter will affect all generic users too, sadly.

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

2 participants