Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement arguments exotic objects #1522

Merged
merged 3 commits into from
Oct 5, 2021
Merged

Implement arguments exotic objects #1522

merged 3 commits into from
Oct 5, 2021

Conversation

jedel1043
Copy link
Member

@jedel1043 jedel1043 commented Aug 27, 2021 •
edited
Loading

This Pull Request builds from #1354 and #1518 to implement arguments exotic objects.

It changes the following:

  • Implements argument exotic objects
  • Refactors ClosureFunction to hold external context as a NativeObject
  • Adds a new FunctionBuilder::closure_with_captures function

Should I extract the last two changes as their own PR?.

Drafting this because right now we don't have a way to know if we are call_constructing in strict mode, so a lot of tests fail because they expect an unmapped argument ordinary object in strict mode and a mapped argument exotic object in non-strict mode.

@jedel1043 jedel1043 changed the title Args exotic Implement arguments exotic objects Aug 27, 2021
@Razican
Copy link
Member

Razican commented Aug 27, 2021

Should I extract the last two changes as their own PR?.

I would be happy to have smaller change-sets to review xD

Drafting this because right now we don't have a way to know if we are call_constructing in strict mode, so a lot of tests fail because they expect an unmapped argument ordinary object in strict mode and a mapped argument exotic object in non-strict mode.

On this, we can have an is_strict() function in the context maybe, to at least add the logic. It would always return false for now, for example.

@jedel1043
Copy link
Member Author

Should I extract the last two changes as their own PR?.

I would be happy to have smaller change-sets to review xD

Okay! I'm still gonna draft it until #1518 merges just in case

@jedel1043
Copy link
Member Author

jedel1043 commented Sep 9, 2021 •
edited
Loading

Now that #1550 merged, this is just waiting for another review on #1523 in order to merge it and finish the implementation.

jedel1043
jedel1043 commented Sep 20, 2021
View reviewed changes
boa/src/object/gcobject.rs Outdated Show resolved Hide resolved
@jedel1043
Copy link
Member Author

Test262 conformance changes:

Test result master count PR count difference
Total 80,918 80,918 0
Passed 33,178 33,280 +102
Ignored 15,898 15,898 0
Failed 31,842 31,740 -102
Panics 0 0 0
Conformance 41.00% 41.13% +0.13%
Fixed tests (102): 
test/language/statements/for-of/arguments-mapped-aliasing.js (previously Failed)
test/language/statements/for-of/arguments-mapped.js (previously Failed)
test/language/arguments-object/10.6-11-b-1.js [strict mode] (previously Failed)
test/language/arguments-object/10.6-11-b-1.js (previously Failed)
test/language/arguments-object/10.6-12-2.js (previously Failed)
test/language/arguments-object/10.6-7-1.js [strict mode] (previously Failed)
test/language/arguments-object/10.6-7-1.js (previously Failed)
test/language/arguments-object/10.6-13-a-3.js (previously Failed)
test/language/arguments-object/10.6-13-c-1-s.js [strict mode] (previously Failed)
test/language/arguments-object/10.6-5-1.js [strict mode] (previously Failed)
test/language/arguments-object/10.6-5-1.js (previously Failed)
test/language/arguments-object/10.6-14-c-4-s.js [strict mode] (previously Failed)
test/language/arguments-object/S10.6_A5_T1.js [strict mode] (previously Failed)
test/language/arguments-object/S10.6_A5_T1.js (previously Failed)
test/language/arguments-object/S10.6_A2.js [strict mode] (previously Failed)
test/language/arguments-object/S10.6_A2.js (previously Failed)
test/language/arguments-object/S10.6_A3_T1.js [strict mode] (previously Failed)
test/language/arguments-object/S10.6_A3_T1.js (previously Failed)
test/language/arguments-object/10.6-13-c-2-s.js [strict mode] (previously Failed)
test/language/arguments-object/10.6-13-c-2-s.js (previously Failed)
test/language/arguments-object/10.6-13-a-1.js (previously Failed)
test/language/arguments-object/10.6-14-c-1-s.js [strict mode] (previously Failed)
test/language/arguments-object/10.6-14-c-1-s.js (previously Failed)
test/language/arguments-object/10.6-10-c-ii-1.js (previously Failed)
test/language/arguments-object/S10.6_A4.js (previously Failed)
test/language/arguments-object/10.6-13-a-2.js (previously Failed)
test/language/arguments-object/10.6-10-c-ii-2.js (previously Failed)
test/language/arguments-object/10.6-13-c-3-s.js [strict mode] (previously Failed)
test/language/arguments-object/10.6-2gs.js [strict mode] (previously Failed)
test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-delete-3.js (previously Failed)
test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-nonwritable-5.js (previously Failed)
test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-nonwritable-3.js (previously Failed)
test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-delete-2.js (previously Failed)
test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-2.js (previously Failed)
test/language/arguments-object/mapped/nonconfigurable-nonenumerable-nonwritable-descriptors-set-by-param.js (previously Failed)
test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-3.js (previously Failed)
test/language/arguments-object/mapped/nonconfigurable-nonenumerable-nonwritable-descriptors-set-by-arguments.js (previously Failed)
test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-delete-4.js (previously Failed)
test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-4.js (previously Failed)
test/language/arguments-object/mapped/writable-enumerable-configurable-descriptor.js (previously Failed)
test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-nonwritable-4.js (previously Failed)
test/built-ins/String/prototype/trim/15.5.4.20-2-51.js [strict mode] (previously Failed)
test/built-ins/String/prototype/trim/15.5.4.20-2-51.js (previously Failed)
test/built-ins/Object/create/15.2.3.5-4-15.js [strict mode] (previously Failed)
test/built-ins/Object/create/15.2.3.5-4-15.js (previously Failed)
test/built-ins/Object/keys/15.2.3.14-6-4.js [strict mode] (previously Failed)
test/built-ins/Object/keys/15.2.3.14-6-4.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-293-2.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-296-1.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-339-4.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-339-4.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-293-3.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-43-1.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-43-1.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-540-10.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-540-10.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-531-7.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-531-7.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-258-1.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-258-1.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-149-1.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-149-1.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-175-1.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-175-1.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-228-1.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-228-1.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-292-1.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-339-2.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-339-2.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-531-16.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-531-16.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-96-1.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-96-1.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-294-1.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-540-5.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-540-5.js (previously Failed)
test/built-ins/Object/prototype/toString/Object.prototype.toString.call-arguments.js [strict mode] (previously Failed)
test/built-ins/Object/prototype/toString/Object.prototype.toString.call-arguments.js (previously Failed)
test/built-ins/Object/getPrototypeOf/15.2.3.2-2-28.js [strict mode] (previously Failed)
test/built-ins/Object/getPrototypeOf/15.2.3.2-2-28.js (previously Failed)
test/built-ins/Object/preventExtensions/15.2.3.10-3-11.js [strict mode] (previously Failed)
test/built-ins/Object/preventExtensions/15.2.3.10-3-11.js (previously Failed)
test/built-ins/Object/preventExtensions/15.2.3.10-3-21.js [strict mode] (previously Failed)
test/built-ins/Object/preventExtensions/15.2.3.10-3-21.js (previously Failed)
test/built-ins/ArrayIteratorPrototype/next/args-mapped-expansion-before-exhaustion.js (previously Failed)
test/built-ins/ArrayIteratorPrototype/next/args-mapped-expansion-after-exhaustion.js (previously Failed)
test/built-ins/ArrayIteratorPrototype/next/args-mapped-truncation-before-exhaustion.js (previously Failed)
test/built-ins/ArrayIteratorPrototype/next/args-mapped-iteration.js (previously Failed)
test/built-ins/Array/prototype/filter/15.4.4.20-1-15.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/filter/15.4.4.20-1-15.js (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-1-15.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-1-15.js (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-1-15.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-1-15.js (previously Failed)
test/built-ins/Array/prototype/map/15.4.4.19-1-15.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/map/15.4.4.19-1-15.js (previously Failed)
test/built-ins/Array/prototype/some/15.4.4.17-1-15.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/some/15.4.4.17-1-15.js (previously Failed)
test/built-ins/Array/prototype/every/15.4.4.16-1-15.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/every/15.4.4.16-1-15.js (previously Failed)
test/built-ins/Array/prototype/forEach/15.4.4.18-1-15.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/forEach/15.4.4.18-1-15.js (previously Failed)

@jedel1043 jedel1043 marked this pull request as ready for review September 20, 2021 06:33
@jedel1043 jedel1043 requested review from RageKnify, Razican, HalidOdat and raskad and removed request for RageKnify September 20, 2021 06:33
raskad
raskad requested changes Sep 21, 2021
View reviewed changes
boa/src/object/gcobject.rs Outdated Show resolved Hide resolved
boa/src/object/internal_methods/arguments.rs Outdated Show resolved Hide resolved
boa/src/object/internal_methods/arguments.rs Outdated Show resolved Hide resolved
boa/src/builtins/function/arguments.rs Outdated Show resolved Hide resolved
@jedel1043 jedel1043 merged commit df836f1 into boa-dev:master Oct 5, 2021
@jedel1043 jedel1043 deleted the args_exotic branch October 5, 2021 16:29
@jedel1043 jedel1043 added this to the v0.14.0 milestone Oct 5, 2021
@jedel1043 jedel1043 added builtins PRs and Issues related to builtins/intrinsics enhancement New feature or request labels Oct 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raskad raskad raskad approved these changes

@RageKnify RageKnify RageKnify approved these changes

@Razican Razican Razican approved these changes

@HalidOdat HalidOdat Awaiting requested review from HalidOdat

Assignees
No one assigned
Labels
builtins PRs and Issues related to builtins/intrinsics enhancement New feature or request
Projects
None yet
Milestone
v0.14.0
Development

Successfully merging this pull request may close these issues.
4 participants
@jedel1043 @Razican @RageKnify @raskad