Reviewers: wingo (holiday until 11 sep), Diego Pino, vjaquez, Sven Panne,
danno, rossberg, marja, Michael Starzinger,
Message:
On 2014/08/04 06:55:57, Sven Panne wrote:
NOT LGTM.
https://codereview.chromium.org/422363002/diff/20001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/422363002/diff/20001/src/preparser.h#newcode783
src/preparser.h:783: Type type_ : 2;
This actually blows up memory usage instead of reducing it and has several
portability issues. Please use the BitField class from utils.h, like the
rest
of
v8.
First of all, let me say that I do not have any problem at all
changing this to use "BitField".
But as far as portability is concerned, bitfield integrals in structs are
defined in the C++ specification as a compiler feature (it does not need
runtime support from the standard library) and all compilers must support
them. Of course compilers are free to reorder the location of the fields
or make decisions about their packing, which of course plays badly with
dumping structs with bitfields to a file or to the network -- which is a
no-concern as in V8 we are just using "PreParserExpression" internally.
Could you please elaborate what would be the portability problem here
that I am missing?
Description:
Clean up manual bit field usage in PreParserExpression
Instead of using an integer value and manual bit-fiddling, use C++'s
support for specifying bit sizes for integral types. This way the bits
used to describe a PreParserExpression are handled by the compiler.
BUG=
LOG=
Please review this at https://codereview.chromium.org/422363002/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+85, -83 lines):
M src/preparser.h
--
--
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.
