Reviewers: rossberg,
Message:
PTAL
Description:
[strong] Class constructor bodies cannot contain "use strong" directive
Since the constructor is also the class object itself, allowing it to
retroactively become a strong object would have unintuitive consequences
wrt the strength of the other functions of the class, and whether instances
would be considered instances of a strong class.
BUG=v8:3956
LOG=N
Please review this at https://codereview.chromium.org/1314203002/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+64, -30 lines):
M src/messages.h
M src/parser.cc
M src/preparser.cc
M test/cctest/test-parsing.cc
M test/mjsunit/strong/function-arity.js
Index: src/messages.h
diff --git a/src/messages.h b/src/messages.h
index
f08135bb16e17c5db3d713a3514a418f12b44536..5c9c32357d5f3b01547a881a207be365c04c6825
100644
--- a/src/messages.h
+++ b/src/messages.h
@@ -334,6 +334,8 @@ class CallSite {
T(StrictWith, "Strict mode code may not include a with
statement") \
T(StrongArguments,
\
"In strong mode, 'arguments' is deprecated, use '...args'
instead") \
+
T(StrongConstructorDirective,
\
+ "\"use strong\" directive is disallowed in class constructor
body") \
T(StrongConstructorReturnMisplaced,
\
"In strong mode, returning from a constructor before its
super " \
"constructor invocation or all assignments to 'this' is
deprecated") \
Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index
7ddd0fce0c804b2924ce422307c0e0b12110ed07..c0d26cc15835e235a6e6bdd04bcef2f886858011
100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -1321,6 +1321,25 @@ void*
Parser::ParseStatementList(ZoneList<Statement*>* body, int end_token,
token_loc.end_pos - token_loc.beg_pos ==
ast_value_factory()->use_strong_string()->length() + 2;
if (use_strict_found || use_strong_found) {
+ // Strong mode implies strict mode. If there are several "use
strict"
+ // / "use strong" directives, do the strict mode changes only
once.
+ if (is_sloppy(scope_->language_mode())) {
+ scope_->SetLanguageMode(static_cast<LanguageMode>(
+ scope_->language_mode() | STRICT));
+ }
+
+ if (use_strong_found) {
+ scope_->SetLanguageMode(static_cast<LanguageMode>(
+ scope_->language_mode() | STRONG));
+ if (i::IsConstructor(function_state_->kind())) {
+ // "use strong" cannot occur in a class constructor body, to
avoid
+ // unintuitive strong class object semantics.
+ ParserTraits::ReportMessageAt(
+ token_loc, MessageTemplate::kStrongConstructorDirective);
+ *ok = false;
+ return nullptr;
+ }
+ }
if (!scope_->HasSimpleParameters()) {
// TC39 deemed "use strict" directives to be an error when
occurring
// in the body of a function with non-simple parameter list, on
@@ -1334,18 +1353,6 @@ void*
Parser::ParseStatementList(ZoneList<Statement*>* body, int end_token,
*ok = false;
return nullptr;
}
-
- // Strong mode implies strict mode. If there are several "use
strict"
- // / "use strong" directives, do the strict mode changes only
once.
- if (is_sloppy(scope_->language_mode())) {
- scope_->SetLanguageMode(static_cast<LanguageMode>(
- scope_->language_mode() | STRICT));
- }
-
- if (use_strong_found) {
- scope_->SetLanguageMode(static_cast<LanguageMode>(
- scope_->language_mode() | STRONG));
- }
// Because declarations in strict eval code don't leak into the
scope
// of the eval call, it is likely that functions declared in
strict
// eval code will be used within the eval code, so lazy parsing
is
Index: src/preparser.cc
diff --git a/src/preparser.cc b/src/preparser.cc
index
7e75e78ef9c2e0753c26e6f8bf9f5303ddc02f70..1c8d9643f0c0f92b7edf21226a02f19400de2b50
100644
--- a/src/preparser.cc
+++ b/src/preparser.cc
@@ -262,6 +262,14 @@ void PreParser::ParseStatementList(int end_token,
bool* ok,
} else if (use_strong_found) {
scope_->SetLanguageMode(static_cast<LanguageMode>(
scope_->language_mode() | STRONG));
+ if (i::IsConstructor(function_state_->kind())) {
+ // "use strong" cannot occur in a class constructor body, to
avoid
+ // unintuitive strong class object semantics.
+ PreParserTraits::ReportMessageAt(
+ token_loc, MessageTemplate::kStrongConstructorDirective);
+ *ok = false;
+ return;
+ }
} else if (!statement.IsStringLiteral()) {
directive_prologue = false;
}
Index: test/cctest/test-parsing.cc
diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc
index
1ea550caab1dc65b1fe0f320531bb250748c604d..d34d4b5aa22e310f8f3646c1ca1d4dc27899d39b
100644
--- a/test/cctest/test-parsing.cc
+++ b/test/cctest/test-parsing.cc
@@ -6098,6 +6098,41 @@ TEST(StrongConstructorReturns) {
}
+TEST(StrongConstructorDirective) {
+ const char* context_data[][2] = {
+ {"class c { ", " }"},
+ {"(class c { ", " });"},
+ {"let a = (class c { ", " });"},
+ {NULL}
+ };
+
+ const char* error_data[] = {
+ "constructor() { \"use strong\" }",
+ "constructor(...rest) { \"use strong\" }",
+ "foo() {} constructor() { \"use strong\" }",
+ "foo(...rest) { \"use strict\" } constructor() { \"use strong\" }",
+ NULL
+ };
+
+ const char* success_data[] = {
+ "constructor() { \"use strict\" }",
+ "foo() { \"use strong\" }",
+ "foo() { \"use strong\" } constructor() {}",
+ NULL
+ };
+
+ static const ParserFlag always_flags[] = {
+ kAllowHarmonyRestParameters, kAllowHarmonySloppy,
kAllowHarmonySloppyLet,
+ kAllowStrongMode
+ };
+
+ RunParserSyncTest(context_data, error_data, kError, NULL, 0,
+ always_flags, arraysize(always_flags));
+ RunParserSyncTest(context_data, success_data, kSuccess, NULL, 0,
+ always_flags, arraysize(always_flags));
+}
+
+
TEST(StrongUndefinedLocal) {
const char* context_data[][2] = {{"", ""}, {NULL}};
Index: test/mjsunit/strong/function-arity.js
diff --git a/test/mjsunit/strong/function-arity.js
b/test/mjsunit/strong/function-arity.js
index
8eddc5e4a73bc916790d6d7162601e9746dfea38..64d76c1ac0fe65fbefdaf3a6db3157fb65a1f80d
100644
--- a/test/mjsunit/strong/function-arity.js
+++ b/test/mjsunit/strong/function-arity.js
@@ -176,10 +176,6 @@ function generateSpread(n) {
let defs = [
`'use strong';
class C { constructor(${genParams(parameterCount)}) {} }`,
- `'use strict';
- class C {
- constructor(${genParams(parameterCount, true)}) { 'use
strong'; }
- }`,
];
for (let def of defs) {
let calls = [
@@ -217,15 +213,6 @@ function generateSpread(n) {
super(${genArgs(argumentCount)});
}
}`,
- `'use strict';
- class B {
- constructor(${genParams(parameterCount, true)}) { 'use
strong'; }
- }
- class C extends B {
- constructor() {
- super(${genArgs(argumentCount)});
- }
- }`,
];
for (let def of defs) {
let code = `${def}; new C();`;
@@ -253,11 +240,6 @@ function generateSpread(n) {
constructor(${genParams(parameterCount)}) {}
}
class C extends B {}`,
- `'use strict';
- class B {
- constructor(${genParams(parameterCount, true)}) { 'use
strong'; }
- }
- class C extends B {}`,
];
for (let def of defs) {
let code = `${def}; new C(${genArgs(argumentCount)})`;
--
--
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.
