Bug 1529772 - Part 4: Implement ASI for fields that don't have initializers. r=jwalden
authorJason Orendorff <jorendorff@mozilla.com>
Mon, 15 Apr 2019 20:55:25 +0000
changeset 469567 27f0cd20a8b0
parent 469566 e771e63200c6
child 469568 cad2b29b79d2
push id35874
push userccoroiu@mozilla.com
push dateTue, 16 Apr 2019 04:04:58 +0000
treeherdermozilla-central@be3f40425b52 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwalden
bugs1529772
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1529772 - Part 4: Implement ASI for fields that don't have initializers. r=jwalden The only reason this wasn't already working is that propertyOrMethodName() relied on a Semi token to help it recognize a FieldDeclaration without an Initializer. This obviously can't work if the semicolon isn't there, so this patch makes Field the default case. That means the caller, classMember(), must be prepared for propertyOrMethodName() to succeed with PropertyType::Field when in fact maybe there's nothing but gibberish coming up; but we already handle that. Differential Revision: https://phabricator.services.mozilla.com/D26038
js/src/frontend/Parser.cpp
js/src/jit-test/tests/fields/error.js
js/src/tests/jstests.list
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -9708,20 +9708,21 @@ GeneralParser<ParseHandler, Unit>::prope
   //     get PropertyName       ==> PropertyType::Getter
   //     set PropertyName       ==> PropertyType::Setter
   //     PropertyName :         ==> PropertyType::Normal
   //     PropertyName           ==> see below
   //
   // In the last case, where there's not a `:` token to consume, we peek at
   // (but don't consume) the next token to decide how to set `*propType`.
   //
-  //     `=` or `;`             ==> PropertyType::Field (classes only)
-  //     `=`                    ==> PropertyType::CoverInitializedName
   //     `,` or `}`             ==> PropertyType::Shorthand
   //     `(`                    ==> PropertyType::Method
+  //     `=`, not in a class    ==> PropertyType::CoverInitializedName
+  //     '=', in a class        ==> PropertyType::Field
+  //     any token, in a class  ==> PropertyType::Field (ASI)
   //
   // The caller must check `*propType` and throw if whatever we parsed isn't
   // allowed here (for example, a getter in a destructuring pattern).
   //
   // This method does *not* match `static` (allowed in classes) or `...`
   // (allowed in object literals and patterns). The caller must take care of
   // those before calling this method.
 
@@ -9793,28 +9794,18 @@ GeneralParser<ParseHandler, Unit>::prope
     if (isGenerator || isAsync || isGetter || isSetter) {
       error(JSMSG_BAD_PROP_ID);
       return null();
     }
     *propType = PropertyType::Normal;
     return propName;
   }
 
-  if (propertyNameContext == PropertyNameInClass &&
-      (tt == TokenKind::Semi || tt == TokenKind::Assign)) {
-    if (isGenerator || isAsync || isGetter || isSetter) {
-      error(JSMSG_BAD_PROP_ID);
-      return null();
-    }
-    anyChars.ungetToken();
-    *propType = PropertyType::Field;
-    return propName;
-  }
-
-  if (TokenKindIsPossibleIdentifierName(ltok) &&
+  if (propertyNameContext != PropertyNameInClass &&
+      TokenKindIsPossibleIdentifierName(ltok) &&
       (tt == TokenKind::Comma || tt == TokenKind::RightCurly ||
        tt == TokenKind::Assign)) {
     if (isGenerator || isAsync || isGetter || isSetter) {
       error(JSMSG_BAD_PROP_ID);
       return null();
     }
 
     anyChars.ungetToken();
@@ -9837,16 +9828,26 @@ GeneralParser<ParseHandler, Unit>::prope
     } else if (isSetter) {
       *propType = PropertyType::Setter;
     } else {
       *propType = PropertyType::Method;
     }
     return propName;
   }
 
+  if (propertyNameContext == PropertyNameInClass) {
+    if (isGenerator || isAsync || isGetter || isSetter) {
+      error(JSMSG_BAD_PROP_ID);
+      return null();
+    }
+    anyChars.ungetToken();
+    *propType = PropertyType::Field;
+    return propName;
+  }
+
   error(JSMSG_COLON_AFTER_ID);
   return null();
 }
 
 template <class ParseHandler, typename Unit>
 typename ParseHandler::UnaryNodeType
 GeneralParser<ParseHandler, Unit>::computedPropertyName(
     YieldHandling yieldHandling, const Maybe<DeclarationKind>& maybeDecl,
--- a/js/src/jit-test/tests/fields/error.js
+++ b/js/src/jit-test/tests/fields/error.js
@@ -1,14 +1,14 @@
 // |jit-test| --enable-experimental-fields
 
 load(libdir + "asserts.js");
 
 let source = `class C {
-    x
+    x =
 }`;
 assertErrorMessage(() => Function(source), SyntaxError, /./);
 
 source = `class C {
     -2;
     -2 = 2;
 }`;
 assertErrorMessage(() => Function(source), SyntaxError, /./);
@@ -74,10 +74,52 @@ source = `class C {
 }`;
 assertErrorMessage(() => Function(source), SyntaxError, /./);
 
 source = `class C {
     x = sper();
 }`;
 eval(source);
 
+
+// The following test cases fail to parse because ASI does not happen if the
+// next token might be valid, even if it leads to a SyntaxError further down
+// the road.
+
+source = `class C {
+    x = 0
+    ["computedMethodName"](){}
+}`;
+assertThrowsInstanceOf(() => Function(source), SyntaxError);
+
+source = `class C {
+    x = 0
+    *f(){}
+}`;
+assertThrowsInstanceOf(() => Function(source), SyntaxError);
+
+
+// The following test cases fail to parse because ASI doesn't happen without a
+// newline.
+
+source = `class C { x y }`;
+assertThrowsInstanceOf(() => Function(source), SyntaxError);
+
+source = `class C { if var }  // identifiers that look like keywords`;
+assertThrowsInstanceOf(() => Function(source), SyntaxError);
+
+source = `class C { x = 1 y }`;
+assertThrowsInstanceOf(() => Function(source), SyntaxError);
+
+source = `class C { x async f() {} }`;
+assertThrowsInstanceOf(() => Function(source), SyntaxError);
+
+source = `class C { x static f() {} }`;
+assertThrowsInstanceOf(() => Function(source), SyntaxError);
+
+source = `class C { field1 static field2 }`;
+assertThrowsInstanceOf(() => Function(source), SyntaxError);
+
+source = `class C { x get f() {} }`;
+assertThrowsInstanceOf(() => Function(source), SyntaxError);
+
 if (typeof reportCompare === "function")
   reportCompare(true, true);
--- a/js/src/tests/jstests.list
+++ b/js/src/tests/jstests.list
@@ -553,70 +553,16 @@ skip script test262/built-ins/RegExp/pro
 # The Intl spec recently changed from canonicalizing locales using RFC 5646 to
 # using UTS 35.  This still needs to be implemented in bug 1522070.  See also
 # <https://github.com/tc39/ecma402/commit/77cd7634e6aef828b0d0a4d97dece622c5f8e01f>.
 skip script test262/intl402/Intl/getCanonicalLocales/non-iana-canon.js
 
 # https://bugzilla.mozilla.org/show_bug.cgi?id=1508684
 skip script test262/language/expressions/import.meta/syntax/invalid-assignment-target-update-expr.js
 
-# https://bugzilla.mozilla.org/show_bug.cgi?id=1529772
-skip script test262/language/expressions/class/elements/after-same-line-gen-literal-names-asi.js
-skip script test262/language/expressions/class/elements/after-same-line-method-literal-names-asi.js
-skip script test262/language/expressions/class/elements/after-same-line-static-async-gen-literal-names-asi.js
-skip script test262/language/expressions/class/elements/after-same-line-static-async-method-literal-names-asi.js
-skip script test262/language/expressions/class/elements/after-same-line-static-gen-literal-names-asi.js
-skip script test262/language/expressions/class/elements/after-same-line-static-method-literal-names-asi.js
-skip script test262/language/expressions/class/elements/multiple-definitions-computed-names.js
-skip script test262/language/expressions/class/elements/multiple-definitions-literal-names-asi.js
-skip script test262/language/expressions/class/elements/multiple-definitions-rs-field-identifier.js
-skip script test262/language/expressions/class/elements/multiple-stacked-definitions-computed-names.js
-skip script test262/language/expressions/class/elements/multiple-stacked-definitions-literal-names-asi.js
-skip script test262/language/expressions/class/elements/multiple-stacked-definitions-rs-field-identifier.js
-skip script test262/language/expressions/class/elements/new-no-sc-line-method-computed-names.js
-skip script test262/language/expressions/class/elements/new-no-sc-line-method-literal-names-asi.js
-skip script test262/language/expressions/class/elements/new-no-sc-line-method-rs-field-identifier.js
-skip script test262/language/expressions/class/elements/new-sc-line-gen-literal-names-asi.js
-skip script test262/language/expressions/class/elements/new-sc-line-method-literal-names-asi.js
-skip script test262/language/expressions/class/elements/regular-definitions-computed-names.js
-skip script test262/language/expressions/class/elements/regular-definitions-literal-names-asi.js
-skip script test262/language/expressions/class/elements/regular-definitions-rs-field-identifier.js
-skip script test262/language/expressions/class/elements/same-line-async-gen-literal-names-asi.js
-skip script test262/language/expressions/class/elements/same-line-async-method-literal-names-asi.js
-skip script test262/language/expressions/class/elements/same-line-gen-literal-names-asi.js
-skip script test262/language/expressions/class/elements/same-line-method-literal-names-asi.js
-skip script test262/language/expressions/class/elements/syntax/valid/grammar-fields-multi-line.js
-skip script test262/language/expressions/class/elements/wrapped-in-sc-literal-names-asi.js
-skip script test262/language/statements/class/elements/after-same-line-gen-literal-names-asi.js
-skip script test262/language/statements/class/elements/after-same-line-method-literal-names-asi.js
-skip script test262/language/statements/class/elements/after-same-line-static-async-gen-literal-names-asi.js
-skip script test262/language/statements/class/elements/after-same-line-static-async-method-literal-names-asi.js
-skip script test262/language/statements/class/elements/after-same-line-static-gen-literal-names-asi.js
-skip script test262/language/statements/class/elements/after-same-line-static-method-literal-names-asi.js
-skip script test262/language/statements/class/elements/multiple-definitions-computed-names.js
-skip script test262/language/statements/class/elements/multiple-definitions-literal-names-asi.js
-skip script test262/language/statements/class/elements/multiple-definitions-rs-field-identifier.js
-skip script test262/language/statements/class/elements/multiple-stacked-definitions-computed-names.js
-skip script test262/language/statements/class/elements/multiple-stacked-definitions-literal-names-asi.js
-skip script test262/language/statements/class/elements/multiple-stacked-definitions-rs-field-identifier.js
-skip script test262/language/statements/class/elements/new-no-sc-line-method-computed-names.js
-skip script test262/language/statements/class/elements/new-no-sc-line-method-literal-names-asi.js
-skip script test262/language/statements/class/elements/new-no-sc-line-method-rs-field-identifier.js
-skip script test262/language/statements/class/elements/new-sc-line-gen-literal-names-asi.js
-skip script test262/language/statements/class/elements/new-sc-line-method-literal-names-asi.js
-skip script test262/language/statements/class/elements/regular-definitions-computed-names.js
-skip script test262/language/statements/class/elements/regular-definitions-literal-names-asi.js
-skip script test262/language/statements/class/elements/regular-definitions-rs-field-identifier.js
-skip script test262/language/statements/class/elements/same-line-async-gen-literal-names-asi.js
-skip script test262/language/statements/class/elements/same-line-async-method-literal-names-asi.js
-skip script test262/language/statements/class/elements/same-line-gen-literal-names-asi.js
-skip script test262/language/statements/class/elements/same-line-method-literal-names-asi.js
-skip script test262/language/statements/class/elements/syntax/valid/grammar-fields-multi-line.js
-skip script test262/language/statements/class/elements/wrapped-in-sc-literal-names-asi.js
-
 # https://bugzilla.mozilla.org/show_bug.cgi?id=1542406
 skip script test262/language/expressions/class/elements/derived-cls-direct-eval-err-contains-supercall-1.js
 skip script test262/language/expressions/class/elements/derived-cls-direct-eval-err-contains-supercall-2.js
 skip script test262/language/expressions/class/elements/derived-cls-direct-eval-err-contains-supercall.js
 skip script test262/language/expressions/class/elements/derived-cls-direct-eval-err-contains-superproperty-1.js
 skip script test262/language/expressions/class/elements/derived-cls-direct-eval-err-contains-superproperty-2.js
 skip script test262/language/expressions/class/elements/derived-cls-indirect-eval-err-contains-supercall-1.js
 skip script test262/language/expressions/class/elements/derived-cls-indirect-eval-err-contains-supercall-2.js