On 2015/06/05 09:05:38, sejunho wrote:
On 2015/06/05 08:30:43, vogelheim wrote:
> https://codereview.chromium.org/1155043005/diff/1/src/api.cc
> File src/api.cc (right):
>
> https://codereview.chromium.org/1155043005/diff/1/src/api.cc#newcode7214
> src/api.cc:7214: if (index > i::LAST_SPACE || (i::FIRST_SPACE > 0 &&
index <
> i::FIRST_SPACE))
> The way I read globals.h, i::FIRST_SPACE would usually be 0, meaning
this
> effectively eliminates the 2nd condition.
In the android_arm build(ndk-r10c, r10d), the compiler output the warning
message like this:
warning: comparison of unsigned expression < 0 is always false
[-Wtype-limits]
if (index > i::LAST_SPACE || index < i::FIRST_SPACE)
^
> Also, I'm not really sure why this would be the right place to make an
assertion
> about the i::FIRST_SPACE constant in the first place.
>
> Would you care to explain a bit more what you're trying to do? I'm
afraid I
> don't quite get it.
I think it is just simple solution for the warning. Maybe the meaningless
comparison consider that constant about the i::FIRST_SPACE would be
changed.
I want just fix the warning without breaking original intent.
Thanks for the explanation. I get it now.
It's not super readable, though: Previously the compiler complained about a
check that can never be true, and now we make it never-be-true in a somewhat
more obtuse fashion.
I asked around, and one proposal would be to add a compile time assert about
FIRST_SPACE, and then drop the second clause entirely. Like so:
COMPILE_ASSERT(i::FIRST_SPACE == 0, first_space_assumed_to_be_zero);
if (index > i::LAST_SPACE) ...
Would that work?
Reference for COMPILE_ASSERT:
https://code.google.com/p/chromium/codesearch#chromium/src/v8/src/base/macros.h&q=COMPILE_ASSERT&sq=package:chromium&l=122-134
Also, sorry for being a bit pedantic. :-|
https://codereview.chromium.org/1155043005/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
