Add support for CheckedNumeric Max and Min constexpr functions

Add support for CheckedNumeric Max and Min constexpr functions

This also adds variadic helper functions CheckMax and CheckMin that
return the maximum or minimum (respectively) result of an arbitrary
number of arithmetic, CheckedNumeric, or StrictNumeric arguments.

Committed: https://crrev.com/711ac6a11fe3f06803f2f3d76f199e7ac67574bf
Cr-Commit-Position: refs/heads/master@{#436190}

[jschuh, 2016-12-03 18:32:57]

The CQ bit was checked by jschuh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-03 18:33:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jschuh, 2016-12-03 19:25:04]

jschuh@chromium.org changed reviewers:
+ dcheng@chromium.org

[jschuh, 2016-12-03 19:25:05]

dcheng@ - Feel free to punt this back if you don't have time to review. I'm just
trying to branch out reviewers for this code a bit.

[commit-bot: I haz the power, 2016-12-03 21:02:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-03 21:02:07]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[dcheng, 2016-12-03 21:34:29]

https://codereview.chromium.org/2545253002/diff/40001/base/numerics/safe_conv...
File base/numerics/safe_conversions.h (right):

https://codereview.chromium.org/2545253002/diff/40001/base/numerics/safe_conv...
base/numerics/safe_conversions.h:236: constexpr StrictNumeric<typename
UnderlyingType<T>::type> StrictNum(
Nit: usually we name the deducing helpers MakeFoo, e.g. MakeUnique or
make_scoped_refptr. I think that's what I'd personally prefer to see in the
base/numerics interface for consitency.

https://codereview.chromium.org/2545253002/diff/40001/base/numerics/safe_conv...
File base/numerics/safe_conversions_impl.h (right):

https://codereview.chromium.org/2545253002/diff/40001/base/numerics/safe_conv...
base/numerics/safe_conversions_impl.h:357: // Determines the type that can
represent the lowest arithmetic value.
Nit: it kind of feels like we should be more consistent about terminology here
and just call this minimum instead of lowest.

https://codereview.chromium.org/2545253002/diff/40001/base/numerics/safe_math...
File base/numerics/safe_math_impl.h (right):

https://codereview.chromium.org/2545253002/diff/40001/base/numerics/safe_math...
base/numerics/safe_math_impl.h:734: return value_ <=
std::numeric_limits<T>::max() &&
Out of curiosity, how come this used to work?

[jschuh, 2016-12-03 22:29:13]

https://codereview.chromium.org/2545253002/diff/40001/base/numerics/safe_conv...
File base/numerics/safe_conversions.h (right):

https://codereview.chromium.org/2545253002/diff/40001/base/numerics/safe_conv...
base/numerics/safe_conversions.h:236: constexpr StrictNumeric<typename
UnderlyingType<T>::type> StrictNum(
On 2016/12/03 21:34:29, dcheng wrote:
> Nit: usually we name the deducing helpers MakeFoo, e.g. MakeUnique or
> make_scoped_refptr. I think that's what I'd personally prefer to see in the
> base/numerics interface for consitency.

Fair. I'll fix it here and convert CheckNum to MakeChecked in another CL as
well.

https://codereview.chromium.org/2545253002/diff/40001/base/numerics/safe_conv...
File base/numerics/safe_conversions_impl.h (right):

https://codereview.chromium.org/2545253002/diff/40001/base/numerics/safe_conv...
base/numerics/safe_conversions_impl.h:357: // Determines the type that can
represent the lowest arithmetic value.
On 2016/12/03 21:34:29, dcheng wrote:
> Nit: it kind of feels like we should be more consistent about terminology here
> and just call this minimum instead of lowest.

I tried to be consistent with std::numeric_limits<> (where the right thing is
max and lowest, not min). I'm not wedded to that approach, but I was going to
replace all the occurrences of -numeric_limits<FLOAT>::max() and
numeric_limits<INTEGER>::min() with numeric_limits<>::lowest(). So, this just
felt more consistent in that world.

https://codereview.chromium.org/2545253002/diff/40001/base/numerics/safe_math...
File base/numerics/safe_math_impl.h (right):

https://codereview.chromium.org/2545253002/diff/40001/base/numerics/safe_math...
base/numerics/safe_math_impl.h:734: return value_ <=
std::numeric_limits<T>::max() &&
On 2016/12/03 21:34:29, dcheng wrote:
> Out of curiosity, how come this used to work?

We didn't have any code tickling the constexpr path. I stumbled on it when I
originally wrote all of the tests as static_assert in an earlier version of this
CL. Unfortunately, our Android compilers choked in a few other areas, so I had
to opt for runtime tests.

[dcheng, 2016-12-04 01:42:15]

LGTM

https://codereview.chromium.org/2545253002/diff/40001/base/numerics/safe_conv...
File base/numerics/safe_conversions.h (right):

https://codereview.chromium.org/2545253002/diff/40001/base/numerics/safe_conv...
base/numerics/safe_conversions.h:236: constexpr StrictNumeric<typename
UnderlyingType<T>::type> StrictNum(
On 2016/12/03 22:29:12, jschuh (very slow) wrote:
> On 2016/12/03 21:34:29, dcheng wrote:
> > Nit: usually we name the deducing helpers MakeFoo, e.g. MakeUnique or
> > make_scoped_refptr. I think that's what I'd personally prefer to see in the
> > base/numerics interface for consitency.
> 
> Fair. I'll fix it here and convert CheckNum to MakeChecked in another CL as
> well. 

MakeStrict / MakeChecked almost feel too generic (it's not obvious they have
anything to do with arithmetic without context). But I'm happy to bikeshed in
the followup.

[jschuh, 2016-12-04 03:34:52]

On 2016/12/04 01:42:15, dcheng wrote:
> LGTM
> 
>
https://codereview.chromium.org/2545253002/diff/40001/base/numerics/safe_conv...
> File base/numerics/safe_conversions.h (right):
> 
>
https://codereview.chromium.org/2545253002/diff/40001/base/numerics/safe_conv...
> base/numerics/safe_conversions.h:236: constexpr StrictNumeric<typename
> UnderlyingType<T>::type> StrictNum(
> On 2016/12/03 22:29:12, jschuh (very slow) wrote:
> > On 2016/12/03 21:34:29, dcheng wrote:
> > > Nit: usually we name the deducing helpers MakeFoo, e.g. MakeUnique or
> > > make_scoped_refptr. I think that's what I'd personally prefer to see in
the
> > > base/numerics interface for consitency.
> > 
> > Fair. I'll fix it here and convert CheckNum to MakeChecked in another CL as
> > well. 
> 
> MakeStrict / MakeChecked almost feel too generic (it's not obvious they have
> anything to do with arithmetic without context). But I'm happy to bikeshed in
> the followup.

Good point. How about MakeStrictNum?

[dcheng, 2016-12-04 03:41:41]

On 2016/12/04 03:34:52, jschuh (very slow) wrote:
> On 2016/12/04 01:42:15, dcheng wrote:
> > LGTM
> > 
> >
>
https://codereview.chromium.org/2545253002/diff/40001/base/numerics/safe_conv...
> > File base/numerics/safe_conversions.h (right):
> > 
> >
>
https://codereview.chromium.org/2545253002/diff/40001/base/numerics/safe_conv...
> > base/numerics/safe_conversions.h:236: constexpr StrictNumeric<typename
> > UnderlyingType<T>::type> StrictNum(
> > On 2016/12/03 22:29:12, jschuh (very slow) wrote:
> > > On 2016/12/03 21:34:29, dcheng wrote:
> > > > Nit: usually we name the deducing helpers MakeFoo, e.g. MakeUnique or
> > > > make_scoped_refptr. I think that's what I'd personally prefer to see in
> the
> > > > base/numerics interface for consitency.
> > > 
> > > Fair. I'll fix it here and convert CheckNum to MakeChecked in another CL
as
> > > well. 
> > 
> > MakeStrict / MakeChecked almost feel too generic (it's not obvious they have
> > anything to do with arithmetic without context). But I'm happy to bikeshed
in
> > the followup.
> 
> Good point. How about MakeStrictNum?

That name LGTM

[jschuh, 2016-12-04 05:29:14]

Patchset #5 (id:80001) has been deleted

[jschuh, 2016-12-04 05:29:57]

The CQ bit was checked by jschuh@chromium.org

[jschuh, 2016-12-04 05:29:58]

The patchset sent to the CQ was uploaded after l-g-t-m from dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2545253002/#ps100001
(title: "MakeStrictNum")

[commit-bot: I haz the power, 2016-12-04 05:30:05]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-04 07:17:52]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1480829397584920,
"parent_rev": "f33a187039dfee6f2926b999f783594c394981b3", "commit_rev":
"fa48742034e378eefa980a64c1e6c5e3c43c124b"}

[commit-bot: I haz the power, 2016-12-04 07:18:48]

Committed patchset #5 (id:100001)

[commit-bot: I haz the power, 2016-12-04 07:20:19]

Description was changed from

==========
Add support for CheckedNumeric Max and Min constexpr functions

This also adds variadic helper functions CheckMax and CheckMin that
return the maximum or minimum (respectively) result of an arbitrary
number of arithmetic, CheckedNumeric, or StrictNumeric arguments.
==========

to

==========
Add support for CheckedNumeric Max and Min constexpr functions

This also adds variadic helper functions CheckMax and CheckMin that
return the maximum or minimum (respectively) result of an arbitrary
number of arithmetic, CheckedNumeric, or StrictNumeric arguments.

Committed: https://crrev.com/711ac6a11fe3f06803f2f3d76f199e7ac67574bf
Cr-Commit-Position: refs/heads/master@{#436190}
==========

[commit-bot: I haz the power, 2016-12-04 07:20:20]

Patchset 5 (id:??) landed as
https://crrev.com/711ac6a11fe3f06803f2f3d76f199e7ac67574bf
Cr-Commit-Position: refs/heads/master@{#436190}