Reviewers: arv, rossberg,
Description:
Modules: simplify logic around allocation of module internal variables
Since recursive modules are gone, only the top-level scope can have
module inner scopes. Rename Scope::AllocateModulesRecursively to
Scope::AllocateModules, and add test showing the module Variables
are still allocated appropriately in the top level scope.
BUG=v8:1569,v8:3940
Please review this at https://codereview.chromium.org/999893003/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+29, -18 lines):
M src/scopes.h
M src/scopes.cc
M test/cctest/test-parsing.cc
Index: src/scopes.cc
diff --git a/src/scopes.cc b/src/scopes.cc
index
b8c357c47ec3ef30ea547493b60949aa93d9aaed..c744dd418bbaaca2b74567048f30ecf208274da3
100644
--- a/src/scopes.cc
+++ b/src/scopes.cc
@@ -656,9 +656,9 @@ bool Scope::AllocateVariables(ParseInfo* info,
AstNodeFactory* factory) {
PropagateScopeInfo(outer_scope_calls_sloppy_eval);
// 2) Allocate module instances.
- if (FLAG_harmony_modules && (is_script_scope() || is_module_scope())) {
+ if (FLAG_harmony_modules && is_script_scope()) {
DCHECK(num_modules_ == 0);
- AllocateModulesRecursively(this);
+ AllocateModules();
}
// 3) Resolve variables.
@@ -1437,19 +1437,18 @@ void Scope::AllocateVariablesRecursively(Isolate*
isolate) {
}
-void Scope::AllocateModulesRecursively(Scope* host_scope) {
- if (already_resolved()) return;
- if (is_module_scope()) {
- DCHECK(module_descriptor_->IsFrozen());
- DCHECK(module_var_ == NULL);
- module_var_ =
- host_scope->NewInternal(ast_value_factory_->dot_module_string());
- ++host_scope->num_modules_;
- }
-
+void Scope::AllocateModules() {
+ DCHECK(is_script_scope());
+ DCHECK(!already_resolved());
for (int i = 0; i < inner_scopes_.length(); i++) {
- Scope* inner_scope = inner_scopes_.at(i);
- inner_scope->AllocateModulesRecursively(host_scope);
+ Scope* scope = inner_scopes_.at(i);
+ if (scope->is_module_scope()) {
+ DCHECK(!scope->already_resolved());
+ DCHECK(scope->module_descriptor_->IsFrozen());
+ DCHECK_NULL(scope->module_var_);
+ scope->module_var_ =
NewInternal(ast_value_factory_->dot_module_string());
+ ++num_modules_;
+ }
}
}
Index: src/scopes.h
diff --git a/src/scopes.h b/src/scopes.h
index
9057c35ff261712c9d345fd3648ce28de65550a8..44434fa3622091d19d6b9e511e881883fb0b802f
100644
--- a/src/scopes.h
+++ b/src/scopes.h
@@ -692,7 +692,7 @@ class Scope: public ZoneObject {
void AllocateNonParameterLocal(Isolate* isolate, Variable* var);
void AllocateNonParameterLocals(Isolate* isolate);
void AllocateVariablesRecursively(Isolate* isolate);
- void AllocateModulesRecursively(Scope* host_scope);
+ void AllocateModules();
// Resolve and fill in the allocation information for all variables
// in this scopes. Must be called *after* all scopes have been
Index: test/cctest/test-parsing.cc
diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc
index
12e1fe907ed558ad1564bf92f081521530e9ff84..c91c7b062bc4698142e1c2a38ad409f946f95914
100644
--- a/test/cctest/test-parsing.cc
+++ b/test/cctest/test-parsing.cc
@@ -5220,6 +5220,8 @@ TEST(ImportExportParsingErrors) {
TEST(ModuleParsingInternals) {
+ i::FLAG_harmony_modules = true;
+
i::Isolate* isolate = CcTest::i_isolate();
i::Factory* factory = isolate->factory();
v8::HandleScope handles(CcTest::isolate());
@@ -5244,9 +5246,19 @@ TEST(ModuleParsingInternals) {
parser.set_allow_harmony_scoping(true);
info.set_module();
CHECK(parser.Parse(&info));
+ CHECK(i::Compiler::Analyze(&info));
+
i::FunctionLiteral* func = info.function();
- CHECK_EQ(i::MODULE_SCOPE, func->scope()->scope_type());
- i::ModuleDescriptor* descriptor = func->scope()->module();
+ i::Scope* module_scope = func->scope();
+ i::Scope* outer_scope = module_scope->outer_scope();
+ CHECK(outer_scope->is_script_scope());
+ CHECK_NULL(outer_scope->outer_scope());
+ CHECK_EQ(1, outer_scope->num_modules());
+ CHECK(module_scope->is_module_scope());
+ CHECK_NOT_NULL(module_scope->module_var());
+ CHECK_EQ(i::INTERNAL, module_scope->module_var()->mode());
+
+ i::ModuleDescriptor* descriptor = module_scope->module();
CHECK_NOT_NULL(descriptor);
CHECK_EQ(1, descriptor->Length());
const i::AstRawString* export_name = avf.GetOneByteString("y");
@@ -5254,7 +5266,7 @@ TEST(ModuleParsingInternals) {
descriptor->LookupLocalExport(export_name, &zone);
CHECK_NOT_NULL(local_name);
CHECK(local_name->IsOneByteEqualTo("x"));
- i::ZoneList<i::Declaration*>* declarations =
func->scope()->declarations();
+ i::ZoneList<i::Declaration*>* declarations =
module_scope->declarations();
CHECK_EQ(3, declarations->length());
CHECK(declarations->at(0)->proxy()->raw_name()->IsOneByteEqualTo("x"));
i::ImportDeclaration* import_decl =
--
--
v8-dev mailing list
[email protected]
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 [email protected].
For more options, visit https://groups.google.com/d/optout.
