Teach ModuleDescriptor about basic local exports

Add() becomes AddLocalExport, which takes an export_name and a local_name.
New parsing tests exercise this.

Also start generating exports for default exports (though this doesn't yet
handle anonymous default exports).

BUG=v8:1569
LOG=n

Review URL: https://codereview.chromium.org/934323004

Cr-Commit-Position: refs/heads/master@{#26758}

src/ast-value-factory.h

@@ -234,6 +234,7 @@ class AstValue : public ZoneObject {
   F(anonymous_function, "(anonymous function)")         \
   F(arguments, "arguments")                             \
   F(constructor, "constructor")                         \
+  F(default, "default")                                 \
   F(done, "done")                                       \
   F(dot, ".")                                           \
   F(dot_for, ".for")                                    \

src/messages.js

@@ -176,8 +176,8 @@ var kMessages = {
   symbol_to_string:              ["Cannot convert a Symbol value to a string"],
   symbol_to_primitive:           ["Cannot convert a Symbol wrapper object to a primitive value"],
   symbol_to_number:              ["Cannot convert a Symbol value to a number"],
-  invalid_module_path:           ["Module does not export '", "%0", "', or export is not itself a module"],
   module_export_undefined:       ["Export '", "%0", "' is not defined in module"],
+  duplicate_export:              ["Duplicate export of '", "%0", "'"],
   unexpected_super:              ["'super' keyword unexpected here"],
   extends_value_not_a_function:  ["Class extends value ", "%0", " is not a function or null"],
   prototype_parent_not_an_object: ["Class extends value does not have valid prototype property ", "%0"],

src/modules.cc

@@ -11,28 +11,27 @@
 namespace v8 {
 namespace internal {
 
-// ---------------------------------------------------------------------------
-// Addition.
 
-void ModuleDescriptor::Add(const AstRawString* name, Zone* zone, bool* ok) {
-  void* key = const_cast<AstRawString*>(name);
+void ModuleDescriptor::AddLocalExport(const AstRawString* export_name,
+                                      const AstRawString* local_name,
+                                      Zone* zone, bool* ok) {
+  void* key = const_cast<AstRawString*>(export_name);
 
-  ZoneHashMap** map = &exports_;
   ZoneAllocationPolicy allocator(zone);
 
-  if (*map == nullptr) {
-    *map = new (zone->New(sizeof(ZoneHashMap)))
+  if (exports_ == nullptr) {
+    exports_ = new (zone->New(sizeof(ZoneHashMap)))
         ZoneHashMap(ZoneHashMap::PointersMatch,
                     ZoneHashMap::kDefaultHashMapCapacity, allocator);
   }
 
   ZoneHashMap::Entry* p =
-      (*map)->Lookup(key, name->hash(), !IsFrozen(), allocator);
+      exports_->Lookup(key, export_name->hash(), !IsFrozen(), allocator);
   if (p == nullptr || p->value != nullptr) {
     *ok = false;
   }
 
-  p->value = key;
+  p->value = const_cast<AstRawString*>(local_name);
 }
 }
 }  // namespace v8::internal

src/modules.h

@@ -28,7 +28,8 @@ class ModuleDescriptor : public ZoneObject {
 
   // Add a name to the list of exports. If it already exists, or this descriptor
   // is frozen, that's an error.
-  void Add(const AstRawString* name, Zone* zone, bool* ok);
+  void AddLocalExport(const AstRawString* export_name,
+                      const AstRawString* local_name, Zone* zone, bool* ok);
 
   // Do not allow any further refinements, directly or through unification.
   void Freeze() { frozen_ = true; }
@@ -67,10 +68,14 @@ class ModuleDescriptor : public ZoneObject {
   class Iterator {
    public:
     bool done() const { return entry_ == NULL; }
-    const AstRawString* name() const {
+    const AstRawString* export_name() const {
       DCHECK(!done());
       return static_cast<const AstRawString*>(entry_->key);
     }
+    const AstRawString* local_name() const {
+      DCHECK(!done());
+      return static_cast<const AstRawString*>(entry_->value);
+    }
     void Advance() { entry_ = exports_->Next(entry_); }
 
    private:

src/parser.cc

@@ -1290,8 +1290,11 @@ Statement* Parser::ParseModule(bool* ok) {
   ModuleDescriptor* descriptor = scope->module();
   for (ModuleDescriptor::Iterator it = descriptor->iterator(); !it.done();
        it.Advance()) {
-    if (scope->LookupLocal(it.name()) == NULL) {
-      ParserTraits::ReportMessage("module_export_undefined", it.name());
+    if (scope->LookupLocal(it.local_name()) == NULL) {
+      // TODO(adamk): Pass both local_name and export_name once ParserTraits
+      // supports multiple arg error messages.
+      // Also try to report this at a better location.
+      ParserTraits::ReportMessage("module_export_undefined", it.local_name());
       *ok = false;
       return NULL;
     }
@@ -1312,7 +1315,9 @@ Literal* Parser::ParseModuleSpecifier(bool* ok) {
 }
 
 
-void* Parser::ParseExportClause(ZoneList<const AstRawString*>* names,
+void* Parser::ParseExportClause(ZoneList<const AstRawString*>* export_names,
+                                ZoneList<Scanner::Location>* export_locations,
+                                ZoneList<const AstRawString*>* local_names,
                                 Scanner::Location* reserved_loc, bool* ok) {
   // ExportClause :
   //   '{' '}'
@@ -1337,14 +1342,17 @@ void* Parser::ParseExportClause(ZoneList<const AstRawString*>* names,
         !Token::IsIdentifier(name_tok, STRICT, false)) {
       *reserved_loc = scanner()->location();
     }
-    const AstRawString* name = ParseIdentifierName(CHECK_OK);
-    names->Add(name, zone());
+    const AstRawString* local_name = ParseIdentifierName(CHECK_OK);
     const AstRawString* export_name = NULL;
     if (CheckContextualKeyword(CStrVector("as"))) {
       export_name = ParseIdentifierName(CHECK_OK);
     }
-    // TODO(ES6): Return the export_name as well as the name.
-    USE(export_name);
+    if (export_name == NULL) {
+      export_name = local_name;
+    }
+    export_names->Add(export_name, zone());
+    local_names->Add(local_name, zone());
+    export_locations->Add(scanner()->location(), zone());
     if (peek() == Token::RBRACE) break;
     Expect(Token::COMMA, CHECK_OK);
   }
@@ -1488,16 +1496,20 @@ Statement* Parser::ParseExportDefault(bool* ok) {
   //    'export' 'default' ClassDeclaration
   //    'export' 'default' AssignmentExpression[In] ';'
 
+  Expect(Token::DEFAULT, CHECK_OK);
+  Scanner::Location default_loc = scanner()->location();
+
+  ZoneList<const AstRawString*> names(1, zone());
   Statement* result = NULL;
   switch (peek()) {
     case Token::FUNCTION:
       // TODO(ES6): Support parsing anonymous function declarations here.
-      result = ParseFunctionDeclaration(NULL, CHECK_OK);
+      result = ParseFunctionDeclaration(&names, CHECK_OK);
       break;
 
     case Token::CLASS:
       // TODO(ES6): Support parsing anonymous class declarations here.
-      result = ParseClassDeclaration(NULL, CHECK_OK);
+      result = ParseClassDeclaration(&names, CHECK_OK);
       break;
 
     default: {
@@ -1509,7 +1521,20 @@ Statement* Parser::ParseExportDefault(bool* ok) {
     }
   }
 
-  // TODO(ES6): Add default export to scope_->module()
+  const AstRawString* default_string = ast_value_factory()->default_string();
+
+  DCHECK_LE(names.length(), 1);
+  if (names.length() == 1) {
+    scope_->module()->AddLocalExport(default_string, names.first(), zone(), ok);
+    if (!*ok) {
+      ParserTraits::ReportMessageAt(default_loc, "duplicate_export",
+                                    default_string);
+      return NULL;
+    }
+  } else {
+    // TODO(ES6): Assign result to a const binding with the name "*default*"
+    // and add an export entry with "*default*" as the local name.
+  }
 
   return result;
 }
@@ -1528,10 +1553,8 @@ Statement* Parser::ParseExportDeclaration(bool* ok) {
 
   Statement* result = NULL;
   ZoneList<const AstRawString*> names(1, zone());
-  bool is_export_from = false;
   switch (peek()) {
     case Token::DEFAULT:
-      Consume(Token::DEFAULT);
       return ParseExportDefault(ok);
 
     case Token::MUL: {
@@ -1539,12 +1562,9 @@ Statement* Parser::ParseExportDeclaration(bool* ok) {
       ExpectContextualKeyword(CStrVector("from"), CHECK_OK);
       Literal* module = ParseModuleSpecifier(CHECK_OK);
       ExpectSemicolon(CHECK_OK);
-      // TODO(ES6): Do something with the return value
-      // of ParseModuleSpecifier.
+      // TODO(ES6): scope_->module()->AddStarExport(...)
       USE(module);
-      is_export_from = true;
-      result = factory()->NewEmptyStatement(pos);
-      break;
+      return factory()->NewEmptyStatement(pos);
     }
 
     case Token::LBRACE: {
@@ -1560,13 +1580,14 @@ Statement* Parser::ParseExportDeclaration(bool* ok) {
       // encountered, and then throw a SyntaxError if we are in the
       // non-FromClause case.
       Scanner::Location reserved_loc = Scanner::Location::invalid();
-      ParseExportClause(&names, &reserved_loc, CHECK_OK);
+      ZoneList<const AstRawString*> export_names(1, zone());
+      ZoneList<Scanner::Location> export_locations(1, zone());
+      ZoneList<const AstRawString*> local_names(1, zone());
+      ParseExportClause(&export_names, &export_locations, &local_names,
+                        &reserved_loc, CHECK_OK);
+      Literal* indirect_export_module_specifier = NULL;
       if (CheckContextualKeyword(CStrVector("from"))) {
-        Literal* module = ParseModuleSpecifier(CHECK_OK);
-        // TODO(ES6): Do something with the return value
-        // of ParseModuleSpecifier.
-        USE(module);
-        is_export_from = true;
+        indirect_export_module_specifier = ParseModuleSpecifier(CHECK_OK);
       } else if (reserved_loc.IsValid()) {
         // No FromClause, so reserved words are invalid in ExportClause.
         *ok = false;
@@ -1574,8 +1595,25 @@ Statement* Parser::ParseExportDeclaration(bool* ok) {
         return NULL;
       }
       ExpectSemicolon(CHECK_OK);
-      result = factory()->NewEmptyStatement(pos);
-      break;
+      const int length = export_names.length();
+      DCHECK_EQ(length, local_names.length());
+      DCHECK_EQ(length, export_locations.length());
+      if (indirect_export_module_specifier == NULL) {
+        for (int i = 0; i < length; ++i) {
+          scope_->module()->AddLocalExport(export_names[i], local_names[i],
+                                           zone(), ok);
+          if (!*ok) {
+            ParserTraits::ReportMessageAt(export_locations[i],
+                                          "duplicate_export", export_names[i]);
+            return NULL;
+          }
+        }
+      } else {
+        for (int i = 0; i < length; ++i) {
+          // TODO(ES6): scope_->module()->AddIndirectExport(...);(
+        }
+      }
+      return factory()->NewEmptyStatement(pos);
     }
 
     case Token::FUNCTION:
@@ -1598,37 +1636,18 @@ Statement* Parser::ParseExportDeclaration(bool* ok) {
       return NULL;
   }
 
-  // Every export of a module may be assigned.
+  // Extract declared names into export declarations.
+  ModuleDescriptor* descriptor = scope_->module();
   for (int i = 0; i < names.length(); ++i) {
-    Variable* var = scope_->Lookup(names[i]);
-    if (var == NULL) {
-      // TODO(sigurds) This is an export that has no definition yet,
-      // not clear what to do in this case.
-      continue;
-    }
-    if (!IsImmutableVariableMode(var->mode())) {
-      var->set_maybe_assigned();
-    }
-  }
-
-  // TODO(ES6): Handle 'export from' once imports are properly implemented.
-  // For now we just drop such exports on the floor.
-  if (!is_export_from) {
-    // Extract declared names into export declarations and module descriptor.
-    ModuleDescriptor* descriptor = scope_->module();
-    for (int i = 0; i < names.length(); ++i) {
-      // TODO(adamk): Make early errors here provide the right error message
-      // (duplicate exported names).
-      descriptor->Add(names[i], zone(), CHECK_OK);
-      // TODO(rossberg): Rethink whether we actually need to store export
-      // declarations (for compilation?).
-      // ExportDeclaration* declaration =
-      //     factory()->NewExportDeclaration(proxy, scope_, position);
-      // scope_->AddDeclaration(declaration);
+    descriptor->AddLocalExport(names[i], names[i], zone(), ok);
+    if (!*ok) {
+      // TODO(adamk): Possibly report this error at the right place.
+      ParserTraits::ReportMessage("duplicate_export", names[i]);
+      return NULL;
     }
   }
 
-  DCHECK(result != NULL);
+  DCHECK_NOT_NULL(result);
   return result;
 }
 

src/parser.h

@@ -713,7 +713,9 @@ class Parser : public ParserBase<ParserTraits> {
   Statement* ParseImportDeclaration(bool* ok);
   Statement* ParseExportDeclaration(bool* ok);
   Statement* ParseExportDefault(bool* ok);
-  void* ParseExportClause(ZoneList<const AstRawString*>* names,
+  void* ParseExportClause(ZoneList<const AstRawString*>* export_names,
+                          ZoneList<Scanner::Location>* export_locations,
+                          ZoneList<const AstRawString*>* local_names,
                           Scanner::Location* reserved_loc, bool* ok);
   void* ParseNamedImports(ZoneList<const AstRawString*>* names, bool* ok);
   Statement* ParseStatement(ZoneList<const AstRawString*>* labels, bool* ok);

src/scopeinfo.cc

@@ -560,8 +560,8 @@ Handle<ModuleInfo> ModuleInfo::Create(Isolate* isolate,
   int i = 0;
   for (ModuleDescriptor::Iterator it = descriptor->iterator(); !it.done();
        it.Advance(), ++i) {
-    Variable* var = scope->LookupLocal(it.name());
-    info->set_name(i, *(it.name()->string()));
+    Variable* var = scope->LookupLocal(it.local_name());
+    info->set_name(i, *(it.export_name()->string()));
     info->set_mode(i, var->mode());
     DCHECK(var->index() >= 0);
     info->set_index(i, var->index());

test/cctest/cctest.status

@@ -63,9 +63,6 @@
   # are actually 13 * 38 * 5 * 128 = 316160 individual tests hidden here.
   'test-parsing/ParserSync': [PASS, NO_VARIANTS],
 
-  # Modules are busted
-  'test-parsing/ExportsMaybeAssigned': [SKIP],
-
   # This tests only the type system, so there is no point in running several
   # variants.
   'test-hydrogen-types/*': [PASS, NO_VARIANTS],

test/cctest/test-parsing.cc

@@ -3246,70 +3246,6 @@ TEST(IfArgumentsArrayAccessedThenParametersMaybeAssigned) {
 }
 
 
-TEST(ExportsMaybeAssigned) {
-  i::FLAG_use_strict = true;
-  i::FLAG_harmony_scoping = true;
-  i::FLAG_harmony_modules = true;
-
-  i::Isolate* isolate = CcTest::i_isolate();
-  i::Factory* factory = isolate->factory();
-  i::HandleScope scope(isolate);
-  LocalContext env;
-
-  const char* src =
-      "module A {"
-      "  export var x = 1;"
-      "  export function f() { return x };"
-      "  export const y = 2;"
-      "  module B {}"
-      "  export module C {}"
-      "};"
-      "A.f";
-
-  i::ScopedVector<char> program(Utf8LengthHelper(src) + 1);
-  i::SNPrintF(program, "%s", src);
-  i::Handle<i::String> source = factory->InternalizeUtf8String(program.start());
-  source->PrintOn(stdout);
-  printf("\n");
-  i::Zone zone;
-  v8::Local<v8::Value> v = CompileRun(src);
-  i::Handle<i::Object> o = v8::Utils::OpenHandle(*v);
-  i::Handle<i::JSFunction> f = i::Handle<i::JSFunction>::cast(o);
-  i::Context* context = f->context();
-  i::AstValueFactory avf(&zone, isolate->heap()->HashSeed());
-  avf.Internalize(isolate);
-
-  i::Scope* script_scope =
-      new (&zone) i::Scope(&zone, NULL, i::SCRIPT_SCOPE, &avf);
-  script_scope->Initialize();
-  i::Scope* s =
-      i::Scope::DeserializeScopeChain(isolate, &zone, context, script_scope);
-  DCHECK(s != script_scope);
-  const i::AstRawString* name_x = avf.GetOneByteString("x");
-  const i::AstRawString* name_f = avf.GetOneByteString("f");
-  const i::AstRawString* name_y = avf.GetOneByteString("y");
-  const i::AstRawString* name_B = avf.GetOneByteString("B");
-  const i::AstRawString* name_C = avf.GetOneByteString("C");
-
-  // Get result from h's function context (that is f's context)
-  i::Variable* var_x = s->Lookup(name_x);
-  CHECK(var_x != NULL);
-  CHECK(var_x->maybe_assigned() == i::kMaybeAssigned);
-  i::Variable* var_f = s->Lookup(name_f);
-  CHECK(var_f != NULL);
-  CHECK(var_f->maybe_assigned() == i::kMaybeAssigned);
-  i::Variable* var_y = s->Lookup(name_y);
-  CHECK(var_y != NULL);
-  CHECK(var_y->maybe_assigned() == i::kNotAssigned);
-  i::Variable* var_B = s->Lookup(name_B);
-  CHECK(var_B != NULL);
-  CHECK(var_B->maybe_assigned() == i::kNotAssigned);
-  i::Variable* var_C = s->Lookup(name_C);
-  CHECK(var_C != NULL);
-  CHECK(var_C->maybe_assigned() == i::kNotAssigned);
-}
-
-
 TEST(InnerAssignment) {
   i::Isolate* isolate = CcTest::i_isolate();
   i::Factory* factory = isolate->factory();
@@ -5108,6 +5044,7 @@ TEST(BasicImportExportParsing) {
       "export { yield } from 'm.js'",
       "export { static } from 'm.js'",
       "export { let } from 'm.js'",
+      "var a; export { a as b, a as c };",
 
       "import 'somemodule.js';",
       "import { } from 'm.js';",
@@ -5211,6 +5148,10 @@ TEST(ImportExportParsingErrors) {
       "export { arguments }",
       "export { arguments as foo }",
       "var a; export { a, a };",
+      "var a, b; export { a as b, b };",
+      "var a, b; export { a as c, b as c };",
+      "export default function f(){}; export default class C {};",
+      "export default function f(){}; var a; export { a as default };",
 
       "import from;",
       "import from 'm.js';",

test/message/export-duplicate-as.js

+// Copyright 2015 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// MODULE
+
+var a, b;
+export { a as c };
+export { a, b as c };

test/message/export-duplicate-as.out

+# Copyright 2015 the V8 project authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+*%(basename)s:9: SyntaxError: Duplicate export of 'c'
+export { a, b as c };
+                 ^
+SyntaxError: Duplicate export of 'c'

test/message/export-duplicate-default.js

+// Copyright 2015 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// MODULE
+
+export default function f() {};
+export default class C {};

test/message/export-duplicate-default.out

+# Copyright 2015 the V8 project authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+*%(basename)s:8: SyntaxError: Duplicate export of 'default'
+export default class C {};
+       ^^^^^^^
+SyntaxError: Duplicate export of 'default'

test/message/export-duplicate.js

+// Copyright 2015 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// MODULE
+
+var a, b;
+export { a };
+export { a, b };

test/message/export-duplicate.out

+# Copyright 2015 the V8 project authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+*%(basename)s:9: SyntaxError: Duplicate export of 'a'
+export { a, b };
+         ^
+SyntaxError: Duplicate export of 'a'

test/message/testcfg.py

@@ -36,6 +36,7 @@ from testrunner.objects import testcase
 
 FLAGS_PATTERN = re.compile(r"//\s+Flags:(.*)")
 INVALID_FLAGS = ["--enable-slow-asserts"]
+MODULE_PATTERN = re.compile(r"^// MODULE$", flags=re.MULTILINE)
 
 
 class MessageTestSuite(testsuite.TestSuite):
@@ -63,6 +64,8 @@ class MessageTestSuite(testsuite.TestSuite):
     for match in flags_match:
       result += match.strip().split()
     result += context.mode_flags
+    if MODULE_PATTERN.search(source):
+      result.append("--module")
     result = [x for x in result if x not in INVALID_FLAGS]
     result.append(os.path.join(self.root, testcase.path + ".js"))
     return testcase.flags + result