Bug 1196041 - Disallow getter/setter with expression closure in class declaration. r=efaust
authorTooru Fujisawa <arai_a@mac.com>
Thu, 03 Sep 2015 00:16:32 +0900
changeset 294637 caec7964e8f756acd30cf7ffe5999106cde6b350
parent 294636 9c1c2581ad6501c9a8a36920043856d46ec19c20
child 294638 2809ae70b476bd0098e6b6d6475f9834c28408fd
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersefaust
bugs1196041
milestone43.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 1196041 - Disallow getter/setter with expression closure in class declaration. r=efaust
js/src/frontend/ParseNode.h
js/src/frontend/Parser.cpp
js/src/frontend/Parser.h
js/src/tests/ecma_6/Class/geterNoExprClosure.js
--- a/js/src/frontend/ParseNode.h
+++ b/js/src/frontend/ParseNode.h
@@ -1711,25 +1711,39 @@ enum FunctionSyntaxKind
 {
     Expression,
     Statement,
     Arrow,
     Method,
     ClassConstructor,
     DerivedClassConstructor,
     Getter,
-    Setter
+    GetterNoExpressionClosure,
+    Setter,
+    SetterNoExpressionClosure
 };
 
 static inline bool
 IsConstructorKind(FunctionSyntaxKind kind)
 {
     return kind == ClassConstructor || kind == DerivedClassConstructor;
 }
 
+static inline bool
+IsGetterKind(FunctionSyntaxKind kind)
+{
+    return kind == Getter || kind == GetterNoExpressionClosure;
+}
+
+static inline bool
+IsSetterKind(FunctionSyntaxKind kind)
+{
+    return kind == Setter || kind == SetterNoExpressionClosure;
+}
+
 static inline ParseNode*
 FunctionArgsList(ParseNode* fn, unsigned* numFormals)
 {
     MOZ_ASSERT(fn->isKind(PNK_FUNCTION));
     ParseNode* argsBody = fn->pn_body;
     MOZ_ASSERT(argsBody->isKind(PNK_ARGSBODY));
     *numFormals = argsBody->pn_count;
     if (*numFormals > 0 && argsBody->last()->isKind(PNK_STATEMENTLIST))
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -1426,20 +1426,22 @@ Parser<ParseHandler>::newFunction(Handle
         allocKind = gc::AllocKind::FUNCTION_EXTENDED;
         break;
       case ClassConstructor:
       case DerivedClassConstructor:
         flags = JSFunction::INTERPRETED_CLASS_CONSTRUCTOR;
         allocKind = gc::AllocKind::FUNCTION_EXTENDED;
         break;
       case Getter:
+      case GetterNoExpressionClosure:
         flags = JSFunction::INTERPRETED_GETTER;
         allocKind = gc::AllocKind::FUNCTION_EXTENDED;
         break;
       case Setter:
+      case SetterNoExpressionClosure:
         flags = JSFunction::INTERPRETED_SETTER;
         allocKind = gc::AllocKind::FUNCTION_EXTENDED;
         break;
       default:
         flags = (generatorKind == NotGenerator
                  ? JSFunction::INTERPRETED_NORMAL
                  : JSFunction::INTERPRETED_GENERATOR);
     }
@@ -1855,17 +1857,17 @@ Parser<ParseHandler>::functionArguments(
         if (!matched)
             hasArguments = true;
     }
     if (hasArguments) {
         bool hasDefaults = false;
         Node duplicatedArg = null();
         bool disallowDuplicateArgs = kind == Arrow || kind == Method || kind == ClassConstructor;
 
-        if (kind == Getter) {
+        if (IsGetterKind(kind)) {
             report(ParseError, false, null(), JSMSG_ACCESSOR_WRONG_ARGS, "getter", "no", "s");
             return false;
         }
 
         while (true) {
             if (*hasRest) {
                 report(ParseError, false, null(), JSMSG_PARAMETER_AFTER_REST);
                 return false;
@@ -1921,17 +1923,17 @@ Parser<ParseHandler>::functionArguments(
               case TOK_YIELD:
                 if (!checkYieldNameValidity())
                     return false;
                 MOZ_ASSERT(yieldHandling == YieldIsName);
                 goto TOK_NAME;
 
               case TOK_TRIPLEDOT:
               {
-                if (kind == Setter) {
+                if (IsSetterKind(kind)) {
                     report(ParseError, false, null(),
                            JSMSG_ACCESSOR_WRONG_ARGS, "setter", "one", "");
                     return false;
                 }
                 *hasRest = true;
                 if (!tokenStream.getToken(&tt))
                     return false;
                 // FIXME: This fails to handle a rest parameter named |yield|
@@ -1998,44 +2000,44 @@ Parser<ParseHandler>::functionArguments(
                 }
                 Node def_expr = assignExprWithoutYield(yieldHandling, JSMSG_YIELD_IN_DEFAULT);
                 if (!def_expr)
                     return false;
                 if (!handler.setLastFunctionArgumentDefault(funcpn, def_expr))
                     return false;
             }
 
-            if (parenFreeArrow || kind == Setter)
+            if (parenFreeArrow || IsSetterKind(kind))
                 break;
 
             if (!tokenStream.matchToken(&matched, TOK_COMMA))
                 return false;
             if (!matched)
                 break;
         }
 
         if (!parenFreeArrow) {
             TokenKind tt;
             if (!tokenStream.getToken(&tt))
                 return false;
             if (tt != TOK_RP) {
-                if (kind == Setter) {
+                if (IsSetterKind(kind)) {
                     report(ParseError, false, null(),
                            JSMSG_ACCESSOR_WRONG_ARGS, "setter", "one", "");
                     return false;
                 }
 
                 report(ParseError, false, null(), JSMSG_PAREN_AFTER_FORMAL);
                 return false;
             }
         }
 
         if (!hasDefaults)
             funbox->length = pc->numArgs() - *hasRest;
-    } else if (kind == Setter) {
+    } else if (IsSetterKind(kind)) {
         report(ParseError, false, null(), JSMSG_ACCESSOR_WRONG_ARGS, "setter", "one", "");
         return false;
     }
 
     return true;
 }
 
 template <>
@@ -2776,17 +2778,19 @@ Parser<ParseHandler>::functionArgsAndBod
     }
 
     // Parse the function body.
     FunctionBodyType bodyType = StatementListBody;
     TokenKind tt;
     if (!tokenStream.getToken(&tt, TokenStream::Operand))
         return false;
     if (tt != TOK_LC) {
-        if (funbox->isStarGenerator() || kind == Method || IsConstructorKind(kind)) {
+        if (funbox->isStarGenerator() || kind == Method ||
+            kind == GetterNoExpressionClosure || kind == SetterNoExpressionClosure ||
+            IsConstructorKind(kind)) {
             report(ParseError, false, null(), JSMSG_CURLY_BEFORE_BODY);
             return false;
         }
 
         if (kind != Arrow) {
 #if JS_HAS_EXPR_CLOSURES
             addTelemetry(JSCompartment::DeprecatedExpressionClosure);
 #else
@@ -6314,18 +6318,20 @@ Parser<ParseHandler>::debuggerStatement(
     return handler.newDebuggerStatement(p);
 }
 
 static JSOp
 JSOpFromPropertyType(PropertyType propType)
 {
     switch (propType) {
       case PropertyType::Getter:
+      case PropertyType::GetterNoExpressionClosure:
         return JSOP_INITPROP_GETTER;
       case PropertyType::Setter:
+      case PropertyType::SetterNoExpressionClosure:
         return JSOP_INITPROP_SETTER;
       case PropertyType::Normal:
       case PropertyType::Method:
       case PropertyType::GeneratorMethod:
       case PropertyType::Constructor:
       case PropertyType::DerivedConstructor:
         return JSOP_INITPROP;
       default:
@@ -6334,18 +6340,22 @@ JSOpFromPropertyType(PropertyType propTy
 }
 
 static FunctionSyntaxKind
 FunctionSyntaxKindFromPropertyType(PropertyType propType)
 {
     switch (propType) {
       case PropertyType::Getter:
         return Getter;
+      case PropertyType::GetterNoExpressionClosure:
+        return GetterNoExpressionClosure;
       case PropertyType::Setter:
         return Setter;
+      case PropertyType::SetterNoExpressionClosure:
+        return SetterNoExpressionClosure;
       case PropertyType::Method:
         return Method;
       case PropertyType::GeneratorMethod:
         return Method;
       case PropertyType::Constructor:
         return ClassConstructor;
       case PropertyType::DerivedConstructor:
         return DerivedClassConstructor;
@@ -6469,16 +6479,20 @@ Parser<FullParseHandler>::classDefinitio
         if (propType != PropertyType::Getter && propType != PropertyType::Setter &&
             propType != PropertyType::Method && propType != PropertyType::GeneratorMethod &&
             propType != PropertyType::Constructor && propType != PropertyType::DerivedConstructor)
         {
             report(ParseError, false, null(), JSMSG_BAD_METHOD_DEF);
             return null();
         }
 
+        if (propType == PropertyType::Getter)
+            propType = PropertyType::GetterNoExpressionClosure;
+        if (propType == PropertyType::Setter)
+            propType = PropertyType::SetterNoExpressionClosure;
         if (!isStatic && propAtom == context->names().constructor) {
             if (propType != PropertyType::Method) {
                 report(ParseError, false, propName, JSMSG_BAD_METHOD_DEF);
                 return null();
             }
             if (seenConstructor) {
                 report(ParseError, false, propName, JSMSG_DUPLICATE_PROPERTY, "constructor");
                 return null();
@@ -6489,18 +6503,18 @@ Parser<FullParseHandler>::classDefinitio
             report(ParseError, false, propName, JSMSG_BAD_METHOD_DEF);
             return null();
         }
 
         // FIXME: Implement ES6 function "name" property semantics
         // (bug 883377).
         RootedPropertyName funName(context);
         switch (propType) {
-          case PropertyType::Getter:
-          case PropertyType::Setter:
+          case PropertyType::GetterNoExpressionClosure:
+          case PropertyType::SetterNoExpressionClosure:
             funName = nullptr;
             break;
           case PropertyType::Constructor:
           case PropertyType::DerivedConstructor:
             funName = name;
             break;
           default:
             if (tokenStream.isCurrentTokenType(TOK_NAME))
--- a/js/src/frontend/Parser.h
+++ b/js/src/frontend/Parser.h
@@ -353,17 +353,19 @@ struct BindData;
 class CompExprTransplanter;
 
 enum VarContext { HoistVars, DontHoistVars };
 enum PropListType { ObjectLiteral, ClassBody, DerivedClassBody };
 enum class PropertyType {
     Normal,
     Shorthand,
     Getter,
+    GetterNoExpressionClosure,
     Setter,
+    SetterNoExpressionClosure,
     Method,
     GeneratorMethod,
     Constructor,
     DerivedConstructor
 };
 
 // Specify a value for an ES6 grammar parametrization.  We have no enum for
 // [Return] because its behavior is exactly equivalent to checking whether
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/Class/geterNoExprClosure.js
@@ -0,0 +1,24 @@
+// getter/setter with expression closure is allowed only in object literal.
+
+function test() {
+  assertThrowsInstanceOf(() => eval(`
+class foo {
+  constructor() {}
+
+  get a() 1
+}
+`), SyntaxError);
+  assertThrowsInstanceOf(() => eval(`
+class foo {
+  constructor() {}
+
+  set a(v) 1
+}
+`), SyntaxError);
+}
+
+if (classesEnabled())
+    test();
+
+if (typeof reportCompare === 'function')
+    reportCompare(0,0,"OK");