Thanks for doing this. My comments are about the order of the includes.
Other
than that, LGTM.
http://codereview.chromium.org/9297052/diff/1/src/zone-inl.h
File src/zone-inl.h (right):
http://codereview.chromium.org/9297052/diff/1/src/zone-inl.h#newcode31
src/zone-inl.h:31: #include <stddef.h>
<stddef.h> and <stdint.h> come to us via ../include/v8stdint.h, which
comes to us via globals.h. I think it's safe to leave them out of
individual files (that's the whole point of globals.h). Since they
can't participate in a circular dependency, I'm not much worried about
having them explicit everywhere.
http://codereview.chromium.org/9297052/diff/1/src/zone-inl.h#newcode34
src/zone-inl.h:34: #include "checks.h"
Since zone.h includes checks.h, globals.h, and splay-tree.h, and this
file includes zone.h, and they are part of the same 'module', you can
feel free to leave them out here (or put them in, I don't care too
much).
http://codereview.chromium.org/9297052/diff/1/src/zone-inl.h#newcode41
src/zone-inl.h:41: #include "zone.h"
Please include zone.h first, separated from the other includes by a
blank line, like so:
#include "zone.h"
#include "counters.h"
etc.
http://codereview.chromium.org/9297052/diff/1/src/zone.cc
File src/zone.cc (right):
http://codereview.chromium.org/9297052/diff/1/src/zone.cc#newcode30
src/zone.cc:30: #include "allocation.h"
You are welcome to leave allocation.h out since it comes to us via
zone-inl.h (or put it in, I don't much care).
Please include headers in this order:
#include "v8.h"
#include "zone-inl.h"
#include <string.h>
#include "allocation.h"
The first block are those that (possibly) can't be reordered, and they
are intentionally not alphabetized. v8.h is first because it contains
all the included that have circular dependencies included in just the
right order.
http://codereview.chromium.org/9297052/diff/1/src/zone.h
File src/zone.h (right):
http://codereview.chromium.org/9297052/diff/1/src/zone.h#newcode31
src/zone.h:31: #include <stddef.h>
globals.h includes ../include/v8stdint.h, which includes <stddef.h>. I
think it's safe to rely on that, otherwise we would be including
<stddef.h> (for NULL) in almost every single file.
Suggestion: remove this line, "globals.h" is enough.
http://codereview.chromium.org/9297052/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
