Implement Set methods from ECMAScript Specification Features/#4128 #4145
Conversation
fix 24.2.4.9 Set.prototype.intersection ( other ) edit documentation from steps fix 24.2.4.5 Set.prototype.difference ( other ) edit documentation from steps
fix 24.2.4.9 Set.prototype.intersection ( other ) edit documentation from steps fix 24.2.4.5 Set.prototype.difference ( other ) edit documentation from steps
done 24.2.4.12 Set.prototype.isSupersetOf ( other )
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4145 +/- ##
==========================================
+ Coverage 47.24% 53.60% +6.36%
==========================================
Files 476 487 +11
Lines 46892 48998 +2106
==========================================
+ Hits 22154 26267 +4113
+ Misses 24738 22731 -2007 ☔ View full report in Codecov by Sentry. |
|
This is looking great so far! It looks like you may need to run Also, if you remove cargo run --release --bin boa_tester -- run -vv -s test/built-ins/Set/prototypeIf you want more verbose feedback, you can run: cargo run --release --bin boa_tester -- run -vvv -s test/built-ins/Set/prototypeAdding the specific method on the end of the path can target that specific method. |
|
i am done all fix need this issus #4128 |
nekevss
added
enhancement
| // 3. Let otherRec be ? GetSetRecord(other). | ||
| // (ECMAScript 2022, 24.2.3.6 step 3) | ||
| let other = args.get_or_undefined(0); | ||
| let Some(other_set) = other |
There was a problem hiding this comment.
suggestion: implement GetSetRecord here over downcasting to OrderedSet
It is actually possible for other to be a user implemented set like object. An example of this would be this test262 test.
There was a problem hiding this comment.
I understand you. I need to implement the equivalent of GetRecord. I think I should do this in the orderset file, and it will be called SetRecord, which will eliminate all the repetitive code I wrote before, right?
There was a problem hiding this comment.
The method that needs to be implemented is here. My initial reaction based off reading is that this abstract op would be implemented in mod.rs.
| .and_then(JsObject::downcast_ref::<OrderedSet>) | ||
| else { | ||
| return Err(JsNativeError::typ() | ||
| .with_message("Method Set.prototype.difference called on incompatible receiver") |
There was a problem hiding this comment.
typo: Set.prototype.difference => Set.prototype.intersection
| let other = args.get_or_undefined(0); | ||
| // 4. Let other_set be the second argument. | ||
| // We try to downcast the second argument into an `OrderedSet`, which is the other set with which we perform the intersection. | ||
| let Some(other_set) = other |
There was a problem hiding this comment.
See comment on using GetSetRecord in difference
| /// | ||
| /// [spec]: https://tc39.es/ecma262/#sec-set.prototype.isdisjointfrom | ||
| /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set/isDisjointFrom | ||
| pub(crate) fn is_dis_joint_from( |
|
|
||
| // 3. Let otherRec be ? GetSetRecord(other). | ||
| let other = args.get_or_undefined(0); | ||
| let Some(other_set) = other |
There was a problem hiding this comment.
suggestion: use GetSetRecord abstract op
See review comment on difference method.
| // 2. Perform ? RequireInternalSlot(O, [[SetData]]). | ||
| // 3. Let otherRec be ? GetSetRecord(other). | ||
| let other = args.get_or_undefined(0); | ||
| let Some(other_set) = other |
There was a problem hiding this comment.
suggestion: use GetSetRecord
See review comment on difference method
This Pull Request fixes/closes #{issue_num}.
It changes the following:
Additional Information:
This implementation aligns with the ECMAScript specification, section 24.2.4, and follows the behavior described in the Set Methods Proposal. Test coverage includes edge cases such as empty sets, overlapping sets, and disjoint sets.
Feel free to let me know if there are areas for improvement or adjustments!