First round of low-level comments.
https://codereview.chromium.org/11093074/diff/1/src/full-codegen.cc File src/full-codegen.cc (right): https://codereview.chromium.org/11093074/diff/1/src/full-codegen.cc#newcode746 src/full-codegen.cc:746: __ push(Immediate(Smi::FromInt(interface->Index()))); See comment further down the file. https://codereview.chromium.org/11093074/diff/1/src/full-codegen.cc#newcode822 src/full-codegen.cc:822: description->set(1, *scope->GetScopeInfo()); Pattern is not GC-safe. https://codereview.chromium.org/11093074/diff/1/src/full-codegen.cc#newcode1123 src/full-codegen.cc:1123: __ push(Immediate(Smi::FromInt(stmt->proxy()->interface()->Index()))); You will have a hard time porting this to other architectures, because the Immediate operand cannot be used there. Better use the uppercase "__ Push()" wrapper for that. https://codereview.chromium.org/11093074/diff/1/src/ia32/full-codegen-ia32.cc File src/ia32/full-codegen-ia32.cc (right): https://codereview.chromium.org/11093074/diff/1/src/ia32/full-codegen-ia32.cc#newcode895 src/ia32/full-codegen-ia32.cc:895: while (host->is_module_scope()) host = host->outer_scope(); Can we move this into a Scope::GlobalScope() helper? We already have a similar thing for Scope::DeclarationScope(). https://codereview.chromium.org/11093074/diff/1/src/runtime.cc File src/runtime.cc (right): https://codereview.chromium.org/11093074/diff/1/src/runtime.cc#newcode8508 src/runtime.cc:8508: while (previous->IsModuleContext()) previous = previous->previous(); Use a Context::global_context() helper here. https://codereview.chromium.org/11093074/diff/1/src/runtime.cc#newcode8517 src/runtime.cc:8517: static_cast<PropertyAttributes>(READ_ONLY | DONT_DELETE | DONT_ENUM); It might make sense to move these constants into the PropertyAttributes class itself, so that others can use them as well. https://codereview.chromium.org/11093074/diff/1/src/runtime.cc#newcode8571 src/runtime.cc:8571: USE(JSObject::PreventExtensions(module)); There should be no need to use the return value of the handlified version. https://codereview.chromium.org/11093074/diff/1/test/cctest/test-decls.cc File test/cctest/test-decls.cc (right): https://codereview.chromium.org/11093074/diff/1/test/cctest/test-decls.cc#newcode737 test/cctest/test-decls.cc:737: Drop the third empty newline. https://codereview.chromium.org/11093074/diff/1/test/cctest/test-decls.cc#newcode779 test/cctest/test-decls.cc:779: Drop the third empty newline. https://codereview.chromium.org/11093074/diff/1/test/cctest/test-decls.cc#newcode808 test/cctest/test-decls.cc:808: Drop the third empty newline. https://codereview.chromium.org/11093074/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
