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

Implement operations for Quantity references #38

Open
3 tasks
iliekturtles opened this issue Dec 20, 2017 · 4 comments · May be fixed by #307
Open
3 tasks

Implement operations for Quantity references #38

iliekturtles opened this issue Dec 20, 2017 · 4 comments · May be fixed by #307

Comments

@iliekturtles
Copy link
Owner

impl Op<Quantity<...>> for Quantity<..> is already complete. Add the following reference implementations:

  • impl<'a, 'b> Op<&'a Quantity<...>> for &'b Quantity<..>
  • impl<'a> Op<&'a Quantity<...>> for Quantity<..>
  • impl<'a> Op<Quantity<...>> for &'a Quantity<..>
@iliekturtles iliekturtles added this to the v1.0.0 milestone Jun 29, 2019
@iliekturtles iliekturtles removed this from the v1.0.0 milestone Jun 29, 2019
@jacg
Copy link
Contributor

jacg commented Apr 9, 2022

Are there any difficulties or pitfalls here, or is it just a fairly mechanical process that simply requires finding the time to do it?

If it's the latter, I'll start chipping away at it.

If it's the former, I could also have a go, if you brief me.

@iliekturtles
Copy link
Owner Author

Should be completely mechanical.

Implementation would start with the following macros

uom/src/system.rs

Lines 588 to 591 in 66d3e85

impl_ops!(Add, add, +, AddAssign, add_assign, +=, Sum,
Mul, mul, *, MulAssign, mul_assign, *=, add_mul);
impl_ops!(Sub, sub, -, SubAssign, sub_assign, -=, Diff,
Div, div, /, DivAssign, div_assign, /=, sub_div);

And you can compare against what the standard library is doing for inspiration on implementing for T and &T. https://doc.rust-lang.org/src/core/ops/arith.rs.html#117-133

Once the major traits are implemented we can go hunting to see if there is anything else like Sub that needs extra impls.

@jacg
Copy link
Contributor

jacg commented May 25, 2022

Implementation would start with the following macros [impl_ops!]

Just to be sure: Are you suggesting to add the implementations of the reference versions, to the bodies of those macros?

I don't quite understand the pattern inside the macro. First we have this group of 4:

  • ....autoconvert! ... AddSubTrait
  • not_autoconvert! ... AddSubTrait
  • ....autoconvert! ... AddSubAssignTrait
  • not_autoconvert! ... AddSubAssignTrait

followed by another group of 4:

  • ....autoconvert! ... MulDivTrait
  • not_autoconvert! ... MulDivTrait
  • ................ ... MulDivTrait
  • ................ ... MulDivAssignTrait

Why is there an asymmetry between the AddSub group and the MulDiv group?

@jacg jacg linked a pull request May 25, 2022 that will close this issue
@iliekturtles
Copy link
Owner Author

I apologize for taking so long to get to this. I'll look at the PR in a second to see what you've done so far. The original thought was to do something to the standard library and use a macro that accepts the type (T vs. &T) as a parameter.

The asymmetry is because multiply and divide allow for operating on the underlying storage type as well as the quantity. e.g. length * time and length * 3.0. Add/subtract require the same quantity on both sides of the operator. e.g. length1 + length2. The [not_]autoconvert! macros decide if the implementation supports operating on quantities with different base units.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants