Bug 1730843 - Part 7 - Implement bytecode/interpreter for record literals r=mgaudet

This patch defines the bytecode for record literals, similarly to how it has been implemented for tuple literals: - `InitRecord [length]` pushes a new record to the stack, allocating memory for `length` properties - `AddRecordProperty` reads the key and the value from the stack, and defines it on the record which is being initialized - `FinishRecord` freezes a record and sorts its keys This patch doesn't implement support for spread in record literals yet. Differential Revision: https://phabricator.services.mozilla.com/D125651
2021-12-20 15:17:19 +00:00 · 2021-12-20 15:17:19 +00:00 · 6d81435bfb
--- a/js/public/friend/ErrorNumbers.msg
+++ b/js/public/friend/ErrorNumbers.msg
@ -823,6 +823,7 @@ MSG_DEF(JSMSG_NEGATIVE_LIMIT,                            0, JSEXN_RANGEERR, "Ite
 // Record and Tuple
 MSG_DEF(JSMSG_RECORD_TUPLE_NO_OBJECT,                    0, JSEXN_TYPEERR, "Record and Tuple can only contain primitive values")
 MSG_DEF(JSMSG_RECORD_NO_PROTO,                           0, JSEXN_SYNTAXERR, "__proto__ is not a valid literal key in records")
+MSG_DEF(JSMSG_RECORD_NO_SYMBOL_KEY,						 0, JSEXN_TYPEERR, "Symbols cannot be used as record keys")
 MSG_DEF(JSMSG_RECORD_INVALID_DESCRIPTOR,                 0, JSEXN_TYPEERR, "Invalid record property descriptor")

 //clang-format on
--- a/js/src/debugger/Script.cpp
+++ b/js/src/debugger/Script.cpp
@ -1596,6 +1596,9 @@ static bool BytecodeIsEffectful(JSOp op) {
    case JSOp::ThrowMsg:
    case JSOp::ForceInterpreter:
 #ifdef ENABLE_RECORD_TUPLE
+    case JSOp::InitRecord:
+    case JSOp::AddRecordProperty:
+    case JSOp::FinishRecord:
    case JSOp::InitTuple:
    case JSOp::AddTupleElement:
    case JSOp::FinishTuple:
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@ -10485,13 +10485,71 @@ bool BytecodeEmitter::emitSpreadIntoArray(UnaryNode* elem) {
 }

 #ifdef ENABLE_RECORD_TUPLE
+bool BytecodeEmitter::emitRecordLiteral(ListNode* record) {
+  if (!emitUint32Operand(JSOp::InitRecord, record->count())) {
+    //              [stack] RECORD
+    return false;
+  }
+
+  for (ParseNode* propdef : record->contents()) {
+    if (propdef->isKind(ParseNodeKind::Spread)) {
+      MOZ_CRASH(
+          "Bytecode emitter for record spread hasn't been implemented yet");
+    } else {
+      BinaryNode* prop = &propdef->as<BinaryNode>();
+
+      ParseNode* key = prop->left();
+      ParseNode* value = prop->right();
+
+      switch (key->getKind()) {
+        case ParseNodeKind::ObjectPropertyName:
+          if (!emitStringOp(JSOp::String, key->as<NameNode>().atom())) {
+            return false;
+          }
+          break;
+        case ParseNodeKind::ComputedName:
+          if (!emitTree(key->as<UnaryNode>().kid())) {
+            return false;
+          }
+          break;
+        default:
+          MOZ_ASSERT(key->isKind(ParseNodeKind::StringExpr) ||
+                     key->isKind(ParseNodeKind::NumberExpr) ||
+                     key->isKind(ParseNodeKind::BigIntExpr));
+          if (!emitTree(key)) {
+            return false;
+          }
+          break;
+      }
+      //            [stack] RECORD KEY
+
+      if (!emitTree(value)) {
+        //          [stack] RECORD KEY VALUE
+        return false;
+      }
+
+      if (!emit1(JSOp::AddRecordProperty)) {
+        //          [stack] RECORD
+        return false;
+      }
+    }
+  }
+
+  if (!emit1(JSOp::FinishRecord)) {
+    //              [stack] RECORD
+    return false;
+  }
+
+  return true;
+}
+
 bool BytecodeEmitter::emitTupleLiteral(ListNode* tuple) {
  if (!emitUint32Operand(JSOp::InitTuple, tuple->count())) {
    //              [stack] TUPLE
    return false;
  }

-  for (ParseNode* elt = tuple->head(); elt; elt = elt->pn_next) {
+  for (ParseNode* elt : tuple->contents()) {
    if (elt->isKind(ParseNodeKind::Spread)) {
      ParseNode* expr = elt->as<UnaryNode>().kid();

@ -11746,7 +11804,10 @@ bool BytecodeEmitter::emitTree(

 #ifdef ENABLE_RECORD_TUPLE
    case ParseNodeKind::RecordExpr:
-      MOZ_CRASH("records are not supported yet by BytecodeEmitter::emitTree");
+      if (!emitRecordLiteral(&pn->as<ListNode>())) {
+        return false;
+      }
+      break;

    case ParseNodeKind::TupleExpr:
      if (!emitTupleLiteral(&pn->as<ListNode>())) {
--- a/js/src/frontend/BytecodeEmitter.h
+++ b/js/src/frontend/BytecodeEmitter.h
@ -1007,6 +1007,7 @@ struct MOZ_STACK_CLASS BytecodeEmitter {
                                               OptionalEmitter& oe);

 #ifdef ENABLE_RECORD_TUPLE
+  [[nodiscard]] bool emitRecordLiteral(ListNode* record);
  [[nodiscard]] bool emitTupleLiteral(ListNode* tuple);
 #endif

--- a/js/src/jit/BaselineCodeGen.cpp
+++ b/js/src/jit/BaselineCodeGen.cpp
@ -2589,21 +2589,21 @@ bool BaselineCodeGen<Handler>::emit_RegExp() {
 }

 #ifdef ENABLE_RECORD_TUPLE
-template <typename Handler>
-bool BaselineCodeGen<Handler>::emit_InitTuple() {
-  MOZ_CRASH("Record and Tuple are not supported by jit");
-  return false;
-}
-template <typename Handler>
-bool BaselineCodeGen<Handler>::emit_AddTupleElement() {
-  MOZ_CRASH("Record and Tuple are not supported by jit");
-  return false;
-}
-template <typename Handler>
-bool BaselineCodeGen<Handler>::emit_FinishTuple() {
-  MOZ_CRASH("Record and Tuple are not supported by jit");
-  return false;
-}
+#  define UNSUPPORTED_OPCODE(OP)                              \
+    template <typename Handler>                               \
+    bool BaselineCodeGen<Handler>::emit_##OP() {              \
+      MOZ_CRASH("Record and Tuple are not supported by jit"); \
+      return false;                                           \
+    }
+
+UNSUPPORTED_OPCODE(InitRecord)
+UNSUPPORTED_OPCODE(AddRecordProperty)
+UNSUPPORTED_OPCODE(FinishRecord)
+UNSUPPORTED_OPCODE(InitTuple)
+UNSUPPORTED_OPCODE(AddTupleElement)
+UNSUPPORTED_OPCODE(FinishTuple)
+
+#  undef UNSUPPORTED_OPCODE
 #endif

 template <typename Handler>
--- a/js/src/jit/WarpBuilder.h
+++ b/js/src/jit/WarpBuilder.h
@ -56,6 +56,9 @@ namespace jit {
  /* Non-syntactic scope */              \
  _(NonSyntacticGlobalThis)              \
  /* Records and Tuples */               \
+  IF_RECORD_TUPLE(_(InitRecord))         \
+  IF_RECORD_TUPLE(_(AddRecordProperty))  \
+  IF_RECORD_TUPLE(_(FinishRecord))       \
  IF_RECORD_TUPLE(_(InitTuple))          \
  IF_RECORD_TUPLE(_(AddTupleElement))    \
  IF_RECORD_TUPLE(_(FinishTuple))        \
--- a/js/src/tests/non262/Record/literal.js
+++ b/js/src/tests/non262/Record/literal.js
@ -0,0 +1,21 @@
+// |reftest| skip-if(!this.hasOwnProperty("Tuple"))
+
+let rec = #{ x: 1, "y": 2, 0: 3, 1n: 4, [`key${4}`]: 5 };
+
+assertEq(rec.x, 1);
+assertEq(rec.y, 2);
+assertEq(rec[0], 3);
+assertEq(rec[1], 4);
+assertEq(rec.key4, 5);
+
+// TODO: Handle duplicated keys
+// let dup = #{ x: 1, x: 2 };
+// assertEq(dup.x, 2);
+
+assertThrowsInstanceOf(
+	() => #{ [Symbol()]: 1 },
+	TypeError,
+	"Symbols cannot be used as record keys"
+  );
+
+if (typeof reportCompare === "function") reportCompare(0, 0);
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@ -3987,6 +3987,46 @@ static MOZ_NEVER_INLINE JS_HAZ_JSNATIVE_CALLER bool Interpret(JSContext* cx,
    END_CASE(InitElemInc)

 #ifdef ENABLE_RECORD_TUPLE
+    CASE(InitRecord) {
+      uint32_t length = GET_UINT32(REGS.pc);
+      RecordType* rec = RecordType::createUninitialized(cx, length);
+      if (!rec) {
+        goto error;
+      }
+      PUSH_EXTENDED_PRIMITIVE(*rec);
+    }
+    END_CASE(InitRecord)
+
+    CASE(AddRecordProperty) {
+      MOZ_ASSERT(REGS.stackDepth() >= 3);
+
+      ReservedRooted<JSObject*> rec(&rootObject0,
+                                    &REGS.sp[-3].toExtendedPrimitive());
+      MOZ_ASSERT(rec->is<RecordType>());
+
+      ReservedRooted<Value> key(&rootValue0, REGS.sp[-2]);
+      ReservedRooted<jsid> id(&rootId0);
+      if (!JS_ValueToId(cx, key, &id)) {
+        goto error;
+      }
+      if (!rec->as<RecordType>().initializeNextProperty(
+              cx, id, REGS.stackHandleAt(-1))) {
+        goto error;
+      }
+
+      REGS.sp -= 2;
+    }
+    END_CASE(AddRecordProperty)
+
+    CASE(FinishRecord) {
+      MOZ_ASSERT(REGS.stackDepth() >= 1);
+      RecordType* rec = &REGS.sp[-1].toExtendedPrimitive().as<RecordType>();
+      if (!rec->finishInitialization(cx)) {
+        goto error;
+      }
+    }
+    END_CASE(FinishRecord)
+
    CASE(InitTuple) {
      uint32_t length = GET_UINT32(REGS.pc);
      TupleType* tup = TupleType::createUninitialized(cx, length);
--- a/js/src/vm/Opcodes.h
+++ b/js/src/vm/Opcodes.h
@ -212,6 +212,7 @@
 * [Index]
 *   [Constants]
 *   [Compound primitives]
+ *     Record literals
 *     Tuple literals
 *   [Expressions]
 *     Unary operators
@ -1530,6 +1531,43 @@
     *   Stack: => regexp
     */ \
    MACRO(RegExp, reg_exp, NULL, 5, 0, 1, JOF_REGEXP) \
+    /*
+     * Initialize a new record, preallocating `length` memory slots. `length` can still grow
+     * if needed, for example when using the spread operator.
+     *
+     * Implements: [RecordLiteral Evaluation][1] step 1.
+     *
+     * [1]: https://tc39.es/proposal-record-tuple/#sec-record-initializer-runtime-semantics-evaluation
+     *
+     *   Category: Compound primitives
+     *   Type: Record literals
+     *   Operands: uint32_t length
+     *   Stack: => rval
+     */ \
+    IF_RECORD_TUPLE(MACRO(InitRecord, init_record, NULL, 5, 0, 1, JOF_UINT32)) \
+    /*
+     * Add the last element in the stack to the preceding tuple.
+     *
+     * Implements: [AddPropertyIntoRecordEntriesList][1].
+     *
+     * [1]: https://tc39.es/proposal-record-tuple/#sec-addpropertyintorecordentrieslist
+     *
+     *   Category: Compound primitives
+     *   Type: Record literals
+     *   Operands:
+     *   Stack: record, key, value => record
+     */ \
+    IF_RECORD_TUPLE(MACRO(AddRecordProperty, add_record_property, NULL, 1, 3, 1, JOF_BYTE)) \
+    /*
+     * Mark a record as "initialized", going from "write-only" mode to
+     * "read-only" mode.
+     *
+     *   Category: Compound primitives
+     *   Type: Record literals
+     *   Operands:
+     *   Stack: record => record
+     */ \
+    IF_RECORD_TUPLE(MACRO(FinishRecord, finish_record, NULL, 1, 1, 1, JOF_BYTE)) \
    /*
     * Initialize a new tuple, preallocating `length` memory slots. `length` can still grow
     * if needed, for example when using the spread operator.
@ -3553,9 +3591,9 @@
  IF_RECORD_TUPLE(/* empty */, MACRO(228))     \
  IF_RECORD_TUPLE(/* empty */, MACRO(229))     \
  IF_RECORD_TUPLE(/* empty */, MACRO(230))     \
-  MACRO(231)                                   \
-  MACRO(232)                                   \
-  MACRO(233)                                   \
+  IF_RECORD_TUPLE(/* empty */, MACRO(231))     \
+  IF_RECORD_TUPLE(/* empty */, MACRO(232))     \
+  IF_RECORD_TUPLE(/* empty */, MACRO(233))     \
  MACRO(234)                                   \
  MACRO(235)                                   \
  MACRO(236)                                   \
--- a/js/src/vm/RecordType.cpp
+++ b/js/src/vm/RecordType.cpp
@ -82,7 +82,11 @@ RecordType* RecordType::createUninitialized(JSContext* cx,

 bool RecordType::initializeNextProperty(JSContext* cx, HandleId key,
                                        HandleValue value) {
-  MOZ_ASSERT(!key.isSymbol());
+  if (key.isSymbol()) {
+    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
+                              JSMSG_RECORD_NO_SYMBOL_KEY);
+    return false;
+  }

  if (!value.isPrimitive()) {
    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,