Move the remaining CheckedNumeric logic out of the macro

Move the remaining CheckedNumeric logic out of the macro

Moves the remaining run-time logic into template classes and functions.
It also eliminates a number of public interfaces used for tests, etc,
and unifies the handling of floating points and integers.

NOTRY=true
BUG=667097
Committed: https://crrev.com/d1c1f2e2b77a2e0f082756f1d908b3ed71712585
Cr-Commit-Position: refs/heads/master@{#433659}

[jschuh, 2016-11-20 08:10:48]

Description was changed from

==========
Move the remaining CheckedNumeric logic out of the macro

This moves the remaining run-time logic into a set of templates.

BUG=667097
==========

to

==========
Move the remaining CheckedNumeric logic out of the macro

This moves the remaining run-time logic into a set of templates.
It also eliminates some public test interfaces.

BUG=667097
==========

[jschuh, 2016-11-20 08:12:49]

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

[commit-bot: I haz the power, 2016-11-20 08:12:56]

Dry run: 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-11-20 08:23:07]

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

[commit-bot: I haz the power, 2016-11-20 08:23:08]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[jschuh, 2016-11-20 16:11:24]

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

[commit-bot: I haz the power, 2016-11-20 16:11:32]

Dry run: 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-11-20 16:25:46]

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

[commit-bot: I haz the power, 2016-11-20 16:25:46]

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

[jschuh, 2016-11-20 22:11:01]

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

[commit-bot: I haz the power, 2016-11-20 22:11:08]

Dry run: 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-11-20 22:22:58]

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

[commit-bot: I haz the power, 2016-11-20 22:22:59]

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

[jschuh, 2016-11-21 06:40:27]

Description was changed from

==========
Move the remaining CheckedNumeric logic out of the macro

This moves the remaining run-time logic into a set of templates.
It also eliminates some public test interfaces.

BUG=667097
==========

to

==========
Move the remaining CheckedNumeric logic out of the macro

Moves the remaining run-time logic into template classes and functions.
It also eliminates a number of public interfaces used for tests, etc,
and unifies the handling of floating points and integers.

BUG=667097
==========

[jschuh, 2016-11-21 14:48:35]

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

[commit-bot: I haz the power, 2016-11-21 14:48:45]

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

[jschuh, 2016-11-21 15:21:06]

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

[jschuh, 2016-11-21 15:21:07]

tsepez@ - Sorry for the largish CL. Most of the content is mechanical
restructuring to move from function to functor style templates. Some of the
remaining fallout and fixes could arguably have been moved into other CLs, but
it seemed cleaner to just do it all in one go.

[Tom Sepez, 2016-11-21 17:19:52]

lgtm

https://codereview.chromium.org/2516153002/diff/210001/base/numerics/safe_mat...
File base/numerics/safe_math_impl.h (right):

https://codereview.chromium.org/2516153002/diff/210001/base/numerics/safe_mat...
base/numerics/safe_math_impl.h:565: if (!IsValueNegative(x) && real_shift <
kBitWidth) {
nit: while we're at it, these might read easier if we inverted the test and had
an early return? (several other places, too).

https://codereview.chromium.org/2516153002/diff/210001/base/numerics/safe_mat...
base/numerics/safe_math_impl.h:669: // This is just boilerplate that wraps the
standard floating point arithmetic.
Maybe this belongs in the safe_math.h file along with the other macro creating
the checked##NAMEs?

[jschuh, 2016-11-21 17:43:25]

https://codereview.chromium.org/2516153002/diff/210001/base/numerics/safe_mat...
File base/numerics/safe_math_impl.h (right):

https://codereview.chromium.org/2516153002/diff/210001/base/numerics/safe_mat...
base/numerics/safe_math_impl.h:565: if (!IsValueNegative(x) && real_shift <
kBitWidth) {
On 2016/11/21 17:19:52, Tom Sepez wrote:
> nit: while we're at it, these might read easier if we inverted the test and
had
> an early return? (several other places, too).

I'm going to leave it for now, because when I looked at it before the early
return resulted in suboptimal code from one of the compilers. When I'm back in
the office I'll see if it mattered or not, and change if it doesn't.

https://codereview.chromium.org/2516153002/diff/210001/base/numerics/safe_mat...
base/numerics/safe_math_impl.h:669: // This is just boilerplate that wraps the
standard floating point arithmetic.
On 2016/11/21 17:19:52, Tom Sepez wrote:
> Maybe this belongs in the safe_math.h file along with the other macro creating
> the checked##NAMEs?

Nah, this is the file with all of the integer implementations of the same
template functions. Whereas safe_math.h has just the CheckedNumeric
implementation.

[jschuh, 2016-11-21 17:43:31]

The CQ bit was checked by jschuh@chromium.org

[commit-bot: I haz the power, 2016-11-21 17:44:09]

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-11-21 19:47:54]

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

[commit-bot: I haz the power, 2016-11-21 19:47:55]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jschuh, 2016-11-21 19:49:04]

The CQ bit was checked by jschuh@chromium.org

[commit-bot: I haz the power, 2016-11-21 19:49:54]

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

[jschuh, 2016-11-21 21:39:42]

The CQ bit was unchecked by jschuh@chromium.org

[jschuh, 2016-11-21 21:40:24]

Description was changed from

==========
Move the remaining CheckedNumeric logic out of the macro

Moves the remaining run-time logic into template classes and functions.
It also eliminates a number of public interfaces used for tests, etc,
and unifies the handling of floating points and integers.

BUG=667097
==========

to

==========
Move the remaining CheckedNumeric logic out of the macro

Moves the remaining run-time logic into template classes and functions.
It also eliminates a number of public interfaces used for tests, etc,
and unifies the handling of floating points and integers.

NOTRY=true
BUG=667097
==========

[jschuh, 2016-11-21 21:40:32]

The CQ bit was checked by jschuh@chromium.org

[commit-bot: I haz the power, 2016-11-21 21:41:03]

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-11-21 21:45:52]

CQ is committing da patch.

Bot data: {"patchset_id": 210001, "attempt_start_ts": 1479764432084090,
"parent_rev": "2971f53f276c65dfb20b44c2eca5d3613bfbe9fc", "commit_rev":
"0c51582bb0ce1a0baf804c7b07e3b73748eed7f2"}

[commit-bot: I haz the power, 2016-11-21 21:46:17]

Description was changed from

==========
Move the remaining CheckedNumeric logic out of the macro

Moves the remaining run-time logic into template classes and functions.
It also eliminates a number of public interfaces used for tests, etc,
and unifies the handling of floating points and integers.

NOTRY=true
BUG=667097
==========

to

==========
Move the remaining CheckedNumeric logic out of the macro

Moves the remaining run-time logic into template classes and functions.
It also eliminates a number of public interfaces used for tests, etc,
and unifies the handling of floating points and integers.

NOTRY=true
BUG=667097
==========

[commit-bot: I haz the power, 2016-11-21 21:46:18]

Committed patchset #12 (id:210001)

[commit-bot: I haz the power, 2016-11-21 21:48:56]

Description was changed from

==========
Move the remaining CheckedNumeric logic out of the macro

Moves the remaining run-time logic into template classes and functions.
It also eliminates a number of public interfaces used for tests, etc,
and unifies the handling of floating points and integers.

NOTRY=true
BUG=667097
==========

to

==========
Move the remaining CheckedNumeric logic out of the macro

Moves the remaining run-time logic into template classes and functions.
It also eliminates a number of public interfaces used for tests, etc,
and unifies the handling of floating points and integers.

NOTRY=true
BUG=667097

Committed: https://crrev.com/d1c1f2e2b77a2e0f082756f1d908b3ed71712585
Cr-Commit-Position: refs/heads/master@{#433659}
==========

[commit-bot: I haz the power, 2016-11-21 21:48:56]

Patchset 12 (id:??) landed as
https://crrev.com/d1c1f2e2b77a2e0f082756f1d908b3ed71712585
Cr-Commit-Position: refs/heads/master@{#433659}