Firstly at line 46 of safe_conversion.h, there's C4100 warning:

  static constexpr bool Do(Src value) {
    // Force a compile failure if instantiated.
    return CheckOnFailure::template HandleFailure<bool>();
  }

Secondly if this is how we handle safe math in the future, it'd probably look like this::

-    THROW_IF_FAILED(ShortAdd(newLeft, delta.X, &newLeft));
+    newLeft = (til::checked_numeric<SHORT>(newLeft) + til::checked_numeric<SHORT>(delta.X)).ValueOrDie();

Yeah ... I know, right. Doesn't look so good.

Firstly at line 46 of safe_conversion.h, there's C4100 warning:

  static constexpr bool Do(Src value) {
    // Force a compile failure if instantiated.
    return CheckOnFailure::template HandleFailure<bool>();
  }

Go to src/inc/LibraryIncludes.h.

Change

// Chromium Numerics (safe math)
#include <base/numerics/safe_math.h>

to

#pragma warning(push)
#pragma warning(suppress:4100) 
// Chromium Numerics (safe math)
#include <base/numerics/safe_math.h>
#pragma warning(pop)

We're not afraid of suppressing warnings from dependent libraries. We really just want the warning/static analysis for our code.

Secondly if this is how we handle safe math in the future, it'd probably look like this::

-    THROW_IF_FAILED(ShortAdd(newLeft, delta.X, &newLeft));
+    newLeft = (til::checked_numeric<SHORT>(newLeft) + til::checked_numeric<SHORT>(delta.X)).ValueOrDie();

Yeah ... I know, right. Doesn't look so good.

I was thinking it'd be more like

    auto checkedLeft = til::CheckAdd(newLeft, delta.X);

I see a base::CheckAdd template that appears to work that way. Which is significantly less ugly. Though if they're all just in the base:: namespace, then maybe we don't need to put them into til? I thought the namespace was deeper than that (like chromium::base::numerics::CheckAdd) which is why I thought we needed to do this.

If we directly use base::CheckAdd it would be like this:

newLeft = base::CheckAdd(newLeft, delta.X).ValueOrDie();

I'm really not a fan of the ValueOrDie part. So I wrapped it. Now it's like this:

newLeft = til::check_add(newLeft, delta.X);

If we directly use base::CheckAdd it would be like this:

newLeft = base::CheckAdd(newLeft, delta.X).ValueOrDie();

I'm really not a fan of the ValueOrDie part. So I wrapped it. Now it's like this:

newLeft = til::check_add(newLeft, delta.X);

I see your point for base::CheckAdd though I'm not sure we should wrap it. I can see a legitimate reason to make the caller choose between ValueOrDie(), ValueOrDefault() or AssignIfValid() in their function.

The could probably just hold the checked value as an auto through their entire set of math operations (so it stays a base::internal::CheckedNumeric<T> the whole time until the final extraction) and it wouldn't be so bad.

The one thing I could think might be remaining is that there isn't really a CheckOrThrow() option. It's CheckOrDie(). So we could maybe wrap it for that reason, or just let the caller handle it through the ValueOrDefault()/AssignIfValid() flows without an exception and try/catch (and use the CheckOrDie() one if the circumstance really warrants a fail-fast.)

As for base::ClampedAdd... I just tried it. You can do this:

int a = 2;
int b = 3;
int c = base::ClampedAdd(a, b)

It will just assign straight out to the base type, not keep it wrapped in the base::internal::ClampedNumeric<T>. So I don't think that one's necessary at all.

So in total.... despite my earlier thought that we should wrap some of these in til namespaces... based on what you've found here and learning more about the numerics library... maybe we just don't for now and close the issue and make one later after we get a bit more experience using the new library. Sound OK?

I'm still rooting for adding til namespace instead of using base from third party code. I’m gonna leave this PR here. Feel free to close it. It is a draft anyway.

I'm still rooting for adding til namespace instead of using base from third party code. I’m gonna leave this PR here. Feel free to close it. It is a draft anyway.

Alright. Not saying I won't change my mind. I do that frequently. Thanks.