LGTM

http://codereview.chromium.org/2884039/diff/25003/32001
File src/arguments.h (right):

http://codereview.chromium.org/2884039/diff/25003/32001#newcode92
src/arguments.h:92: #define RUNTIME_CCONV Arguments args, Isolate*
isolate
I don't really like this macro. Maybe because of its name. When I talked
about macros I had another pattern in mind:
RUNTIME_FUNCTION(Foo) {
  // Here we have args and isolate.
}
This is how it's done for the builtins.

You can leave the current pattern for now, but please use a more
descriptive name.

http://codereview.chromium.org/2884039/diff/25003/32004
File src/assembler.h (right):

http://codereview.chromium.org/2884039/diff/25003/32004#newcode409
src/assembler.h:409: explicit ExternalReference(Isolate* isolate);
Can this be a predefined external reference (e.g. like roots_address)?

http://codereview.chromium.org/2884039/diff/25003/32014
File src/runtime.cc (right):

http://codereview.chromium.org/2884039/diff/25003/32014#newcode621
src/runtime.cc:621: return isolate->heap()->undefined_value();
Please introduce heap local variable for readability reasons.

http://codereview.chromium.org/2884039/show

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to