Fix -Wshift-negative-value transgression in tcmalloc.

Fix -Wshift-negative-value transgression in tcmalloc.

BUG=706476
Review-Url: https://codereview.chromium.org/2793793002
Cr-Commit-Position: refs/heads/master@{#461344}
Committed: https://chromium.googlesource.com/chromium/src/+/726c89662647e7699554827c170d88f7f0543606

[Nico, 2017-04-02 00:49:11]

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

[commit-bot: I haz the power, 2017-04-02 00:49:18]

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

[Nico, 2017-04-02 00:49:43]

thakis@chromium.org changed reviewers:
+ primiano@chromium.org

[commit-bot: I haz the power, 2017-04-02 02:42:55]

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

[commit-bot: I haz the power, 2017-04-02 02:42:56]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Nico, 2017-04-02 11:54:55]

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

[commit-bot: I haz the power, 2017-04-02 11:54:59]

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, 2017-04-02 12:40:55]

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

[commit-bot: I haz the power, 2017-04-02 12:40:55]

Dry run: This issue passed the CQ dry run.

[Primiano Tucci (use gerrit), 2017-04-02 16:49:44]

thanks! lgtm

https://codereview.chromium.org/2793793002/diff/1/third_party/tcmalloc/chromi...
File third_party/tcmalloc/chromium/src/base/basictypes.h (right):

https://codereview.chromium.org/2793793002/diff/1/third_party/tcmalloc/chromi...
third_party/tcmalloc/chromium/src/base/basictypes.h:85: const  int64 kint64min =
 (-kint64max - 1LL);
took my a while to figure out why this was defined like this (which, after some
googling, seems to match most #define INT64_MIN out there).
It's impressive how these days we have very sophisticated features like lambdas
with captures, rvalue references, thread-safe statics, but still, integers are
so frighteningly easy to get wrong.

Instead, I have to admit I still can't figure out the kint64max above ? Why
isn't that just 0x7fff(enough f's)LL ? (and, to be clear, I am not implying we
should change it, but just curious at this point)

[Primiano Tucci (use gerrit), 2017-04-02 16:50:05]

The CQ bit was checked by primiano@chromium.org

[commit-bot: I haz the power, 2017-04-02 16:50:14]

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

[Nico, 2017-04-02 18:17:54]

Thanks!

https://codereview.chromium.org/2793793002/diff/1/third_party/tcmalloc/chromi...
File third_party/tcmalloc/chromium/src/base/basictypes.h (right):

https://codereview.chromium.org/2793793002/diff/1/third_party/tcmalloc/chromi...
third_party/tcmalloc/chromium/src/base/basictypes.h:85: const  int64 kint64min =
 (-kint64max - 1LL);
On 2017/04/02 16:49:44, Primiano Tucci wrote:
> took my a while to figure out why this was defined like this (which, after
some
> googling, seems to match most #define INT64_MIN out there).
> It's impressive how these days we have very sophisticated features like
lambdas
> with captures, rvalue references, thread-safe statics, but still, integers are
> so frighteningly easy to get wrong.
> 
> Instead, I have to admit I still can't figure out the kint64max above ? Why
> isn't that just 0x7fff(enough f's)LL ? (and, to be clear, I am not implying we
> should change it, but just curious at this point)

0xffff is an int literal. 0xffffLL should work, but that wasn't standardized
until I think c++11, and tcmalloc predates that. I agree that would be better,
but the current way fits the file better. (There's a lot of stuff in here we
could delete with c++11.)

[commit-bot: I haz the power, 2017-04-02 18:28:00]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1491151805200480, "parent_rev":
"126f68674476bb25233a1c6bfb3526c8d804cca5", "commit_rev":
"726c89662647e7699554827c170d88f7f0543606"}

[commit-bot: I haz the power, 2017-04-02 18:29:31]

Description was changed from

==========
Fix -Wshift-negative-value transgression in tcmalloc.

BUG=706476
==========

to

==========
Fix -Wshift-negative-value transgression in tcmalloc.

BUG=706476

Review-Url: https://codereview.chromium.org/2793793002
Cr-Commit-Position: refs/heads/master@{#461344}
Committed:
https://chromium.googlesource.com/chromium/src/+/726c89662647e7699554827c170d...
==========

[commit-bot: I haz the power, 2017-04-02 18:29:33]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/726c89662647e7699554827c170d...

[Lei Zhang, 2017-04-03 20:51:08]

https://codereview.chromium.org/2793793002/diff/1/third_party/tcmalloc/chromi...
File third_party/tcmalloc/chromium/src/base/basictypes.h (right):

https://codereview.chromium.org/2793793002/diff/1/third_party/tcmalloc/chromi...
third_party/tcmalloc/chromium/src/base/basictypes.h:85: const  int64 kint64min =
 (-kint64max - 1LL);
On 2017/04/02 18:17:54, Nico (afk until Tue Apr 4) wrote:
> On 2017/04/02 16:49:44, Primiano Tucci wrote:
> > took my a while to figure out why this was defined like this (which, after
> some
> > googling, seems to match most #define INT64_MIN out there).
> > It's impressive how these days we have very sophisticated features like
> lambdas
> > with captures, rvalue references, thread-safe statics, but still, integers
are
> > so frighteningly easy to get wrong.
> > 
> > Instead, I have to admit I still can't figure out the kint64max above ? Why
> > isn't that just 0x7fff(enough f's)LL ? (and, to be clear, I am not implying
we
> > should change it, but just curious at this point)
> 
> 0xffff is an int literal. 0xffffLL should work, but that wasn't standardized
> until I think c++11, and tcmalloc predates that. I agree that would be better,
> but the current way fits the file better. (There's a lot of stuff in here we
> could delete with c++11.)

Do we want to file a bug to refresh tcmalloc with C++11?