On 2014/11/03 17:23:37, cpu wrote:
there are quite a few more of those in the file... not compiled for
windows?
Were the other ones references to the same enum? If so then they should all
be
fixed by this change.
One of the other references to store_mode_ is through a store_mode()
accessor
which is probably sufficient to hide the problem from /analyze, although I
believe the problem still exists (the -1 value will be sign extended to
32-bits
and will compare false against 1). I'm not sure why /analyze doesn't
complain
about the seemingly identical instance of this problem at line 7,034.
There are definitely other places in v8 where the bit field layout is not
working well. I've noted down a few of these but I found them based on a
very
crude find-in-files. It may be worth running /d1reportAllClassLayout and
parsing
the output to look for other inefficiently laid out bit fields so that we
get
the intended memory savings. As for other places where sign extension is
causing
incorrect results I'm not sure of any easy way to find them since I'm not
confident that /analyze identifies them all.
I've tried other fixes for this and they are problematic. Specifying the
underlying type for the enum as unsigned char or bool seems like it should
work
but gcc misbehaves with unsigned char and VC++ misbehaves with bool.
Probably
the best solution is to specify the underlying type as unsigned char (to
save
space) and not use a bit field (to avoid compatibility complexity).
The unsigned char typed enums do work with clang, so if we could ditch gcc
we'd
have a sane solution.
It's a mess.
https://codereview.chromium.org/694003002/
--
--
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.
