[logical assignment] Disallow foo() &&= 1 etc

Having the web compatibility hack (allowing foo() = 1) enabled for
logical assignment was unintentional.

Browser compatibility data:
https://docs.google.com/document/d/1cGorRZ73KvQqu57tT4ahCjSLncibFMUwlkaL-XIstzI/edit?usp=sharing

Bug: v8:10372, v8:10950
Change-Id: I87f6348b75ce72ee5bd5db143f789ceeee596070
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2423721Reviewed-by: Shu-yu Guo <syg@chromium.org>
Commit-Queue: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70105}

src/parsing/expression-scope.h

@@ -505,8 +505,10 @@ class ExpressionParsingScope : public ExpressionScope<Types> {
       return expression;
     }
     this->mark_verified();
+    const bool early_error = false;
     return this->parser()->RewriteInvalidReferenceExpression(
-        expression, beg_pos, end_pos, MessageTemplate::kInvalidLhsInFor);
+        expression, beg_pos, end_pos, MessageTemplate::kInvalidLhsInFor,
+        early_error);
   }
 
   void RecordExpressionError(const Scanner::Location& loc,

src/parsing/parser-base.h

@@ -1353,9 +1353,12 @@ 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.
+  // Modern language features can be exempted from this hack by passing
+  // early_error = true.
   ExpressionT RewriteInvalidReferenceExpression(ExpressionT expression,
                                                 int beg_pos, int end_pos,
-                                                MessageTemplate message);
+                                                MessageTemplate message,
+                                                bool early_error);
 
   bool IsValidReferenceExpression(ExpressionT expression);
 
@@ -2825,9 +2828,12 @@ ParserBase<Impl>::ParseAssignmentExpressionCoverGrammar() {
                                           end_position());
   } else {
     DCHECK(!IsValidReferenceExpression(expression));
+    // For web compatibility reasons, throw early errors only for logical
+    // assignment, not for regular assignment.
+    const bool early_error = Token::IsLogicalAssignmentOp(op);
     expression = RewriteInvalidReferenceExpression(
         expression, lhs_beg_pos, end_position(),
-        MessageTemplate::kInvalidLhsInAssignment);
+        MessageTemplate::kInvalidLhsInAssignment, early_error);
   }
 
   Consume(op);
@@ -3142,9 +3148,10 @@ ParserBase<Impl>::ParseUnaryOrPrefixExpression() {
       expression_scope()->MarkIdentifierAsAssigned();
     }
   } else {
+    const bool early_error = false;
     expression = RewriteInvalidReferenceExpression(
         expression, expression_position, end_position(),
-        MessageTemplate::kInvalidLhsInPrefixOp);
+        MessageTemplate::kInvalidLhsInPrefixOp, early_error);
   }
 
   return factory()->NewCountOperation(op, true /* prefix */, expression,
@@ -3217,9 +3224,10 @@ typename ParserBase<Impl>::ExpressionT
 ParserBase<Impl>::ParsePostfixContinuation(ExpressionT expression,
                                            int lhs_beg_pos) {
   if (V8_UNLIKELY(!IsValidReferenceExpression(expression))) {
+    const bool early_error = false;
     expression = RewriteInvalidReferenceExpression(
         expression, lhs_beg_pos, end_position(),
-        MessageTemplate::kInvalidLhsInPostfixOp);
+        MessageTemplate::kInvalidLhsInPostfixOp, early_error);
   }
   if (impl()->IsIdentifier(expression)) {
     expression_scope()->MarkIdentifierAsAssigned();
@@ -4743,7 +4751,8 @@ template <typename Impl>
 typename ParserBase<Impl>::ExpressionT
 ParserBase<Impl>::RewriteInvalidReferenceExpression(ExpressionT expression,
                                                     int beg_pos, int end_pos,
-                                                    MessageTemplate message) {
+                                                    MessageTemplate message,
+                                                    bool early_error) {
   DCHECK(!IsValidReferenceExpression(expression));
   if (impl()->IsIdentifier(expression)) {
     DCHECK(is_strict(language_mode()));
@@ -4753,7 +4762,8 @@ ParserBase<Impl>::RewriteInvalidReferenceExpression(ExpressionT expression,
                     MessageTemplate::kStrictEvalArguments);
     return impl()->FailureExpression();
   }
-  if (expression->IsCall() && !expression->AsCall()->is_tagged_template()) {
+  if (expression->IsCall() && !expression->AsCall()->is_tagged_template() &&
+      !early_error) {
     expression_scope()->RecordPatternError(
         Scanner::Location(beg_pos, end_pos),
         MessageTemplate::kInvalidDestructuringTarget);
@@ -4767,6 +4777,9 @@ ParserBase<Impl>::RewriteInvalidReferenceExpression(ExpressionT expression,
     ExpressionT error = impl()->NewThrowReferenceError(message, beg_pos);
     return factory()->NewProperty(expression, error, beg_pos);
   }
+  // Tagged templates and other modern language features (which pass early_error
+  // = true) are exempt from the web compatibility hack. Throw a regular early
+  // error.
   ReportMessageAt(Scanner::Location(beg_pos, end_pos), message);
   return impl()->FailureExpression();
 }

test/mjsunit/call-lhs-web-compat-early-errors.js

+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Will be used in the tests
+function foo() {}
+
+function wrapInLazyFunction(s) {
+  // Use an async function, since some tests use the await keyword.
+  return "async function test() { " + s + "}";
+}
+
+function wrapInEagerFunction(s) {
+  // Use an async function, since some tests use the await keyword. Await the
+  // result, so that we get the error right away.
+  return "await (async function test() { " + s + "})();";
+}
+
+function assertEarlyError(s) {
+  assertThrows(wrapInLazyFunction(s), SyntaxError);
+}
+
+function assertLateError(s) {
+  assertDoesNotThrow(wrapInLazyFunction(s));
+  assertThrows(wrapInEagerFunction(s), ReferenceError);
+}
+
+// Web compatibility:
+assertLateError("foo() = 0;");
+assertLateError("foo()++;");
+assertLateError("foo()--;");
+assertLateError("++foo();");
+assertLateError("--foo();");
+assertLateError("foo() = 1;");
+assertLateError("foo() += 1;");
+assertLateError("foo() -= 1;");
+assertLateError("foo() *= 1;");
+assertLateError("foo() /= 1;");
+assertLateError("for (foo() = 0; ; ) {}");
+assertLateError("for ( ; ; foo() = 0) {}");
+assertLateError("for (foo() of []) {}");
+assertLateError("for (foo() in {}) {}");
+assertLateError("for await (foo() of []) {}");
+
+// These are early errors though:
+assertEarlyError("for (let foo() = 0; ;) {}");
+assertEarlyError("for (const foo() = 0; ;) {}");
+assertEarlyError("for (var foo() = 0; ;) {}");
+
+// Modern language features:
+// Tagged templates
+assertEarlyError("foo() `foo` ++;");
+assertEarlyError("foo() `foo` --;");
+assertEarlyError("++ foo() `foo`;");
+assertEarlyError("-- foo() `foo`;");
+assertEarlyError("foo() `foo` = 1;");
+assertEarlyError("foo() `foo` += 1;");
+assertEarlyError("foo() `foo` -= 1;");
+assertEarlyError("foo() `foo` *= 1;");
+assertEarlyError("foo() `foo` /= 1;");
+assertEarlyError("for (foo() `foo` = 0; ; ) {}");
+assertEarlyError("for (; ; foo() `foo` = 0) {}");
+
+// Logical assignment
+assertEarlyError("foo() &&= 1;");
+assertEarlyError("foo() ||= 1;");
+assertEarlyError("foo() ??= 1;");
+assertEarlyError("for (foo() &&= 0; ;) {}");
+assertEarlyError("for ( ; ; foo() &&= 0) {}");

test/test262/test262.status

@@ -653,11 +653,6 @@
   # https://github.com/tc39/ecma262/pull/889
   'annexB/language/function-code/block-decl-func-skip-arguments': [FAIL],
 
-  # Non-simple assignment targets are runtime errors instead of syntax errors for web compat.
-  'language/expressions/logical-assignment/lgcl-or-assignment-operator-non-simple-lhs': [FAIL],
-  'language/expressions/logical-assignment/lgcl-and-assignment-operator-non-simple-lhs': [FAIL],
-  'language/expressions/logical-assignment/lgcl-nullish-assignment-operator-non-simple-lhs': [FAIL],
-
   ############################ INVALID TESTS #############################
 
   # Test makes unjustified assumptions about the number of calls to SortCompare.