Skip to content

Commit

Permalink
Replace integer_quotient with unblock_int_div (#315)
Browse files Browse the repository at this point in the history
Instead of `integer_quotient(a, b)`, we now always write
`a / unblock_int_div(b)`.  This gives us everything we used to have with
`integer_quotient`, but with two big advantages:

1. The form for the units code becomes more similar to the non-units
   code that it replaces (i.e., we're using the `/` symbol).

2. We can now support _templated code_ that works for both integral and
   non-integral types: `a / unblock_int_div(b)` works equally well for,
   say, `b` with floating point rep.

There's no longer any use case for `integer_quotient`.  We therefore
immediately deprecate it.  We will keep it deprecated for the entirety
of the 0.4.0 cycle, and remove it for 0.5.0.

Fixes #253.
  • Loading branch information
chiphogg authored Oct 30, 2024
1 parent 99c02d6 commit fbe3d45
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 14 deletions.
58 changes: 54 additions & 4 deletions au/code/au/quantity.hh
Original file line number Diff line number Diff line change
Expand Up @@ -393,33 +393,83 @@ class Quantity {
constexpr bool are_units_quantity_equivalent =
AreUnitsQuantityEquivalent<UnitT, OtherUnit>::value;
static_assert(are_units_quantity_equivalent || !uses_integer_division,
"Integer division forbidden: use integer_quotient() if you really want it");
"Integer division forbidden: wrap denominator in `unblock_int_div()` if you "
"really want it");
}

constexpr Quantity(Rep value) : value_{value} {}

Rep value_{};
};

////////////////////////////////////////////////////////////////////////////////////////////////////
// Machinery to explicitly unblock integer division.
//
// Dividing by `unblock_int_div(x)` will allow integer division for any `x`. If the division would
// have been allowed anyway, then `unblock_int_div` is a no-op: this enables us to write templated
// code to handle template parameters that may or may not be integral.

template <typename U, typename R>
class AlwaysDivisibleQuantity;

// Unblock integer divisoin for a `Quantity`.
template <typename U, typename R>
constexpr AlwaysDivisibleQuantity<U, R> unblock_int_div(Quantity<U, R> q) {
return AlwaysDivisibleQuantity<U, R>{q};
}

// Unblock integer division for any non-`Quantity` type.
template <typename R>
constexpr AlwaysDivisibleQuantity<UnitProductT<>, R> unblock_int_div(R x) {
return AlwaysDivisibleQuantity<UnitProductT<>, R>{make_quantity<UnitProductT<>>(x)};
}

template <typename U, typename R>
class AlwaysDivisibleQuantity {
public:
// Divide a `Quantity` by this always-divisible quantity type.
template <typename U2, typename R2>
friend constexpr auto operator/(Quantity<U2, R2> q2, AlwaysDivisibleQuantity q) {
return make_quantity<UnitQuotientT<U2, U>>(q2.in(U2{}) / q.q_.in(U{}));
}

// Divide any non-`Quantity` by this always-divisible quantity type.
template <typename T>
friend constexpr auto operator/(T x, AlwaysDivisibleQuantity q) {
return make_quantity<UnitInverseT<U>>(x / q.q_.in(U{}));
}

friend constexpr AlwaysDivisibleQuantity<U, R> unblock_int_div<U, R>(Quantity<U, R> q);
friend constexpr AlwaysDivisibleQuantity<UnitProductT<>, R> unblock_int_div<R>(R x);

private:
constexpr AlwaysDivisibleQuantity(Quantity<U, R> q) : q_{q} {}

Quantity<U, R> q_;
};

// Force integer division beteween two integer Quantities, in a callsite-obvious way.
template <typename U1, typename R1, typename U2, typename R2>
constexpr auto integer_quotient(Quantity<U1, R1> q1, Quantity<U2, R2> q2) {
[[deprecated("Replace `integer_quotient(a, b)` with `a / unblock_int_div(b)`")]] constexpr auto
integer_quotient(Quantity<U1, R1> q1, Quantity<U2, R2> q2) {
static_assert(std::is_integral<R1>::value && std::is_integral<R2>::value,
"integer_quotient() can only be called with integral Rep");
return make_quantity<UnitQuotientT<U1, U2>>(q1.in(U1{}) / q2.in(U2{}));
}

// Force integer division beteween an integer Quantity and a raw number.
template <typename U, typename R, typename T>
constexpr auto integer_quotient(Quantity<U, R> q, T x) {
[[deprecated("Replace `integer_quotient(a, b)` with `a / unblock_int_div(b)`")]] constexpr auto
integer_quotient(Quantity<U, R> q, T x) {
static_assert(std::is_integral<R>::value && std::is_integral<T>::value,
"integer_quotient() can only be called with integral Rep");
return make_quantity<U>(q.in(U{}) / x);
}

// Force integer division beteween a raw number and an integer Quantity.
template <typename T, typename U, typename R>
constexpr auto integer_quotient(T x, Quantity<U, R> q) {
[[deprecated("Replace `integer_quotient(a, b)` with `a / unblock_int_div(b)`")]] constexpr auto
integer_quotient(T x, Quantity<U, R> q) {
static_assert(std::is_integral<T>::value && std::is_integral<R>::value,
"integer_quotient() can only be called with integral Rep");
return make_quantity<UnitInverseT<U>>(x / q.in(U{}));
Expand Down
20 changes: 16 additions & 4 deletions au/code/au/quantity_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -906,17 +906,29 @@ TEST(AreQuantityTypesEquivalent, RequiresSameRepAndEquivalentUnits) {
EXPECT_TRUE((AreQuantityTypesEquivalent<IntQFeet, IntQFeetTimesOne>::value));
}

TEST(integer_quotient, EnablesIntegerDivision) {
constexpr auto dt = integer_quotient(meters(60), (miles / hour)(65));
TEST(UnblockIntDiv, EnablesTruncatingIntegerDivisionIntoQuantity) {
constexpr auto dt = meters(60) / unblock_int_div((miles / hour)(65));
EXPECT_THAT(dt, QuantityEquivalent((hour * meters / mile)(0)));
}

constexpr auto x = integer_quotient(meters(60), 31);
TEST(UnblockIntDiv, EnablesDividingByRawInteger) {
constexpr auto x = meters(60) / unblock_int_div(31);
EXPECT_THAT(x, SameTypeAndValue(meters(1)));
}

constexpr auto freq = integer_quotient(1000, minutes(300));
TEST(UnblockIntDiv, EnablesTruncatingIntegerDivisionIntoRawInteger) {
constexpr auto freq = 1000 / unblock_int_div(minutes(300));
EXPECT_THAT(freq, SameTypeAndValue(inverse(minutes)(3)));
}

TEST(UnblockIntDiv, IsNoOpForDivisionThatWouldBeAllowedAnyway) {
auto expect_unblock_int_div_is_no_op = [](auto n, auto d) {
EXPECT_THAT(n / unblock_int_div(d), SameTypeAndValue(n / d));
};
expect_unblock_int_div_is_no_op(meters(60), (miles / hour)(65.0));
expect_unblock_int_div_is_no_op(1.23, minutes(4.56));
}

TEST(Quantity, CanIntegerDivideQuantitiesOfQuantityEquivalentUnits) {
constexpr auto ratio = meters(60) / meters(25);
EXPECT_EQ(ratio, 2);
Expand Down
12 changes: 6 additions & 6 deletions docs/reference/quantity.md
Original file line number Diff line number Diff line change
Expand Up @@ -473,29 +473,29 @@ number](../discussion/concepts/dimensionless.md#exact-cancellation).
If either _input_ is a raw number, then it only affects the value, not the unit. It's equivalent to
a `Quantity` whose unit is [a unitless unit](./unit.md#unitless-unit).

#### `integer_quotient()`
#### `unblock_int_div()`

Experience has shown that raw integer division can be dangerous in a units library context. It
conflicts with intuitions, and can produce code that is silently and grossly incorrect: see the
[integer division section](../troubleshooting.md#integer-division-forbidden) of the troubleshooting
guide for an example.

To use integer division, you must ask for it explicitly by name, with the `integer_quotient()`
function.
To use integer division, you must ask for it explicitly by name, by calling `unblock_int_div()` on
the denominator.

??? example "Using `integer_quotient()` to explicitly opt in to integer division"
??? example "Using `unblock_int_div()` to explicitly opt in to integer division"

This will not work:

```cpp
miles(125) / hours(2);
// ^--- Forbidden! Compiler error.
// ^--- Forbidden! Compiler error.
```

However, this will work just fine:

```cpp
integer_quotient(miles(125), hours(2));
miles(125) / unblock_int_div(hours(2));
```

It produces `(miles / hour)(62)`.
Expand Down

0 comments on commit fbe3d45

Please sign in to comment.