LGTM from my end. Just nits and suggestions.

https://codereview.chromium.org/1294543002/diff/20001/src/interpreter/bytecode-generator.cc
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/1294543002/diff/20001/src/interpreter/bytecode-generator.cc#newcode8
src/interpreter/bytecode-generator.cc:8: #include
"src/interpreter/bytecode-generator.h"
nit: Please move the include of bytecode-generator.h all the way to the
top of the include list (above "stack" even).

https://codereview.chromium.org/1294543002/diff/20001/src/interpreter/bytecode-generator.cc#newcode28
src/interpreter/bytecode-generator.cc:28:
DCHECK(scope()->is_function_scope());
I assume this is just a temporary guard, because it won't hold for all
compilation units.

https://codereview.chromium.org/1294543002/diff/20001/src/interpreter/bytecode-generator.cc#newcode101
src/interpreter/bytecode-generator.cc:101:
stmt->expression()->Accept(this);
nit: Better use Visit(stmt) here. Performs stack-overflow/bailout check
and allows to override BytecodeGenerator::Visit later if needed.

https://codereview.chromium.org/1294543002/diff/20001/src/runtime/runtime-compiler.cc
File src/runtime/runtime-compiler.cc (right):

https://codereview.chromium.org/1294543002/diff/20001/src/runtime/runtime-compiler.cc#newcode39
src/runtime/runtime-compiler.cc:39: code->kind() == Code::FUNCTION ||
suggestion: Do you think it makes sense to add a
Code::IsJavaScriptCode() predicate for this? We could also wait until we
have more uses of it before we add it.

https://codereview.chromium.org/1294543002/diff/20001/tools/gyp/v8.gyp
File tools/gyp/v8.gyp (right):

https://codereview.chromium.org/1294543002/diff/20001/tools/gyp/v8.gyp#newcode802
tools/gyp/v8.gyp:802: '../../src/interpreter/bytecode-generator.cc',
nit: This will also require BUILD.gn to be adapted.

https://codereview.chromium.org/1294543002/

--
--
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.

Reply via email to