Skip to content

Commit

Permalink
Revert "Reland "Let all early errors be SyntaxErrors.""
Browse files Browse the repository at this point in the history
This reverts commit 89d93e3.

Reason for revert: Breaks layout tests: https://ci.chromium.org/p/v8/builders/ci/V8-Blink%20Linux%2064/32929

Original change's description:
> Reland "Let all early errors be SyntaxErrors."
> 
> This is a reland of 99fd5b9 which includes a missed update to
> test/test262/test262.status.
> 
> Implement the spec change from the following TC39 PR:
> tc39/ecma262#1527
> 
> Bug: v8:9326
> Change-Id: Ie3aac60db550e90fb648fc30886a05419fa41afe
> TBR: [email protected]
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1682989
> Reviewed-by: Sathya Gunasekaran <[email protected]>
> Commit-Queue: Sathya Gunasekaran <[email protected]>
> Cr-Commit-Position: refs/heads/master@{#62500}

[email protected],[email protected],[email protected],[email protected]

Change-Id: Ia56dcda6780a2b1249749e1e7978b35b5e33fbcf
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:9326
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1687678
Reviewed-by: Clemens Hammacher <[email protected]>
Commit-Queue: Clemens Hammacher <[email protected]>
Cr-Commit-Position: refs/heads/master@{#62509}
  • Loading branch information
backes authored and Commit Bot committed Jul 3, 2019
1 parent e937bc5 commit 356b460
Show file tree
Hide file tree
Showing 27 changed files with 325 additions and 296 deletions.
1 change: 0 additions & 1 deletion AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ Rob Wu <[email protected]>
Robert Meijer <[email protected]>
Robert Mustacchi <[email protected]>
Robert Nagy <[email protected]>
Ross Kirsling <[email protected]>
Ruben Bridgewater <[email protected]>
Ryan Dahl <[email protected]>
Sakthipriyan Vairamani (thefourtheye) <[email protected]>
Expand Down
2 changes: 1 addition & 1 deletion DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ deps = {
'v8/test/mozilla/data':
Var('chromium_url') + '/v8/deps/third_party/mozilla-tests.git' + '@' + 'f6c578a10ea707b1a8ab0b88943fe5115ce2b9be',
'v8/test/test262/data':
Var('chromium_url') + '/external/github.com/tc39/test262.git' + '@' + '079b004ac418db8ddcd9134d7cf36b0f5c4a6110',
Var('chromium_url') + '/external/github.com/tc39/test262.git' + '@' + '49eee8bf9d69e3e137e9a196f6e0906a7c6a2cd4',
'v8/test/test262/harness':
Var('chromium_url') + '/external/github.com/test262-utils/test262-harness-py.git' + '@' + '4555345a943d0c99a9461182705543fb171dda4b',
'v8/third_party/qemu-linux-x64': {
Expand Down
3 changes: 2 additions & 1 deletion src/ast/scopes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2469,7 +2469,8 @@ bool ClassScope::ResolvePrivateNames(ParseInfo* info) {
Scanner::Location loc = proxy->location();
info->pending_error_handler()->ReportMessageAt(
loc.beg_pos, loc.end_pos,
MessageTemplate::kInvalidPrivateFieldResolution, proxy->raw_name());
MessageTemplate::kInvalidPrivateFieldResolution, proxy->raw_name(),
kSyntaxError);
return false;
} else {
var->set_is_used();
Expand Down
2 changes: 2 additions & 0 deletions src/common/globals.h
Original file line number Diff line number Diff line change
Expand Up @@ -1170,6 +1170,8 @@ enum MaybeAssignedFlag : uint8_t { kNotAssigned, kMaybeAssigned };

enum RequiresBrandCheckFlag : uint8_t { kNoBrandCheck, kRequiresBrandCheck };

enum ParseErrorType { kSyntaxError = 0, kReferenceError = 1 };

enum class InterpreterPushArgsMode : unsigned {
kArrayFunction,
kWithFinalSpread,
Expand Down
3 changes: 2 additions & 1 deletion src/parsing/expression-scope.h
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,8 @@ class ExpressionParsingScope : public ExpressionScope<Types> {
}
this->mark_verified();
return this->parser()->RewriteInvalidReferenceExpression(
expression, beg_pos, end_pos, MessageTemplate::kInvalidLhsInFor);
expression, beg_pos, end_pos, MessageTemplate::kInvalidLhsInFor,
kSyntaxError);
}

void RecordExpressionError(const Scanner::Location& loc,
Expand Down
40 changes: 23 additions & 17 deletions src/parsing/parser-base.h
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,7 @@ class ParserBase {

if (scanner()->current_token() == Token::AWAIT && !is_async_function()) {
ReportMessageAt(scanner()->location(),
MessageTemplate::kAwaitNotInAsyncFunction);
MessageTemplate::kAwaitNotInAsyncFunction, kSyntaxError);
return;
}

Expand Down Expand Up @@ -935,19 +935,21 @@ class ParserBase {
V8_NOINLINE void ReportMessage(MessageTemplate message) {
Scanner::Location source_location = scanner()->location();
impl()->ReportMessageAt(source_location, message,
static_cast<const char*>(nullptr));
static_cast<const char*>(nullptr), kSyntaxError);
}

template <typename T>
V8_NOINLINE void ReportMessage(MessageTemplate message, T arg) {
V8_NOINLINE void ReportMessage(MessageTemplate message, T arg,
ParseErrorType error_type = kSyntaxError) {
Scanner::Location source_location = scanner()->location();
impl()->ReportMessageAt(source_location, message, arg);
impl()->ReportMessageAt(source_location, message, arg, error_type);
}

V8_NOINLINE void ReportMessageAt(Scanner::Location location,
MessageTemplate message) {
MessageTemplate message,
ParseErrorType error_type) {
impl()->ReportMessageAt(location, message,
static_cast<const char*>(nullptr));
static_cast<const char*>(nullptr), error_type);
}

V8_NOINLINE void ReportUnexpectedToken(Token::Value token);
Expand Down Expand Up @@ -1216,9 +1218,9 @@ class ParserBase {
// Checks if the expression is a valid reference expression (e.g., on the
// left-hand side of assignments). Although ruled out by ECMA as early errors,
// we allow calls for web compatibility and rewrite them to a runtime throw.
ExpressionT RewriteInvalidReferenceExpression(ExpressionT expression,
int beg_pos, int end_pos,
MessageTemplate message);
ExpressionT RewriteInvalidReferenceExpression(
ExpressionT expression, int beg_pos, int end_pos, MessageTemplate message,
ParseErrorType type = kReferenceError);

bool IsValidReferenceExpression(ExpressionT expression);

Expand Down Expand Up @@ -1570,7 +1572,8 @@ ParserBase<Impl>::ParsePropertyOrPrivatePropertyName() {
if (class_scope == nullptr) {
impl()->ReportMessageAt(Scanner::Location(pos, pos + 1),
MessageTemplate::kInvalidPrivateFieldResolution,
impl()->GetRawNameFromIdentifier(name));
impl()->GetRawNameFromIdentifier(name),
kSyntaxError);
return impl()->FailureExpression();
}
key = impl()->ExpressionFromPrivateName(class_scope, name, pos);
Expand Down Expand Up @@ -2658,11 +2661,13 @@ ParserBase<Impl>::ParseAssignmentExpressionCoverGrammar() {
impl()->ReportMessageAt(loc,
MessageTemplate::kInvalidDestructuringTarget);
} else {
// Syntax Error if LHS is neither object literal nor an array literal
// Reference Error if LHS is neither object literal nor an array literal
// (Parenthesized literals are
// CoverParenthesizedExpressionAndArrowParameterList).
// #sec-assignment-operators-static-semantics-early-errors
impl()->ReportMessageAt(loc, MessageTemplate::kInvalidLhsInAssignment);
impl()->ReportMessageAt(loc, MessageTemplate::kInvalidLhsInAssignment,
static_cast<const char*>(nullptr),
kReferenceError);
}
}
expression_scope()->ValidateAsPattern(expression, lhs_beg_pos,
Expand Down Expand Up @@ -4301,7 +4306,7 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseClassLiteral(
impl()->ReportMessageAt(Scanner::Location(unresolvable->position(),
unresolvable->position() + 1),
MessageTemplate::kInvalidPrivateFieldResolution,
unresolvable->raw_name());
unresolvable->raw_name(), kSyntaxError);
return impl()->FailureExpression();
}

Expand Down Expand Up @@ -4452,14 +4457,15 @@ template <typename Impl>
typename ParserBase<Impl>::ExpressionT
ParserBase<Impl>::RewriteInvalidReferenceExpression(ExpressionT expression,
int beg_pos, int end_pos,
MessageTemplate message) {
MessageTemplate message,
ParseErrorType type) {
DCHECK(!IsValidReferenceExpression(expression));
if (impl()->IsIdentifier(expression)) {
DCHECK(is_strict(language_mode()));
DCHECK(impl()->IsEvalOrArguments(impl()->AsIdentifier(expression)));

ReportMessageAt(Scanner::Location(beg_pos, end_pos),
MessageTemplate::kStrictEvalArguments);
MessageTemplate::kStrictEvalArguments, kSyntaxError);
return impl()->FailureExpression();
}
if (expression->IsCall() && !expression->AsCall()->is_tagged_template()) {
Expand All @@ -4476,7 +4482,7 @@ ParserBase<Impl>::RewriteInvalidReferenceExpression(ExpressionT expression,
ExpressionT error = impl()->NewThrowReferenceError(message, beg_pos);
return factory()->NewProperty(expression, error, beg_pos);
}
ReportMessageAt(Scanner::Location(beg_pos, end_pos), message);
ReportMessageAt(Scanner::Location(beg_pos, end_pos), message, type);
return impl()->FailureExpression();
}

Expand Down Expand Up @@ -4570,7 +4576,7 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseV8Intrinsic() {

if (has_spread) {
ReportMessageAt(Scanner::Location(pos, position()),
MessageTemplate::kIntrinsicWithSpread);
MessageTemplate::kIntrinsicWithSpread, kSyntaxError);
return impl()->FailureExpression();
}

Expand Down
16 changes: 10 additions & 6 deletions src/parsing/parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -696,9 +696,11 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {

// Reporting errors.
void ReportMessageAt(Scanner::Location source_location,
MessageTemplate message, const char* arg = nullptr) {
pending_error_handler()->ReportMessageAt(
source_location.beg_pos, source_location.end_pos, message, arg);
MessageTemplate message, const char* arg = nullptr,
ParseErrorType error_type = kSyntaxError) {
pending_error_handler()->ReportMessageAt(source_location.beg_pos,
source_location.end_pos, message,
arg, error_type);
scanner_.set_parser_error();
}

Expand All @@ -707,9 +709,11 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
V8_INLINE void ReportUnidentifiableError() { UNREACHABLE(); }

void ReportMessageAt(Scanner::Location source_location,
MessageTemplate message, const AstRawString* arg) {
pending_error_handler()->ReportMessageAt(
source_location.beg_pos, source_location.end_pos, message, arg);
MessageTemplate message, const AstRawString* arg,
ParseErrorType error_type = kSyntaxError) {
pending_error_handler()->ReportMessageAt(source_location.beg_pos,
source_location.end_pos, message,
arg, error_type);
scanner_.set_parser_error();
}

Expand Down
29 changes: 19 additions & 10 deletions src/parsing/pending-compilation-error-handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,26 +30,26 @@ MessageLocation PendingCompilationErrorHandler::MessageDetails::GetLocation(
return MessageLocation(script, start_position_, end_position_);
}

void PendingCompilationErrorHandler::ReportMessageAt(int start_position,
int end_position,
MessageTemplate message,
const char* arg) {
void PendingCompilationErrorHandler::ReportMessageAt(
int start_position, int end_position, MessageTemplate message,
const char* arg, ParseErrorType error_type) {
if (has_pending_error_) return;
has_pending_error_ = true;

error_details_ =
MessageDetails(start_position, end_position, message, nullptr, arg);
error_type_ = error_type;
}

void PendingCompilationErrorHandler::ReportMessageAt(int start_position,
int end_position,
MessageTemplate message,
const AstRawString* arg) {
void PendingCompilationErrorHandler::ReportMessageAt(
int start_position, int end_position, MessageTemplate message,
const AstRawString* arg, ParseErrorType error_type) {
if (has_pending_error_) return;
has_pending_error_ = true;

error_details_ =
MessageDetails(start_position, end_position, message, arg, nullptr);
error_type_ = error_type;
}

void PendingCompilationErrorHandler::ReportWarningAt(int start_position,
Expand Down Expand Up @@ -97,8 +97,17 @@ void PendingCompilationErrorHandler::ThrowPendingError(Isolate* isolate,
isolate->debug()->OnCompileError(script);

Factory* factory = isolate->factory();
Handle<Object> error =
factory->NewSyntaxError(error_details_.message(), argument);
Handle<Object> error;
switch (error_type_) {
case kReferenceError:
error = factory->NewReferenceError(error_details_.message(), argument);
break;
case kSyntaxError:
error = factory->NewSyntaxError(error_details_.message(), argument);
break;
default:
UNREACHABLE();
}

if (!error->IsJSObject()) {
isolate->Throw(*error, &location);
Expand Down
11 changes: 8 additions & 3 deletions src/parsing/pending-compilation-error-handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,17 @@ class Script;
class PendingCompilationErrorHandler {
public:
PendingCompilationErrorHandler()
: has_pending_error_(false), stack_overflow_(false) {}
: has_pending_error_(false),
stack_overflow_(false),
error_type_(kSyntaxError) {}

void ReportMessageAt(int start_position, int end_position,
MessageTemplate message, const char* arg = nullptr);
MessageTemplate message, const char* arg = nullptr,
ParseErrorType error_type = kSyntaxError);

void ReportMessageAt(int start_position, int end_position,
MessageTemplate message, const AstRawString* arg);
MessageTemplate message, const AstRawString* arg,
ParseErrorType error_type = kSyntaxError);

void ReportWarningAt(int start_position, int end_position,
MessageTemplate message, const char* arg = nullptr);
Expand Down Expand Up @@ -106,6 +110,7 @@ class PendingCompilationErrorHandler {
bool unidentifiable_error_ = false;

MessageDetails error_details_;
ParseErrorType error_type_;

std::forward_list<MessageDetails> warning_messages_;

Expand Down
19 changes: 12 additions & 7 deletions src/parsing/preparser.h
Original file line number Diff line number Diff line change
Expand Up @@ -1483,9 +1483,11 @@ class PreParser : public ParserBase<PreParser> {

// Reporting errors.
void ReportMessageAt(Scanner::Location source_location,
MessageTemplate message, const char* arg = nullptr) {
pending_error_handler()->ReportMessageAt(
source_location.beg_pos, source_location.end_pos, message, arg);
MessageTemplate message, const char* arg = nullptr,
ParseErrorType error_type = kSyntaxError) {
pending_error_handler()->ReportMessageAt(source_location.beg_pos,
source_location.end_pos, message,
arg, error_type);
scanner()->set_parser_error();
}

Expand All @@ -1496,14 +1498,17 @@ class PreParser : public ParserBase<PreParser> {

V8_INLINE void ReportMessageAt(Scanner::Location source_location,
MessageTemplate message,
const PreParserIdentifier& arg) {
const PreParserIdentifier& arg,
ParseErrorType error_type = kSyntaxError) {
UNREACHABLE();
}

void ReportMessageAt(Scanner::Location source_location,
MessageTemplate message, const AstRawString* arg) {
pending_error_handler()->ReportMessageAt(
source_location.beg_pos, source_location.end_pos, message, arg);
MessageTemplate message, const AstRawString* arg,
ParseErrorType error_type = kSyntaxError) {
pending_error_handler()->ReportMessageAt(source_location.beg_pos,
source_location.end_pos, message,
arg, error_type);
scanner()->set_parser_error();
}

Expand Down
4 changes: 2 additions & 2 deletions test/message/fail/new-target-assignment.out
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
*%(basename)s:5: SyntaxError: Invalid left-hand side in assignment
*%(basename)s:5: ReferenceError: Invalid left-hand side in assignment
function f() { new.target = 5 }
^^^^^^^^^^
SyntaxError: Invalid left-hand side in assignment
ReferenceError: Invalid left-hand side in assignment
4 changes: 2 additions & 2 deletions test/message/fail/new-target-postfix-op.out
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
*%(basename)s:5: SyntaxError: Invalid left-hand side expression in postfix operation
*%(basename)s:5: ReferenceError: Invalid left-hand side expression in postfix operation
function f() { new.target++ }
^^^^^^^^^^
SyntaxError: Invalid left-hand side expression in postfix operation
ReferenceError: Invalid left-hand side expression in postfix operation
4 changes: 2 additions & 2 deletions test/message/fail/new-target-prefix-op.out
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
*%(basename)s:5: SyntaxError: Invalid left-hand side expression in prefix operation
*%(basename)s:5: ReferenceError: Invalid left-hand side expression in prefix operation
function f() { ++new.target }
^^^^^^^^^^
SyntaxError: Invalid left-hand side expression in prefix operation
ReferenceError: Invalid left-hand side expression in prefix operation
14 changes: 7 additions & 7 deletions test/mjsunit/es6/new-target.js
Original file line number Diff line number Diff line change
Expand Up @@ -401,13 +401,13 @@ function get_new_target() { return new.target; }


(function TestEarlyErrors() {
assertThrows(function() { Function("new.target = 42"); }, SyntaxError);
assertThrows(function() { Function("var foo = 1; new.target = foo = 42"); }, SyntaxError);
assertThrows(function() { Function("var foo = 1; foo = new.target = 42"); }, SyntaxError);
assertThrows(function() { Function("new.target--"); }, SyntaxError);
assertThrows(function() { Function("--new.target"); }, SyntaxError);
assertThrows(function() { Function("(new.target)++"); }, SyntaxError);
assertThrows(function() { Function("++(new.target)"); }, SyntaxError);
assertThrows(function() { Function("new.target = 42"); }, ReferenceError);
assertThrows(function() { Function("var foo = 1; new.target = foo = 42"); }, ReferenceError);
assertThrows(function() { Function("var foo = 1; foo = new.target = 42"); }, ReferenceError);
assertThrows(function() { Function("new.target--"); }, ReferenceError);
assertThrows(function() { Function("--new.target"); }, ReferenceError);
assertThrows(function() { Function("(new.target)++"); }, ReferenceError);
assertThrows(function() { Function("++(new.target)"); }, ReferenceError);
assertThrows(function() { Function("for (new.target of {});"); }, SyntaxError);
})();

Expand Down
20 changes: 10 additions & 10 deletions test/mjsunit/es6/templates.js
Original file line number Diff line number Diff line change
Expand Up @@ -725,20 +725,20 @@ var global = this;
(function testTaggedTemplateInvalidAssignmentTargetStrict() {
"use strict";
function f() {}
assertThrows(() => Function("++f`foo`"), SyntaxError);
assertThrows(() => Function("f`foo`++"), SyntaxError);
assertThrows(() => Function("--f`foo`"), SyntaxError);
assertThrows(() => Function("f`foo`--"), SyntaxError);
assertThrows(() => Function("f`foo` = 1"), SyntaxError);
assertThrows(() => Function("++f`foo`"), ReferenceError);
assertThrows(() => Function("f`foo`++"), ReferenceError);
assertThrows(() => Function("--f`foo`"), ReferenceError);
assertThrows(() => Function("f`foo`--"), ReferenceError);
assertThrows(() => Function("f`foo` = 1"), ReferenceError);
})();

(function testTaggedTemplateInvalidAssignmentTargetSloppy() {
function f() {}
assertThrows(() => Function("++f`foo`"), SyntaxError);
assertThrows(() => Function("f`foo`++"), SyntaxError);
assertThrows(() => Function("--f`foo`"), SyntaxError);
assertThrows(() => Function("f`foo`--"), SyntaxError);
assertThrows(() => Function("f`foo` = 1"), SyntaxError);
assertThrows(() => Function("++f`foo`"), ReferenceError);
assertThrows(() => Function("f`foo`++"), ReferenceError);
assertThrows(() => Function("--f`foo`"), ReferenceError);
assertThrows(() => Function("f`foo`--"), ReferenceError);
assertThrows(() => Function("f`foo` = 1"), ReferenceError);
})();

// Disable eval caching if a tagged template occurs in a nested function
Expand Down
Loading

0 comments on commit 356b460

Please sign in to comment.