Skip to content

Commit

Permalink
[counters] Add UseCounters for 'f() = 0' syntax
Browse files Browse the repository at this point in the history
This syntax was formerly legal per ECMAScript, but has been a
SyntaxError for some time now. V8 deviates from spec in that it
is instead a runtime error; we'd like to know if we can get
away with removing it (at least in sloppy mode) or if the spec
should be changed.

c.f. tc39/ecma262#257 (comment)

Also add self to authors file

BUG=v8:4480

Review-Url: https://codereview.chromium.org/2599253002
Cr-Commit-Position: refs/heads/master@{#41960}
  • Loading branch information
bakkot authored and Commit bot committed Dec 27, 2016
1 parent e0359c3 commit bf9e013
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 0 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ Julien Brianceau <jbriance@cisco.com>
JunHo Seo <sejunho@gmail.com>
Kang-Hao (Kenny) Lu <kennyluck@csail.mit.edu>
Karl Skomski <karl@skomski.com>
Kevin Gibbons <bakkot@gmail.com>
Luis Reis <luis.m.reis@gmail.com>
Luke Zarko <lukezarko@gmail.com>
Maciej Małecki <me@mmalecki.com>
Expand Down
2 changes: 2 additions & 0 deletions include/v8.h
Original file line number Diff line number Diff line change
Expand Up @@ -6488,6 +6488,8 @@ class V8_EXPORT Isolate {
kLegacyDateParser = 33,
kDefineGetterOrSetterWouldThrow = 34,
kFunctionConstructorReturnedUndefined = 35,
kAssigmentExpressionLHSIsCallInSloppy = 36,
kAssigmentExpressionLHSIsCallInStrict = 37,

// If you add new values here, you'll also need to update Chromium's:
// UseCounter.h, V8PerIsolateData.cpp, histograms.xml
Expand Down
5 changes: 5 additions & 0 deletions src/parsing/parser-base.h
Original file line number Diff line number Diff line change
Expand Up @@ -4331,7 +4331,12 @@ ParserBase<Impl>::CheckAndRewriteReferenceExpression(
}
if (expression->IsCall()) {
// If it is a call, make it a runtime error for legacy web compatibility.
// Bug: https://bugs.chromium.org/p/v8/issues/detail?id=4480
// Rewrite `expr' to `expr[throw ReferenceError]'.
impl()->CountUsage(
is_strict(language_mode())
? v8::Isolate::kAssigmentExpressionLHSIsCallInStrict
: v8::Isolate::kAssigmentExpressionLHSIsCallInSloppy);
ExpressionT error = impl()->NewThrowReferenceError(message, beg_pos);
return factory()->NewProperty(expression, error, beg_pos);
}
Expand Down
43 changes: 43 additions & 0 deletions test/cctest/test-usecounters.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,46 @@ TEST(DefineGetterSetterThrowUseCount) {
"a.__defineSetter__('b', ()=>{});");
CHECK_EQ(2, use_counts[v8::Isolate::kDefineGetterOrSetterWouldThrow]);
}

TEST(AssigmentExpressionLHSIsCall) {
v8::Isolate* isolate = CcTest::isolate();
v8::HandleScope scope(isolate);
LocalContext env;
int use_counts[v8::Isolate::kUseCounterFeatureCount] = {};
global_use_counts = use_counts;
CcTest::isolate()->SetUseCounterCallback(MockUseCounterCallback);

// AssignmentExpressions whose LHS is not a call do not increment counters
CompileRun("function f(){ a = 0; a()[b] = 0; }");
CHECK_EQ(0, use_counts[v8::Isolate::kAssigmentExpressionLHSIsCallInSloppy]);
CHECK_EQ(0, use_counts[v8::Isolate::kAssigmentExpressionLHSIsCallInStrict]);
CompileRun("function f(){ ++a; ++a()[b]; }");
CHECK_EQ(0, use_counts[v8::Isolate::kAssigmentExpressionLHSIsCallInSloppy]);
CHECK_EQ(0, use_counts[v8::Isolate::kAssigmentExpressionLHSIsCallInStrict]);
CompileRun("function f(){ 'use strict'; a = 0; a()[b] = 0; }");
CHECK_EQ(0, use_counts[v8::Isolate::kAssigmentExpressionLHSIsCallInSloppy]);
CHECK_EQ(0, use_counts[v8::Isolate::kAssigmentExpressionLHSIsCallInStrict]);
CompileRun("function f(){ 'use strict'; ++a; ++a()[b]; }");
CHECK_EQ(0, use_counts[v8::Isolate::kAssigmentExpressionLHSIsCallInSloppy]);
CHECK_EQ(0, use_counts[v8::Isolate::kAssigmentExpressionLHSIsCallInStrict]);

// AssignmentExpressions whose LHS is a call increment appropriate counters
CompileRun("function f(){ a() = 0; }");
CHECK_NE(0, use_counts[v8::Isolate::kAssigmentExpressionLHSIsCallInSloppy]);
CHECK_EQ(0, use_counts[v8::Isolate::kAssigmentExpressionLHSIsCallInStrict]);
use_counts[v8::Isolate::kAssigmentExpressionLHSIsCallInSloppy] = 0;
CompileRun("function f(){ 'use strict'; a() = 0; }");
CHECK_EQ(0, use_counts[v8::Isolate::kAssigmentExpressionLHSIsCallInSloppy]);
CHECK_NE(0, use_counts[v8::Isolate::kAssigmentExpressionLHSIsCallInStrict]);
use_counts[v8::Isolate::kAssigmentExpressionLHSIsCallInStrict] = 0;

// UpdateExpressions whose LHS is a call increment appropriate counters
CompileRun("function f(){ ++a(); }");
CHECK_NE(0, use_counts[v8::Isolate::kAssigmentExpressionLHSIsCallInSloppy]);
CHECK_EQ(0, use_counts[v8::Isolate::kAssigmentExpressionLHSIsCallInStrict]);
use_counts[v8::Isolate::kAssigmentExpressionLHSIsCallInSloppy] = 0;
CompileRun("function f(){ 'use strict'; ++a(); }");
CHECK_EQ(0, use_counts[v8::Isolate::kAssigmentExpressionLHSIsCallInSloppy]);
CHECK_NE(0, use_counts[v8::Isolate::kAssigmentExpressionLHSIsCallInStrict]);
use_counts[v8::Isolate::kAssigmentExpressionLHSIsCallInStrict] = 0;
}

0 comments on commit bf9e013

Please sign in to comment.