From 6d81435bfbd624943be021659e41f9eb267cf1e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Mon, 20 Dec 2021 15:17:19 +0000 Subject: [PATCH] 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 --- js/public/friend/ErrorNumbers.msg | 1 + js/src/debugger/Script.cpp | 3 ++ js/src/frontend/BytecodeEmitter.cpp | 65 ++++++++++++++++++++++++++- js/src/frontend/BytecodeEmitter.h | 1 + js/src/jit/BaselineCodeGen.cpp | 30 ++++++------- js/src/jit/WarpBuilder.h | 3 ++ js/src/tests/non262/Record/literal.js | 21 +++++++++ js/src/vm/Interpreter.cpp | 40 +++++++++++++++++ js/src/vm/Opcodes.h | 44 ++++++++++++++++-- js/src/vm/RecordType.cpp | 6 ++- 10 files changed, 193 insertions(+), 21 deletions(-) create mode 100644 js/src/tests/non262/Record/literal.js diff --git a/js/public/friend/ErrorNumbers.msg b/js/public/friend/ErrorNumbers.msg index dcbfa3043a5b..1a8f9f10653d 100644 --- 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 diff --git a/js/src/debugger/Script.cpp b/js/src/debugger/Script.cpp index 019c88e918fa..809b5c3fee13 100644 --- 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: diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index 1606f10e14d0..e1d3c8430703 100644 --- 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(); + + ParseNode* key = prop->left(); + ParseNode* value = prop->right(); + + switch (key->getKind()) { + case ParseNodeKind::ObjectPropertyName: + if (!emitStringOp(JSOp::String, key->as().atom())) { + return false; + } + break; + case ParseNodeKind::ComputedName: + if (!emitTree(key->as().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().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())) { + return false; + } + break; case ParseNodeKind::TupleExpr: if (!emitTupleLiteral(&pn->as())) { diff --git a/js/src/frontend/BytecodeEmitter.h b/js/src/frontend/BytecodeEmitter.h index 521326e9249f..2794b352760a 100644 --- 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 diff --git a/js/src/jit/BaselineCodeGen.cpp b/js/src/jit/BaselineCodeGen.cpp index 0353f4eadd64..94893f154f64 100644 --- a/js/src/jit/BaselineCodeGen.cpp +++ b/js/src/jit/BaselineCodeGen.cpp @@ -2589,21 +2589,21 @@ bool BaselineCodeGen::emit_RegExp() { } #ifdef ENABLE_RECORD_TUPLE -template -bool BaselineCodeGen::emit_InitTuple() { - MOZ_CRASH("Record and Tuple are not supported by jit"); - return false; -} -template -bool BaselineCodeGen::emit_AddTupleElement() { - MOZ_CRASH("Record and Tuple are not supported by jit"); - return false; -} -template -bool BaselineCodeGen::emit_FinishTuple() { - MOZ_CRASH("Record and Tuple are not supported by jit"); - return false; -} +# define UNSUPPORTED_OPCODE(OP) \ + template \ + bool BaselineCodeGen::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 diff --git a/js/src/jit/WarpBuilder.h b/js/src/jit/WarpBuilder.h index 6eab6c8575c6..0909ce9f5f4b 100644 --- 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)) \ diff --git a/js/src/tests/non262/Record/literal.js b/js/src/tests/non262/Record/literal.js new file mode 100644 index 000000000000..e54825c99ef7 --- /dev/null +++ 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); diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp index 3b87c297c372..e490c4915e14 100644 --- 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 rec(&rootObject0, + ®S.sp[-3].toExtendedPrimitive()); + MOZ_ASSERT(rec->is()); + + ReservedRooted key(&rootValue0, REGS.sp[-2]); + ReservedRooted id(&rootId0); + if (!JS_ValueToId(cx, key, &id)) { + goto error; + } + if (!rec->as().initializeNextProperty( + cx, id, REGS.stackHandleAt(-1))) { + goto error; + } + + REGS.sp -= 2; + } + END_CASE(AddRecordProperty) + + CASE(FinishRecord) { + MOZ_ASSERT(REGS.stackDepth() >= 1); + RecordType* rec = ®S.sp[-1].toExtendedPrimitive().as(); + if (!rec->finishInitialization(cx)) { + goto error; + } + } + END_CASE(FinishRecord) + CASE(InitTuple) { uint32_t length = GET_UINT32(REGS.pc); TupleType* tup = TupleType::createUninitialized(cx, length); diff --git a/js/src/vm/Opcodes.h b/js/src/vm/Opcodes.h index 0fda092c5956..1797453d07a0 100644 --- 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) \ diff --git a/js/src/vm/RecordType.cpp b/js/src/vm/RecordType.cpp index 6dd4415f73f1..54ff12e01db7 100644 --- 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,