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

Refactor regexp costructor #1434

Merged
merged 5 commits into from
Aug 6, 2021
Merged

Refactor regexp costructor #1434

merged 5 commits into from
Aug 6, 2021

Conversation

raskad
Copy link
Member

@raskad raskad commented Jul 27, 2021

This Pull Request relates to #1304.

It changes the following:

  • Refactor the regexp constructor to match the spec
  • Fix various errors in regexp functions

The test built-ins/RegExp/prototype/Symbol.split/limit-0-bail.js is now failing, because Regexp[Symbol.split] gets the flags property from the regexp prototype, which is undefined. I'm not sure where the root cause for that is, as everything in the immediate call path seems spec compliant to me. But I think we can fix that later as the split implementation itself seems spec compliant.

@raskad raskad mentioned this pull request Jul 27, 2021
Closed
8 tasks
@raskad
Copy link
Member Author

raskad commented Jul 27, 2021

Test result master count PR count difference
Total 78,897 78,897 0
Passed 28,432 28,616 +184
Ignored 15,614 15,614 0
Failed 34,851 34,667 -184
Panics 2 2 0
Conformance 36.04% 36.27% +0.23%
Fixed tests: 
test/built-ins/Array/isArray/15.4.3.2-1-10.js [strict mode] (previously Failed)
test/built-ins/Array/isArray/15.4.3.2-1-10.js (previously Failed)
test/built-ins/Array/prototype/lastIndexOf/15.4.4.15-5-21.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/lastIndexOf/15.4.4.15-5-21.js (previously Failed)
test/built-ins/Array/prototype/indexOf/15.4.4.14-1-12.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/indexOf/15.4.4.14-1-12.js (previously Failed)
test/built-ins/Array/prototype/filter/15.4.4.20-1-12.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/filter/15.4.4.20-1-12.js (previously Failed)
test/built-ins/Array/prototype/filter/15.4.4.20-9-c-iii-23.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/filter/15.4.4.20-9-c-iii-23.js (previously Failed)
test/built-ins/Array/prototype/filter/15.4.4.20-5-16.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/filter/15.4.4.20-5-16.js (previously Failed)
test/built-ins/Array/prototype/every/15.4.4.16-5-16.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/every/15.4.4.16-5-16.js (previously Failed)
test/built-ins/Array/prototype/every/15.4.4.16-7-c-iii-22.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/every/15.4.4.16-7-c-iii-22.js (previously Failed)
test/built-ins/Array/prototype/every/15.4.4.16-1-12.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/every/15.4.4.16-1-12.js (previously Failed)
test/built-ins/Array/prototype/some/15.4.4.17-5-16.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/some/15.4.4.17-5-16.js (previously Failed)
test/built-ins/Array/prototype/some/15.4.4.17-1-12.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/some/15.4.4.17-1-12.js (previously Failed)
test/built-ins/Array/prototype/some/15.4.4.17-7-c-iii-22.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/some/15.4.4.17-7-c-iii-22.js (previously Failed)
test/built-ins/Array/prototype/forEach/15.4.4.18-1-12.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/forEach/15.4.4.18-1-12.js (previously Failed)
test/built-ins/Array/prototype/forEach/15.4.4.18-5-16.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/forEach/15.4.4.18-5-16.js (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-1-12.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-1-12.js (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-9-c-ii-32.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-9-c-ii-32.js (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-1-12.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-1-12.js (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-9-c-ii-32.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-9-c-ii-32.js (previously Failed)
test/built-ins/Array/prototype/map/15.4.4.19-1-12.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/map/15.4.4.19-1-12.js (previously Failed)
test/built-ins/Array/prototype/map/15.4.4.19-5-16.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/map/15.4.4.19-5-16.js (previously Failed)
test/built-ins/Object/defineProperties/15.2.3.7-5-b-79.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperties/15.2.3.7-5-b-79.js (previously Failed)
test/built-ins/Object/defineProperties/15.2.3.7-5-b-104.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperties/15.2.3.7-5-b-104.js (previously Failed)
test/built-ins/Object/defineProperties/15.2.3.7-5-b-158.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperties/15.2.3.7-5-b-158.js (previously Failed)
test/built-ins/Object/defineProperties/15.2.3.7-5-b-26.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperties/15.2.3.7-5-b-26.js (previously Failed)
test/built-ins/Object/defineProperties/15.2.3.7-5-b-183.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperties/15.2.3.7-5-b-183.js (previously Failed)
test/built-ins/Object/defineProperties/15.2.3.7-5-b-132.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperties/15.2.3.7-5-b-132.js (previously Failed)
test/built-ins/Object/defineProperties/15.2.3.7-5-b-211.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperties/15.2.3.7-5-b-211.js (previously Failed)
test/built-ins/Object/defineProperties/15.2.3.7-5-a-14.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperties/15.2.3.7-5-a-14.js (previously Failed)
test/built-ins/Object/defineProperties/15.2.3.7-5-b-246.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperties/15.2.3.7-5-b-246.js (previously Failed)
test/built-ins/Object/defineProperties/15.2.3.7-2-13.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperties/15.2.3.7-2-13.js (previously Failed)
test/built-ins/Object/defineProperties/15.2.3.7-5-b-51.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperties/15.2.3.7-5-b-51.js (previously Failed)
test/built-ins/Object/getPrototypeOf/15.2.3.2-2-26.js [strict mode] (previously Failed)
test/built-ins/Object/getPrototypeOf/15.2.3.2-2-26.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-40.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-40.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-585.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-585.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-40.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-40.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-118.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-118.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-393.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-393.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-172-1.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-172-1.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-255.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-255.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-146.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-146.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-409.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-4-409.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-172.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-172.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-65.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-65.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-93.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-93.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-255-1.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-255-1.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-40-1.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-40-1.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-93-1.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-93-1.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-146-1.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-146-1.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-197.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-197.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-225.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-225.js (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-225-1.js [strict mode] (previously Failed)
test/built-ins/Object/defineProperty/15.2.3.6-3-225-1.js (previously Failed)
test/built-ins/Object/create/15.2.3.5-4-12.js [strict mode] (previously Failed)
test/built-ins/Object/create/15.2.3.5-4-12.js (previously Failed)
test/built-ins/Object/create/15.2.3.5-4-66.js [strict mode] (previously Failed)
test/built-ins/Object/create/15.2.3.5-4-66.js (previously Failed)
test/built-ins/Object/create/15.2.3.5-4-144.js [strict mode] (previously Failed)
test/built-ins/Object/create/15.2.3.5-4-144.js (previously Failed)
test/built-ins/Object/create/15.2.3.5-4-198.js [strict mode] (previously Failed)
test/built-ins/Object/create/15.2.3.5-4-198.js (previously Failed)
test/built-ins/Object/create/15.2.3.5-4-223.js [strict mode] (previously Failed)
test/built-ins/Object/create/15.2.3.5-4-223.js (previously Failed)
test/built-ins/Object/create/15.2.3.5-4-119.js [strict mode] (previously Failed)
test/built-ins/Object/create/15.2.3.5-4-119.js (previously Failed)
test/built-ins/Object/create/15.2.3.5-4-172.js [strict mode] (previously Failed)
test/built-ins/Object/create/15.2.3.5-4-172.js (previously Failed)
test/built-ins/Object/create/15.2.3.5-4-91.js [strict mode] (previously Failed)
test/built-ins/Object/create/15.2.3.5-4-91.js (previously Failed)
test/built-ins/Object/create/15.2.3.5-4-250.js [strict mode] (previously Failed)
test/built-ins/Object/create/15.2.3.5-4-250.js (previously Failed)
test/built-ins/Object/create/15.2.3.5-4-35.js [strict mode] (previously Failed)
test/built-ins/Object/create/15.2.3.5-4-35.js (previously Failed)
test/built-ins/Object/create/15.2.3.5-4-286.js [strict mode] (previously Failed)
test/built-ins/Object/create/15.2.3.5-4-286.js (previously Failed)
test/built-ins/Object/prototype/toString/Object.prototype.toString.call-regexp.js [strict mode] (previously Failed)
test/built-ins/Object/prototype/toString/Object.prototype.toString.call-regexp.js (previously Failed)
test/built-ins/String/prototype/matchAll/regexp-matchAll-is-undefined-or-null.js [strict mode] (previously Failed)
test/built-ins/String/prototype/matchAll/regexp-matchAll-is-undefined-or-null.js (previously Failed)
test/built-ins/String/prototype/matchAll/regexp-is-undefined-or-null-invokes-matchAll.js [strict mode] (previously Failed)
test/built-ins/String/prototype/matchAll/regexp-is-undefined-or-null-invokes-matchAll.js (previously Failed)
test/built-ins/String/prototype/matchAll/regexp-prototype-has-no-matchAll.js [strict mode] (previously Failed)
test/built-ins/String/prototype/matchAll/regexp-prototype-has-no-matchAll.js (previously Failed)
test/built-ins/String/prototype/match/S15.5.4.10_A1_T4.js [strict mode] (previously Failed)
test/built-ins/String/prototype/match/S15.5.4.10_A1_T4.js (previously Failed)
test/built-ins/String/prototype/match/invoke-builtin-match.js [strict mode] (previously Failed)
test/built-ins/String/prototype/match/invoke-builtin-match.js (previously Failed)
test/built-ins/RegExp/S15.10.4.1_A8_T13.js [strict mode] (previously Failed)
test/built-ins/RegExp/S15.10.4.1_A8_T13.js (previously Failed)
test/built-ins/RegExp/S15.10.4.1_A5_T7.js [strict mode] (previously Failed)
test/built-ins/RegExp/S15.10.4.1_A5_T7.js (previously Failed)
test/built-ins/RegExp/S15.10.4.1_A5_T1.js [strict mode] (previously Failed)
test/built-ins/RegExp/S15.10.4.1_A5_T1.js (previously Failed)
test/built-ins/RegExp/S15.10.4.1_A5_T9.js [strict mode] (previously Failed)
test/built-ins/RegExp/S15.10.4.1_A5_T9.js (previously Failed)
test/built-ins/RegExp/S15.10.3.1_A2_T2.js [strict mode] (previously Failed)
test/built-ins/RegExp/S15.10.3.1_A2_T2.js (previously Failed)
test/built-ins/RegExp/S15.10.3.1_A2_T1.js [strict mode] (previously Failed)
test/built-ins/RegExp/S15.10.3.1_A2_T1.js (previously Failed)
test/built-ins/RegExp/S15.10.4.1_A7_T2.js [strict mode] (previously Failed)
test/built-ins/RegExp/S15.10.4.1_A7_T2.js (previously Failed)
test/built-ins/RegExp/S15.10.4.1_A5_T8.js [strict mode] (previously Failed)
test/built-ins/RegExp/S15.10.4.1_A5_T8.js (previously Failed)
test/built-ins/RegExp/S15.10.4.1_A5_T6.js [strict mode] (previously Failed)
test/built-ins/RegExp/S15.10.4.1_A5_T6.js (previously Failed)
test/built-ins/RegExp/S15.10.4.1_A8_T12.js [strict mode] (previously Failed)
test/built-ins/RegExp/S15.10.4.1_A8_T12.js (previously Failed)
test/built-ins/RegExp/15.10.4.1-1.js [strict mode] (previously Failed)
test/built-ins/RegExp/15.10.4.1-1.js (previously Failed)
test/built-ins/RegExp/S15.10.4.1_A2_T2.js [strict mode] (previously Failed)
test/built-ins/RegExp/S15.10.4.1_A2_T2.js (previously Failed)
test/built-ins/RegExp/S15.10.4.1_A5_T4.js [strict mode] (previously Failed)
test/built-ins/RegExp/S15.10.4.1_A5_T4.js (previously Failed)
test/built-ins/RegExp/15.10.4.1-3.js [strict mode] (previously Failed)
test/built-ins/RegExp/15.10.4.1-3.js (previously Failed)
test/built-ins/RegExp/prototype/Symbol.search/success-get-index-err.js [strict mode] (previously Failed)
test/built-ins/RegExp/prototype/Symbol.search/success-get-index-err.js (previously Failed)
test/built-ins/RegExp/prototype/Symbol.search/set-lastindex-init-samevalue.js [strict mode] (previously Failed)
test/built-ins/RegExp/prototype/Symbol.search/set-lastindex-init-samevalue.js (previously Failed)
test/built-ins/RegExp/prototype/Symbol.search/set-lastindex-restore-err.js [strict mode] (previously Failed)
test/built-ins/RegExp/prototype/Symbol.search/set-lastindex-restore-err.js (previously Failed)
test/built-ins/RegExp/prototype/Symbol.search/set-lastindex-restore-samevalue.js [strict mode] (previously Failed)
test/built-ins/RegExp/prototype/Symbol.search/set-lastindex-restore-samevalue.js (previously Failed)
test/built-ins/RegExp/prototype/Symbol.replace/get-unicode-error.js [strict mode] (previously Failed)
test/built-ins/RegExp/prototype/Symbol.replace/get-unicode-error.js (previously Failed)
test/built-ins/RegExp/prototype/Symbol.replace/arg-2-coerce-err.js [strict mode] (previously Failed)
test/built-ins/RegExp/prototype/Symbol.replace/arg-2-coerce-err.js (previously Failed)
test/built-ins/RegExp/prototype/Symbol.replace/subst-capture-idx-2.js [strict mode] (previously Failed)
test/built-ins/RegExp/prototype/Symbol.replace/subst-capture-idx-2.js (previously Failed)
test/built-ins/RegExp/prototype/Symbol.replace/poisoned-stdlib.js [strict mode] (previously Failed)
test/built-ins/RegExp/prototype/Symbol.replace/poisoned-stdlib.js (previously Failed)
test/built-ins/RegExp/prototype/Symbol.replace/subst-capture-idx-1.js [strict mode] (previously Failed)
test/built-ins/RegExp/prototype/Symbol.replace/subst-capture-idx-1.js (previously Failed)
test/language/expressions/delete/11.4.1-5-a-28-s.js [strict mode] (previously Failed)
test/language/expressions/delete/11.4.1-5-a-28-s.js (previously Failed)
test/language/expressions/typeof/built-in-exotic-objects-no-call.js [strict mode] (previously Failed)
test/language/expressions/typeof/built-in-exotic-objects-no-call.js (previously Failed)
Broken tests: 
test/built-ins/RegExp/prototype/Symbol.split/limit-0-bail.js [strict mode] (previously Passed)
test/built-ins/RegExp/prototype/Symbol.split/limit-0-bail.js (previously Passed)

@HalidOdat HalidOdat added bug Something isn't working builtins PRs and Issues related to builtins/intrinsics labels Jul 28, 2021
@HalidOdat HalidOdat added this to the v0.13.0 milestone Jul 28, 2021
HalidOdat
HalidOdat reviewed Jul 28, 2021
View reviewed changes
Copy link
Member

@HalidOdat HalidOdat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good :)

Just a small change.

boa/src/builtins/regexp/mod.rs Show resolved Hide resolved
HalidOdat
HalidOdat reviewed Jul 31, 2021
View reviewed changes
boa/src/builtins/regexp/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/regexp/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/regexp/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/regexp/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/regexp/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/regexp/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/string/mod.rs Show resolved Hide resolved
@raskad
Copy link
Member Author

raskad commented Aug 3, 2021

@HalidOdat I rebased this and implemented your suggestions. I refactored a few more things in the RegExp function directly use GcObject instead of Value. You may want to look at 152f081 to see the refactor.

HalidOdat
HalidOdat approved these changes Aug 6, 2021
View reviewed changes
Copy link
Member

@HalidOdat HalidOdat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me :)

@HalidOdat HalidOdat merged commit ebdf89c into boa-dev:master Aug 6, 2021
@raskad raskad deleted the regexp-costructor branch August 14, 2021 00:33
@raskad raskad mentioned this pull request Sep 28, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HalidOdat HalidOdat HalidOdat approved these changes

Assignees
No one assigned
Labels
bug Something isn't working builtins PRs and Issues related to builtins/intrinsics
Projects
None yet
Milestone
v0.13.0
Development

Successfully merging this pull request may close these issues.
2 participants
@raskad @HalidOdat