Closed Bug 1744975 Opened 1 year ago Closed 10 months ago

Implement Tuple.prototype.* methods

Categories

(Core :: JavaScript Engine, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
98 Branch
Tracking Status
firefox98 --- fixed

People

(Reporter: nicolo.ribaudo, Assigned: tjc)

References

(Blocks 1 open bug)

Details

Attachments

(17 files, 7 obsolete files)

48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review

The patches for 1730843 implement a complete version of the new Record/Tuple/Box primitives, but it doesn't include all the Tuple.prototype.* methods. I'm tracking their implementation separately because they are another self-contained part of the proposal.

This bug should be assigned to tjc@igalia.com (I don't think I can assign it; I see the field grayed out)

Depends on: 1730843
Blocks: 1658309
Assignee: nobody → tjc
Priority: -- → P3

Add an ArrayLikeLength intrinsic and call it from self-hosted Array methods.
For any Object o (that's not an ExtendedPrimitive), it just returns o's
length property. In the future, this will be modified to access an
ExtendedPrimitive's length from the internal slot rather than the property.
This is to allow reusing array code for Tuples while preserving the semantics
from the spec that built-in methods always use the internal length slot
rather than calling an overridden method.

Depends on D134821

As per spec 3.1.2

Depends on D134822

Depends on D134823

This is useful for implementing the tuple reversed() method.

Depends on D134824

Depends on D134828

Depends on D134829

This is in order to implement the toLocaleString method for tuples.

Depends on D134830

This will be called by self-hosting tuple methods.

Depends on D134831

Tuple self-hosted methods will rely on Array methods. These methods
should get the internal length slot when called on a Tuple, rather than
the length property.

Depends on D134832

Depends on D134833

Depends on D134834

Depends on D134835

Depends on D134836

Depends on D134838

Attachment #9257110 - Attachment description: WIP: Bug 1744975 - 16. Implement Tuple.of() static method → Bug 1744975 - 16. Implement Tuple.of() static method r? mgaudet r? jandem
Attachment #9257110 - Attachment description: Bug 1744975 - 16. Implement Tuple.of() static method r? mgaudet r? jandem → Bug 1744975 - 16. Implement Tuple.of() static method r=mgaudet r=jandem
Attachment #9257110 - Attachment description: Bug 1744975 - 16. Implement Tuple.of() static method r=mgaudet r=jandem → Bug 1744975 - 16. Implement Tuple.of() static method r? mgaudet r? jandem
Attachment #9257109 - Attachment description: WIP: Bug 1744975 - 15. Implement Tuple.from() static method → Bug 1744975 - 15. Implement Tuple.from() static method r? mgaudet r? jandem
Attachment #9257108 - Attachment description: WIP: Bug 1744975 - 14. Implement Tuple.isTuple() static method → Bug 1744975 - 14. Implement Tuple.isTuple() static method r? mgaudet r? jandem
Attachment #9257107 - Attachment description: WIP: Bug 1744975 - 13. Implement tuple prototype methods → Bug 1744975 - 13. Implement tuple prototype methods r? mgaudet r? jandem
Attachment #9257106 - Attachment description: WIP: Bug 1744975 - 12. Handle tuple length properly in Array code → Bug 1744975 - 12. Handle tuple length properly in Array code r? mgaudet r? jandem
Attachment #9257105 - Attachment description: WIP: Bug 1744975 - 11. Add a TupleLength intrinsic → Bug 1744975 - 11. Add a TupleLength intrinsic r? mgaudet r? jandem
Attachment #9257104 - Attachment description: WIP: Bug 1744975 - 10. Expose array_toLocaleString → Bug 1744975 - 10. Expose array_toLocaleString r? mgaudet r? jandem
Attachment #9257103 - Attachment description: WIP: Bug 1744975 - 9. Allow array_join to be used for Tuples → Bug 1744975 - 9. Allow array_join to be used for Tuples r? mgaudet r? jandem
Attachment #9257102 - Attachment description: WIP: Bug 1744975 - 8. Add support for tuple iteration → Bug 1744975 - 8. Add support for tuple iteration r? mgaudet r? jandem
Attachment #9257101 - Attachment description: WIP: Bug 1744975 - 7. Add IsTuple and ToTupleValue self-hosting intrinsics → Bug 1744975 - 7. Add IsTuple and ToTupleValue self-hosting intrinsics r? mgaudet r? jandem
Attachment #9257100 - Attachment description: WIP: Bug 1744975 - 6. Stringify Record/Tuple wrappers correctly in js::ObjectToSource() → Bug 1744975 - 6. Stringify Record/Tuple wrappers correctly in js::ObjectToSource() r? mgaudet r? jandem
Attachment #9257099 - Attachment description: WIP: Bug 1744975 - 5. Handle Records and Tuples properly in js::ToStringSlow() → Bug 1744975 - 5. Handle Records and Tuples properly in js::ToStringSlow() r? mgaudet r? jandem
Attachment #9257098 - Attachment description: WIP: Bug 1744975 - 4. Factor out ArrayReverseDenseKernel() → Bug 1744975 - 4. Factor out ArrayReverseDenseKernel() r? mgaudet r? jandem
Attachment #9257097 - Attachment description: WIP: Bug 1744975 - 3. Add IsTuple() and ThisTupleValue() methods to TupleObject → Bug 1744975 - 3. Add IsTuple() and ThisTupleValue() methods to TupleObject r? mgaudet r? jandem
Attachment #9257096 - Attachment description: WIP: Bug 1744975 - 2. Throw type error when attempting to convert tuples to numbers → Bug 1744975 - 2. Throw type error when attempting to convert tuples to numbers r? mgaudet r? jandem
Attachment #9257095 - Attachment description: WIP: Bug 1744975 - 1. Add error messages for tuple methods → Bug 1744975 - 1. Add error messages for tuple methods r? mgaudet r? jandem
Attachment #9257094 - Attachment description: WIP: Bug 1744975 - WIP - add ArrayLikeLength intrinsic → Bug 1744975 - WIP - add ArrayLikeLength intrinsic r? mgaudet r? jandem
Attachment #9257094 - Attachment description: Bug 1744975 - WIP - add ArrayLikeLength intrinsic r? mgaudet r? jandem → Bug 1744975 - add ArrayLikeLength intrinsic r? mgaudet r? jandem
Attachment #9257094 - Attachment description: Bug 1744975 - add ArrayLikeLength intrinsic r? mgaudet r? jandem → Bug 1744975 - add ArrayLikeLength intrinsic r=mgaudet,jandem
Attachment #9257095 - Attachment description: Bug 1744975 - 1. Add error messages for tuple methods r? mgaudet r? jandem → Bug 1744975 - 1. Add error messages for tuple methods r=mgaudet,jandem
Attachment #9257096 - Attachment description: Bug 1744975 - 2. Throw type error when attempting to convert tuples to numbers r? mgaudet r? jandem → Bug 1744975 - 2. Throw type error when attempting to convert tuples to numbers r=mgaudet,jandem
Attachment #9257097 - Attachment description: Bug 1744975 - 3. Add IsTuple() and ThisTupleValue() methods to TupleObject r? mgaudet r? jandem → Bug 1744975 - 3. Add IsTuple() and ThisTupleValue() methods to TupleObject r=mgaudet,jandem
Attachment #9257098 - Attachment description: Bug 1744975 - 4. Factor out ArrayReverseDenseKernel() r? mgaudet r? jandem → Bug 1744975 - 4. Factor out ArrayReverseDenseKernel() r=mgaudet,jandem
Attachment #9257099 - Attachment description: Bug 1744975 - 5. Handle Records and Tuples properly in js::ToStringSlow() r? mgaudet r? jandem → Bug 1744975 - 5. Handle Records and Tuples properly in js::ToStringSlow() r=mgaudet,jandem
  • unbox() now returns a TupleType&
  • maybeUnbox() now returns a Maybe<TupleType&>; in both cases, callers are responsible for rooting
  • IsTuple() now takes a Value& rather than a HandleValue
  • ThisTupleValue() now takes a Value& and returns a TupleType&

These changes are meant to minimize assumptions made by TupleObject and TupleType methods about their inputs, and shift responsibility for rooting onto the callers.

Attachment #9257097 - Attachment description: Bug 1744975 - 3. Add IsTuple() and ThisTupleValue() methods to TupleObject r=mgaudet,jandem → Bug 1744975 - 3. Add ThisTupleValue() method to TupleObject r=mgaudet,jandem
Attachment #9257100 - Attachment description: Bug 1744975 - 6. Stringify Record/Tuple wrappers correctly in js::ObjectToSource() r? mgaudet r? jandem → Bug 1744975 - 6. Stringify Record/Tuple wrappers correctly in js::ObjectToSource() r=mgaudet,jandem
Attachment #9257101 - Attachment description: Bug 1744975 - 7. Add IsTuple and ToTupleValue self-hosting intrinsics r? mgaudet r? jandem → Bug 1744975 - 7. Add IsTuple and ToTupleValue self-hosting intrinsics r=mgaudet,jandem
Attachment #9257102 - Attachment description: Bug 1744975 - 8. Add support for tuple iteration r? mgaudet r? jandem → Bug 1744975 - 8. Add support for tuple iteration r=mgaudet,jandem
Attachment #9257103 - Attachment description: Bug 1744975 - 9. Allow array_join to be used for Tuples r? mgaudet r? jandem → Bug 1744975 - 9. Allow array_join to be used for Tuples r=mgaudet,jandem
Attachment #9257104 - Attachment description: Bug 1744975 - 10. Expose array_toLocaleString r? mgaudet r? jandem → Bug 1744975 - 10. Expose array_toLocaleString r=mgaudet,jandem
Attachment #9257105 - Attachment description: Bug 1744975 - 11. Add a TupleLength intrinsic r? mgaudet r? jandem → Bug 1744975 - 11. Add a TupleLength intrinsic r=mgaudet,jandem
Attachment #9257106 - Attachment description: Bug 1744975 - 12. Handle tuple length properly in Array code r? mgaudet r? jandem → Bug 1744975 - 12. Handle tuple length properly in Array code r=mgaudet,jandem
Attachment #9257101 - Attachment description: Bug 1744975 - 7. Add IsTuple and ToTupleValue self-hosting intrinsics r=mgaudet,jandem → Bug 1744975 - 7. Add IsTuple and ThisTupleValue self-hosting intrinsics r=mgaudet,jandem
Attachment #9257107 - Attachment description: Bug 1744975 - 13. Implement tuple prototype methods r? mgaudet r? jandem → Bug 1744975 - 13. Implement tuple prototype methods r=mgaudet,jandem
Attachment #9257108 - Attachment description: Bug 1744975 - 14. Implement Tuple.isTuple() static method r? mgaudet r? jandem → Bug 1744975 - 14. Implement Tuple.isTuple() static method r=mgaudet,jandem
Attachment #9257109 - Attachment description: Bug 1744975 - 15. Implement Tuple.from() static method r? mgaudet r? jandem → Bug 1744975 - 15. Implement Tuple.from() static method r=mgaudet,jandem
Attachment #9257110 - Attachment description: Bug 1744975 - 16. Implement Tuple.of() static method r? mgaudet r? jandem → Bug 1744975 - 16. Implement Tuple.of() static method r=mgaudet,jandem
Attachment #9257111 - Attachment description: WIP: Bug 1744975 - 17. Tests for section 6.1.2.1 of spec (initialization semantics) → Bug 1744975 - 17. Tests for section 6.1.2.1 of spec (initialization semantics) r=mgaudet,jandem
Attachment #9257112 - Attachment description: WIP: Bug 1744975 - 18. Conformance tests for Tuple constructor → Bug 1744975 - 18. Conformance tests for Tuple constructor r=mgaudet,jandem
Attachment #9257113 - Attachment description: WIP: Bug 1744975 - 19. Add test for 9.1.1.1 of spec (Object.prototype.toString) → Bug 1744975 - 19. Add test for 9.1.1.1 of spec (Object.prototype.toString) r=mgaudet,jandem
Attachment #9257114 - Attachment description: WIP: Bug 1744975 - 20. Conformance tests for section 4.1.2.11 of proposal (out-of-range Tuple indices) → Bug 1744975 - 20. Conformance tests for section 4.1.2.11 of proposal (out-of-range Tuple indices) r=mgaudet,jandem
Attachment #9257115 - Attachment description: WIP: Bug 1744975 - 21. Conformance tests for section 4.1.2.6 of proposal (setting Tuple properties) → Bug 1744975 - 21. Conformance tests for section 4.1.2.6 of proposal (setting Tuple properties) r=mgaudet,jandem

I think I'd prefer implementing these builtins in self-hosted code, at least for now, without changing the existing Array builtins. This is all very perf-sensitive code so touching it as little as possible would be less risk for us.

I also think we should rely less on records/tuples being implemented as objects, as that should be just an implementation detail. For instance if we ever decide to implement records/tuples without using JSObject (similar to how JSString and BigInt don't use JSObject), ideally we wouldn't have to change much. I've been proposing we branch on the Value type more than on the object's class, but there are still some places I'd like to convert to that scheme.

Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(tjc)
Attachment #9257103 - Attachment description: Bug 1744975 - 9. Allow array_join to be used for Tuples r=mgaudet,jandem → Bug 1744975 - 8. Allow array_join to be used for Tuples r=mgaudet,jandem
Attachment #9257104 - Attachment description: Bug 1744975 - 10. Expose array_toLocaleString r=mgaudet,jandem → Bug 1744975 - 9. Expose array_toLocaleString r=mgaudet,jandem
Attachment #9257105 - Attachment description: Bug 1744975 - 11. Add a TupleLength intrinsic r=mgaudet,jandem → Bug 1744975 - 10. Add a TupleLength intrinsic r=mgaudet,jandem
Attachment #9257106 - Attachment description: Bug 1744975 - 12. Handle tuple length properly in Array code r=mgaudet,jandem → Bug 1744975 - 11. Handle tuple length properly in Array code r=mgaudet,jandem
Attachment #9257107 - Attachment description: Bug 1744975 - 13. Implement tuple prototype methods r=mgaudet,jandem → Bug 1744975 - 12. Implement tuple prototype methods r=mgaudet,jandem
Attachment #9257102 - Attachment is obsolete: true
Attachment #9257108 - Attachment description: Bug 1744975 - 14. Implement Tuple.isTuple() static method r=mgaudet,jandem → Bug 1744975 - 13. Implement Tuple.isTuple() static method r=mgaudet,jandem
Attachment #9257109 - Attachment description: Bug 1744975 - 15. Implement Tuple.from() static method r=mgaudet,jandem → Bug 1744975 - 14. Implement Tuple.from() static method r=mgaudet,jandem
Attachment #9257110 - Attachment description: Bug 1744975 - 16. Implement Tuple.of() static method r=mgaudet,jandem → Bug 1744975 - 15. Implement Tuple.of() static method r=mgaudet,jandem
Attachment #9257111 - Attachment description: Bug 1744975 - 17. Tests for section 6.1.2.1 of spec (initialization semantics) r=mgaudet,jandem → Bug 1744975 - 16. Tests for section 6.1.2.1 of spec (initialization semantics) r=mgaudet,jandem
Attachment #9257112 - Attachment description: Bug 1744975 - 18. Conformance tests for Tuple constructor r=mgaudet,jandem → Bug 1744975 - 17. Conformance tests for Tuple constructor r=mgaudet,jandem
Attachment #9257113 - Attachment description: Bug 1744975 - 19. Add test for 9.1.1.1 of spec (Object.prototype.toString) r=mgaudet,jandem → Bug 1744975 - 18. Add test for 9.1.1.1 of spec (Object.prototype.toString) r=mgaudet,jandem
Attachment #9257114 - Attachment description: Bug 1744975 - 20. Conformance tests for section 4.1.2.11 of proposal (out-of-range Tuple indices) r=mgaudet,jandem → Bug 1744975 - 19. Conformance tests for section 4.1.2.11 of proposal (out-of-range Tuple indices) r=mgaudet,jandem
Attachment #9257115 - Attachment description: Bug 1744975 - 21. Conformance tests for section 4.1.2.6 of proposal (setting Tuple properties) r=mgaudet,jandem → Bug 1744975 - 20. Conformance tests for section 4.1.2.6 of proposal (setting Tuple properties) r=mgaudet,jandem

(In reply to Jan de Mooij [:jandem] from comment #24)

I think I'd prefer implementing these builtins in self-hosted code, at least for now, without changing the existing Array builtins. This is all very perf-sensitive code so touching it as little as possible would be less risk for us.

Which changes to the existing Array builtins are you referring to? The only one I would expect to have any potential impact is the initial patch (D134821) that changes O.length to ArrayLikeLength(O). That change can be discussed further, but first I wanted to make sure there wasn't anything else you had in mind.

I also think we should rely less on records/tuples being implemented as objects, as that should be just an implementation detail. For instance if we ever decide to implement records/tuples without using JSObject (similar to how JSString and BigInt don't use JSObject), ideally we wouldn't have to change much. I've been proposing we branch on the Value type more than on the object's class, but there are still some places I'd like to convert to that scheme.

I would also be curious about which particular changes relying on the implementation as objects you had in mind; while the decision to implement R/T as objects was made before I joined the project, one of the advantages of it that I can see is code re-use.

Flags: needinfo?(tjc) → needinfo?(jdemooij)

(In reply to Tim Chevalier from comment #25)

Which changes to the existing Array builtins are you referring to? The only one I would expect to have any potential impact is the initial patch (D134821) that changes O.length to ArrayLikeLength(O). That change can be discussed further, but first I wanted to make sure there wasn't anything else you had in mind.

It's not just performance, it seems wrong to me that we have to make changes (that don't correspond to changes in the spec) to the array builtins because they're reused by the Tuple builtins. It'd be better to follow what we did for the similar typed array functions in js/src/builtin/TypedArray.js

What's really nice about the Tuple functions compared to the ancient Array builtins is that they're non-generic, so we could probably implement them much more efficiently than the generic implementation (although performance isn't a serious concern at this point).

Flags: needinfo?(jdemooij)
Attachment #9257094 - Attachment is obsolete: true

I've removed the patch in question because I've since learned that the spec is likely going to change to add length as an own property to Tuples. Thus, the array self-hosted built-ins don't need to change (with the exception of IsConcatSpreadable().) Patches are now ready for review.

Attachment #9257441 - Attachment description: Bug 1744975 - Change type signatures of TupleObject::unbox(), TupleObject::maybeUnbox() r=mgaudet,jandem → Bug 1744975 - Change type signatures of TupleObject::unbox(), TupleObject::maybeUnbox() r=jandem
Attachment #9257095 - Attachment description: Bug 1744975 - 1. Add error messages for tuple methods r=mgaudet,jandem → Bug 1744975 - 1. Add error messages for tuple methods r=jandem
Attachment #9257096 - Attachment description: Bug 1744975 - 2. Throw type error when attempting to convert tuples to numbers r=mgaudet,jandem → Bug 1744975 - 2. Throw type error when attempting to convert tuples to numbers r=jandem
Attachment #9257097 - Attachment description: Bug 1744975 - 3. Add ThisTupleValue() method to TupleObject r=mgaudet,jandem → Bug 1744975 - 3. Add ThisTupleValue() method to TupleObject r=jandem
Attachment #9257098 - Attachment description: Bug 1744975 - 4. Factor out ArrayReverseDenseKernel() r=mgaudet,jandem → Bug 1744975 - 4. Factor out ArrayReverseDenseKernel() r=jandem
Attachment #9257099 - Attachment description: Bug 1744975 - 5. Handle Records and Tuples properly in js::ToStringSlow() r=mgaudet,jandem → Bug 1744975 - 5. Handle Records and Tuples properly in js::ToStringSlow() r=jandem
Attachment #9257100 - Attachment description: Bug 1744975 - 6. Stringify Record/Tuple wrappers correctly in js::ObjectToSource() r=mgaudet,jandem → Bug 1744975 - 6. Stringify Record/Tuple wrappers correctly in js::ObjectToSource() r=jandem
Attachment #9257101 - Attachment description: Bug 1744975 - 7. Add IsTuple and ThisTupleValue self-hosting intrinsics r=mgaudet,jandem → Bug 1744975 - 7. Add IsTuple and ThisTupleValue self-hosting intrinsics r=jandem
Attachment #9257103 - Attachment description: Bug 1744975 - 8. Allow array_join to be used for Tuples r=mgaudet,jandem → Bug 1744975 - 8. Allow array_join to be used for Tuples r=jandem
Attachment #9257104 - Attachment description: Bug 1744975 - 9. Expose array_toLocaleString r=mgaudet,jandem → Bug 1744975 - 9. Expose array_toLocaleString r=jandem
Attachment #9257105 - Attachment description: Bug 1744975 - 10. Add a TupleLength intrinsic r=mgaudet,jandem → Bug 1744975 - 10. Add a TupleLength intrinsic r=jandem
Attachment #9257106 - Attachment description: Bug 1744975 - 11. Handle tuple length properly in Array code r=mgaudet,jandem → Bug 1744975 - 11. Handle tuple length properly in Array code r=jandem
Attachment #9257107 - Attachment description: Bug 1744975 - 12. Implement tuple prototype methods r=mgaudet,jandem → Bug 1744975 - 12. Implement tuple prototype methods r=jandem
Attachment #9257108 - Attachment description: Bug 1744975 - 13. Implement Tuple.isTuple() static method r=mgaudet,jandem → Bug 1744975 - 13. Implement Tuple.isTuple() static method r=jandem
Attachment #9257109 - Attachment description: Bug 1744975 - 14. Implement Tuple.from() static method r=mgaudet,jandem → Bug 1744975 - 14. Implement Tuple.from() static method r=jandem
Attachment #9257110 - Attachment description: Bug 1744975 - 15. Implement Tuple.of() static method r=mgaudet,jandem → Bug 1744975 - 15. Implement Tuple.of() static method r=jandem
Attachment #9257111 - Attachment description: Bug 1744975 - 16. Tests for section 6.1.2.1 of spec (initialization semantics) r=mgaudet,jandem → Bug 1744975 - 16. Tests for section 6.1.2.1 of spec (initialization semantics) r=jandem
Attachment #9257112 - Attachment description: Bug 1744975 - 17. Conformance tests for Tuple constructor r=mgaudet,jandem → Bug 1744975 - 17. Conformance tests for Tuple constructor r=jandem
Attachment #9257113 - Attachment description: Bug 1744975 - 18. Add test for 9.1.1.1 of spec (Object.prototype.toString) r=mgaudet,jandem → Bug 1744975 - 18. Add test for 9.1.1.1 of spec (Object.prototype.toString) r=jandem
Attachment #9257114 - Attachment description: Bug 1744975 - 19. Conformance tests for section 4.1.2.11 of proposal (out-of-range Tuple indices) r=mgaudet,jandem → Bug 1744975 - 19. Conformance tests for section 4.1.2.11 of proposal (out-of-range Tuple indices) r=jandem
Attachment #9257115 - Attachment description: Bug 1744975 - 20. Conformance tests for section 4.1.2.6 of proposal (setting Tuple properties) r=mgaudet,jandem → Bug 1744975 - 20. Conformance tests for section 4.1.2.6 of proposal (setting Tuple properties) r=jandem

I think comment 26 still applies?

(In reply to Jan de Mooij [:jandem] from comment #28)

I think comment 26 still applies?

Which changes do you have in mind?

(In reply to Tim Chevalier from comment #29)

(In reply to Jan de Mooij [:jandem] from comment #28)

I think comment 26 still applies?

Which changes do you have in mind?

Let's avoid touching the array builtins. I'm also pretty sure we can implement these tuple methods more efficiently because they're non-generic so can be specialized for tuples.

Attachment #9257099 - Attachment description: Bug 1744975 - 5. Handle Records and Tuples properly in js::ToStringSlow() r=jandem → Bug 1744975 - 4. Handle Records and Tuples properly in js::ToStringSlow() r=jandem
Attachment #9257100 - Attachment description: Bug 1744975 - 6. Stringify Record/Tuple wrappers correctly in js::ObjectToSource() r=jandem → Bug 1744975 - 5. Stringify Record/Tuple wrappers correctly in js::ObjectToSource() r=jandem
Attachment #9257101 - Attachment description: Bug 1744975 - 7. Add IsTuple and ThisTupleValue self-hosting intrinsics r=jandem → Bug 1744975 - 6. Add IsTuple and ThisTupleValue self-hosting intrinsics r=jandem
Attachment #9257105 - Attachment description: Bug 1744975 - 10. Add a TupleLength intrinsic r=jandem → Bug 1744975 - 7. Add a TupleLength intrinsic r=jandem
Attachment #9257107 - Attachment description: Bug 1744975 - 12. Implement tuple prototype methods r=jandem → Bug 1744975 - 8. Implement tuple prototype methods r=jandem
Attachment #9257108 - Attachment description: Bug 1744975 - 13. Implement Tuple.isTuple() static method r=jandem → Bug 1744975 - 9. Implement Tuple.isTuple() static method r=jandem
Attachment #9257109 - Attachment description: Bug 1744975 - 14. Implement Tuple.from() static method r=jandem → Bug 1744975 - 10. Implement Tuple.from() static method r=jandem

Depends on D134836

Depends on D134836

Attachment #9257111 - Attachment description: Bug 1744975 - 16. Tests for section 6.1.2.1 of spec (initialization semantics) r=jandem → Bug 1744975 - 12. Tests for section 6.1.2.1 of spec (initialization semantics) r=mgaudet,jandem
Attachment #9257112 - Attachment description: Bug 1744975 - 17. Conformance tests for Tuple constructor r=jandem → Bug 1744975 - 13. Conformance tests for Tuple constructor r=jandem
Attachment #9257113 - Attachment description: Bug 1744975 - 18. Add test for 9.1.1.1 of spec (Object.prototype.toString) r=jandem → Bug 1744975 - 14. Add test for 9.1.1.1 of spec (Object.prototype.toString) r=jandem
Attachment #9257114 - Attachment description: Bug 1744975 - 19. Conformance tests for section 4.1.2.11 of proposal (out-of-range Tuple indices) r=jandem → Bug 1744975 - 15. Conformance tests for section 4.1.2.11 of proposal (out-of-range Tuple indices) r=jandem
Attachment #9257115 - Attachment description: Bug 1744975 - 20. Conformance tests for section 4.1.2.6 of proposal (setting Tuple properties) r=jandem → Bug 1744975 - 16. Conformance tests for section 4.1.2.6 of proposal (setting Tuple properties) r=jandem
Attachment #9257098 - Attachment is obsolete: true
Attachment #9257103 - Attachment is obsolete: true
Attachment #9257104 - Attachment is obsolete: true
Attachment #9257106 - Attachment is obsolete: true
Attachment #9257110 - Attachment is obsolete: true

(In reply to Jan de Mooij [:jandem] from comment #30)

(In reply to Tim Chevalier from comment #29)

(In reply to Jan de Mooij [:jandem] from comment #28)

I think comment 26 still applies?

Which changes do you have in mind?

Let's avoid touching the array builtins. I'm also pretty sure we can implement these tuple methods more efficiently because they're non-generic so can be specialized for tuples.

OK, I've removed changes to the array built-ins, except for https://phabricator.services.mozilla.com/D134834#change-jdmntqC3ykIW , which is necessary in order to make Tuples treated as spreadable. Please take a look; thanks!

Thanks! Sorry for the delay, I got side-tracked a bit but I'll try to get to this today or tomorrow.

Attachment #9257097 - Attachment description: Bug 1744975 - 3. Add ThisTupleValue() method to TupleObject r=jandem → Bug 1744975 - 3. Add thisTupleValue() method to TupleObject r=jandem

(In reply to Jan de Mooij [:jandem] from comment #33)

Thanks! Sorry for the delay, I got side-tracked a bit but I'll try to get to this today or tomorrow.

If I'm not mistaken, the whole stack is ready to land now. Let me know if anything else needs to be fixed before landing.

Flags: needinfo?(jdemooij)

(In reply to Tim Chevalier from comment #34)

(In reply to Jan de Mooij [:jandem] from comment #33)

Thanks! Sorry for the delay, I got side-tracked a bit but I'll try to get to this today or tomorrow.

If I'm not mistaken, the whole stack is ready to land now. Let me know if anything else needs to be fixed before landing.

Do you have a green try push? The latest one I saw had some failures AFAICT.

Flags: needinfo?(tjc)

Oops, I forgot to add the line to some of the tests that disables them if records and tuples are disabled. I'm running another try build now ( https://treeherder.mozilla.org/jobs?repo=try&revision=6624b5c895a119b7d51a644aed22d3e0a01d4d ).

Flags: needinfo?(tjc)
Pushed by jdemooij@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/146d317b2766
Change type signatures of TupleObject::unbox(), TupleObject::maybeUnbox() r=jandem
https://hg.mozilla.org/integration/autoland/rev/9bc676b9dab0
1. Add error messages for tuple methods r=jandem
https://hg.mozilla.org/integration/autoland/rev/ed07b1a7007f
2. Throw type error when attempting to convert tuples to numbers r=jandem
https://hg.mozilla.org/integration/autoland/rev/f3449321c6d4
3. Add thisTupleValue() method to TupleObject r=jandem
https://hg.mozilla.org/integration/autoland/rev/4e09070d2ee8
4. Handle Records and Tuples properly in js::ToStringSlow() r=jandem
https://hg.mozilla.org/integration/autoland/rev/d1e492caeb45
5. Stringify Record/Tuple wrappers correctly in js::ObjectToSource() r=jandem
https://hg.mozilla.org/integration/autoland/rev/9750b41619e9
6. Add IsTuple and ThisTupleValue self-hosting intrinsics r=jandem
https://hg.mozilla.org/integration/autoland/rev/1fdbcea41dfa
7. Add a TupleLength intrinsic r=jandem
https://hg.mozilla.org/integration/autoland/rev/325388373bcc
8. Implement tuple prototype methods r=jandem
https://hg.mozilla.org/integration/autoland/rev/de050c420c73
9. Implement Tuple.isTuple() static method r=jandem
https://hg.mozilla.org/integration/autoland/rev/0eb6cd59f507
10. Implement Tuple.from() static method r=jandem
https://hg.mozilla.org/integration/autoland/rev/2ad924ee5e6b
11. Implement Tuple.of() static method r=jandem
https://hg.mozilla.org/integration/autoland/rev/805a82e92070
12. Tests for section 6.1.2.1 of spec (initialization semantics) r=jandem
https://hg.mozilla.org/integration/autoland/rev/37ce48df24d8
13. Conformance tests for Tuple constructor r=jandem
https://hg.mozilla.org/integration/autoland/rev/4323ecb998a7
14. Add test for 9.1.1.1 of spec (Object.prototype.toString) r=jandem
https://hg.mozilla.org/integration/autoland/rev/28d75f32d2ce
15. Conformance tests for section 4.1.2.11 of proposal (out-of-range Tuple indices) r=jandem
https://hg.mozilla.org/integration/autoland/rev/395f90b6e880
16. Conformance tests for section 4.1.2.6 of proposal (setting Tuple properties) r=jandem
Flags: needinfo?(jdemooij)
You need to log in before you can comment on or make changes to this bug.