Fix the receiver when calling parenthesized optional chains

Correctly passing the receiver depends on the Call AST node's type.
Calling a parenthesized optional chain expression is parsed as a Call of
an OptionalChain of a Property.  Currently the computation of the type
does not take optional chains of property loads into consideration, so
calls of parenthesized optional chain expressions always get passed an
undefined receiver.

Bug: v8:10024
Change-Id: I904b0eeca2df30160def674fb32adf821403aef9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1938571Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65252}

src/ast/ast.cc

@@ -835,15 +835,27 @@ Call::CallType Call::GetCallType() const {
   if (expression()->IsSuperCallReference()) return SUPER_CALL;
 
   Property* property = expression()->AsProperty();
+  bool is_optional_chain = false;
+  if (V8_UNLIKELY(property == nullptr && expression()->IsOptionalChain())) {
+    is_optional_chain = true;
+    property = expression()->AsOptionalChain()->expression()->AsProperty();
+  }
   if (property != nullptr) {
     if (property->IsPrivateReference()) {
       return PRIVATE_CALL;
     }
     bool is_super = property->IsSuperAccess();
+    // `super?.` is not syntactically valid, so a property load cannot be both
+    // super and an optional chain.
+    DCHECK(!is_super || !is_optional_chain);
     if (property->key()->IsPropertyName()) {
-      return is_super ? NAMED_SUPER_PROPERTY_CALL : NAMED_PROPERTY_CALL;
+      if (is_super) return NAMED_SUPER_PROPERTY_CALL;
+      if (is_optional_chain) return NAMED_OPTIONAL_CHAIN_PROPERTY_CALL;
+      return NAMED_PROPERTY_CALL;
     } else {
-      return is_super ? KEYED_SUPER_PROPERTY_CALL : KEYED_PROPERTY_CALL;
+      if (is_super) return KEYED_SUPER_PROPERTY_CALL;
+      if (is_optional_chain) return KEYED_OPTIONAL_CHAIN_PROPERTY_CALL;
+      return KEYED_PROPERTY_CALL;
     }
   }
 

src/ast/ast.h

@@ -1717,13 +1717,23 @@ class Call final : public Expression {
     WITH_CALL,
     NAMED_PROPERTY_CALL,
     KEYED_PROPERTY_CALL,
+    NAMED_OPTIONAL_CHAIN_PROPERTY_CALL,
+    KEYED_OPTIONAL_CHAIN_PROPERTY_CALL,
     NAMED_SUPER_PROPERTY_CALL,
     KEYED_SUPER_PROPERTY_CALL,
     PRIVATE_CALL,
     SUPER_CALL,
-    OTHER_CALL
+    OTHER_CALL,
+
+    FIRST_NON_SUPER_PROPERTY_CALL = NAMED_PROPERTY_CALL,
+    LAST_NON_SUPER_PROPERTY_CALL = KEYED_OPTIONAL_CHAIN_PROPERTY_CALL
   };
 
+  static bool IsNonSuperPropertyCall(CallType call_type) {
+    return base::IsInRange(call_type, FIRST_NON_SUPER_PROPERTY_CALL,
+                           LAST_NON_SUPER_PROPERTY_CALL);
+  }
+
   enum PossiblyEval {
     IS_POSSIBLY_EVAL,
     NOT_EVAL,

src/interpreter/bytecode-generator.cc

@@ -4636,16 +4636,21 @@ void BytecodeGenerator::VisitKeyedSuperPropertyLoad(Property* property,
   }
 }
 
-void BytecodeGenerator::VisitOptionalChain(OptionalChain* expr) {
+template <typename ExpressionFunc>
+void BytecodeGenerator::BuildOptionalChain(ExpressionFunc expression_func) {
   BytecodeLabel done;
   OptionalChainNullLabelScope label_scope(this);
-  VisitForAccumulatorValue(expr->expression());
+  expression_func();
   builder()->Jump(&done);
   label_scope.labels()->Bind(builder());
   builder()->LoadUndefined();
   builder()->Bind(&done);
 }
 
+void BytecodeGenerator::VisitOptionalChain(OptionalChain* expr) {
+  BuildOptionalChain([&]() { VisitForAccumulatorValue(expr->expression()); });
+}
+
 void BytecodeGenerator::VisitProperty(Property* expr) {
   AssignType property_kind = Property::GetAssignType(expr);
   if (property_kind != NAMED_SUPER_PROPERTY &&
@@ -4763,6 +4768,16 @@ void BytecodeGenerator::VisitCall(Call* expr) {
       builder()->StoreAccumulatorInRegister(callee);
       break;
     }
+    case Call::NAMED_OPTIONAL_CHAIN_PROPERTY_CALL:
+    case Call::KEYED_OPTIONAL_CHAIN_PROPERTY_CALL: {
+      OptionalChain* chain = callee_expr->AsOptionalChain();
+      Property* property = chain->expression()->AsProperty();
+      BuildOptionalChain([&]() {
+        VisitAndPushIntoRegisterList(property->obj(), &args);
+        VisitPropertyLoadForRegister(args.last_register(), property, callee);
+      });
+      break;
+    }
     case Call::SUPER_CALL:
       UNREACHABLE();
   }
@@ -4815,8 +4830,7 @@ void BytecodeGenerator::VisitCall(Call* expr) {
   } else if (optimize_as_one_shot) {
     DCHECK(!implicit_undefined_receiver);
     builder()->CallNoFeedback(callee, args);
-  } else if (call_type == Call::NAMED_PROPERTY_CALL ||
-             call_type == Call::KEYED_PROPERTY_CALL) {
+  } else if (Call::IsNonSuperPropertyCall(call_type)) {
     DCHECK(!implicit_undefined_receiver);
     builder()->CallProperty(callee, args,
                             feedback_index(feedback_spec()->AddCallICSlot()));

src/interpreter/bytecode-generator.h

@@ -379,6 +379,9 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
                        HandlerTable::CatchPrediction catch_prediction,
                        TryFinallyStatement* stmt_for_coverage = nullptr);
 
+  template <typename ExpressionFunc>
+  void BuildOptionalChain(ExpressionFunc expression_func);
+
   // Visitors for obtaining expression result in the accumulator, in a
   // register, or just getting the effect. Some visitors return a TypeHint which
   // specifies the type of the result of the visited expression.

test/mjsunit/harmony/optional-chaining-this.js

+// Copyright 2019 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.
+
+// Flags: --harmony-optional-chaining
+
+const o = { m() { return this; }, p: 42 };
+const p = { o: o };
+
+function id(x) { return x; }
+
+assertEquals(o?.m().p, 42);
+assertEquals((o?.m)().p, 42);
+assertEquals(o?.m(), (o?.m)());
+assertEquals(p?.o?.m().p, 42);
+assertEquals((p?.o?.m)().p, 42);
+assertEquals(p?.o?.m(), (p?.o?.m)());
+
+assertEquals(o?.[id('m')]().p, 42);
+assertEquals((o?.[id('m')])().p, 42);
+assertEquals(o?.[id('m')](), (o?.[id('m')])());
+assertEquals(p?.[id('o')]?.[id('m')]().p, 42);
+assertEquals((p?.[id('o')]?.[id('m')])().p, 42);
+assertEquals(p?.[id('o')]?.[id('m')](), (p?.[id('o')]?.[id('m')])());