Skip to content

Commit

Permalink
Stop shadowing ONE (#205)
Browse files Browse the repository at this point in the history
Our loosening of the overflow safety surface introduced a warning, which
is prominently visible on our [canonical godbolt link]:


![image](https://github.com/aurora-opensource/au/assets/1819744/d55ef99f-7073-4644-9ffc-f75fa3893441)

Oops!  We shadowed the _magnitude_ `ONE` (globally available in the
library) with the _constant_ `ONE`.

The quick fix is to rename the newcomer (which is not publicly visible
anyway) to `UNITY`.

To prevent this problem from recurring, we also add `-Wshadow` to our
regular compiler options.

[canonical godbolt link]: https://godbolt.org/z/KrvfhP4M3
  • Loading branch information
chiphogg authored Dec 4, 2023
1 parent cd9cccf commit 013c343
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 4 deletions.
8 changes: 4 additions & 4 deletions au/math.hh
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,8 @@ constexpr auto int_pow(Quantity<U, R> q) {
template <typename TargetRep, typename TargetUnits, typename U, typename R>
constexpr auto inverse_in(TargetUnits target_units, Quantity<U, R> q) {
using Rep = std::common_type_t<TargetRep, R>;
constexpr auto ONE = make_constant(UnitProductT<>{});
return static_cast<TargetRep>(ONE.in<Rep>(associated_unit(target_units) * U{}) / q.in(U{}));
constexpr auto UNITY = make_constant(UnitProductT<>{});
return static_cast<TargetRep>(UNITY.in<Rep>(associated_unit(target_units) * U{}) / q.in(U{}));
}

//
Expand Down Expand Up @@ -270,10 +270,10 @@ constexpr auto inverse_in(TargetUnits target_units, Quantity<U, R> q) {
// This will fail at compile time for types that can't hold 1'000'000.
constexpr R threshold = 1'000'000;

constexpr auto ONE = make_constant(UnitProductT<>{});
constexpr auto UNITY = make_constant(UnitProductT<>{});

static_assert(
ONE.in<R>(associated_unit(target_units) * U{}) >= threshold ||
UNITY.in<R>(associated_unit(target_units) * U{}) >= threshold ||
std::is_floating_point<R>::value,
"Dangerous inversion risking truncation to 0; must supply explicit Rep if truly desired");

Expand Down
1 change: 1 addition & 0 deletions build/copts.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ BASE_CLANG_COPTS = [
# Diagnostics
"-fcolor-diagnostics",
"-Wall",
"-Wshadow",
"-Wthread-safety",
"-Wself-assign",
]

0 comments on commit 013c343

Please sign in to comment.