LGTM. Cool stuff! I left just nits.
Would you mind renaming all variable and parameter names in all functions
you
touched to correspond to our coding style guide ("Function names, variable
names, and
filenames should be descriptive; eschew abbreviation."). I started
commenting in
one file but stopped because I spotted these violations in almost every
file. We
should use proper names, no abbreviations, e.g., obj -> object, str ->
string,
... Since you are already touching that code, it would be awesome to clean
that
up.
https://codereview.chromium.org/259173003/diff/1/src/heap-inl.h
File src/heap-inl.h (right):
https://codereview.chromium.org/259173003/diff/1/src/heap-inl.h#newcode134
src/heap-inl.h:134: uint32_t hash_field) {
indent
https://codereview.chromium.org/259173003/diff/1/src/heap-inl.h#newcode406
src/heap-inl.h:406: return false;
Is this necessary here? This case is caught by the default UNREACHABLE
before
below.
https://codereview.chromium.org/259173003/diff/1/src/heap.cc
File src/heap.cc (right):
https://codereview.chromium.org/259173003/diff/1/src/heap.cc#newcode2423
src/heap.cc:2423: HeapObject* obj;
obj -> object; c++ google styleguide "Function names, variable names,
and filenames should be descriptive; eschew abbreviation."
https://codereview.chromium.org/259173003/diff/1/src/heap.cc#newcode3606
src/heap.cc:3606: JSObject* js_obj = JSObject::cast(result);
js_obj -> js_object
https://codereview.chromium.org/259173003/diff/1/src/heap.cc#newcode3683
src/heap.cc:3683: JSObject* js_obj;
js_obj -> js_object
https://codereview.chromium.org/259173003/diff/1/src/heap.cc#newcode3984
src/heap.cc:3984: PretenureFlag pretenure) {
indent
https://codereview.chromium.org/259173003/diff/1/src/heap.cc#newcode4026
src/heap.cc:4026: AllocationResult
Heap::CopyAndTenureFixedCOWArray(FixedArray* src) {
src -> source
https://codereview.chromium.org/259173003/diff/1/src/heap.cc#newcode4032
src/heap.cc:4032: HeapObject* obj;
obj -> object
https://codereview.chromium.org/259173003/diff/1/src/heap.cc#newcode4059
src/heap.cc:4059: AllocationResult
Heap::CopyFixedArrayWithMap(FixedArray* src, Map* map) {
src -> source
https://codereview.chromium.org/259173003/diff/1/src/heap.cc#newcode4061
src/heap.cc:4061: HeapObject* obj;
obj -> object
https://codereview.chromium.org/259173003/diff/1/src/heap.cc#newcode4084
src/heap.cc:4084: AllocationResult
Heap::CopyFixedDoubleArrayWithMap(FixedDoubleArray* src,
src -> source
https://codereview.chromium.org/259173003/diff/1/src/heap.cc#newcode4087
src/heap.cc:4087: HeapObject* obj;
obj -> object
https://codereview.chromium.org/259173003/diff/1/src/heap.cc#newcode4100
src/heap.cc:4100: AllocationResult
Heap::CopyConstantPoolArrayWithMap(ConstantPoolArray* src,
src -> source
https://codereview.chromium.org/259173003/diff/1/src/heap.cc#newcode4635
src/heap.cc:4635: case INVALID_SPACE:
Instead of case INVALID_SPACE we could also use default...
https://codereview.chromium.org/259173003/diff/1/src/mark-compact.cc
File src/mark-compact.cc (right):
https://codereview.chromium.org/259173003/diff/1/src/mark-compact.cc#newcode21
src/mark-compact.cc:21: #include "spaces-inl.h"
order:
#include "spaces-inl.h"
#include "stub-cache.h"
#include "sweeper-thread.h"
https://codereview.chromium.org/259173003/diff/1/src/spaces.h
File src/spaces.h (right):
https://codereview.chromium.org/259173003/diff/1/src/spaces.h#newcode1500
src/spaces.h:1500: static inline FreeListNode* cast(Object* maybe) {
maybe -> object
https://codereview.chromium.org/259173003/diff/1/src/v8globals.h
File src/v8globals.h (right):
https://codereview.chromium.org/259173003/diff/1/src/v8globals.h#newcode165
src/v8globals.h:165: INVALID_SPACE,
Can we have a comment here?
https://codereview.chromium.org/259173003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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 v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
