Support GetOwnProperty for string exotic object #1291

jarkonik · 2021-05-28T18:22:58Z

This Pull Request makes GetOwnProperty for String objects behave according to specification ( https://tc39.es/ecma262/#sec-string-exotic-objects-getownproperty-p )

const str = "abc";
console.log(str[0]);

previously resulted in printing undefined, now it results in printing "a"

RageKnify

I ask for 2 small changes. I would also ask that you look at the 2 new panics to try to understand what introduced them.

Test262 conformance changes:

Test result	master count	PR count	difference
Total	78,873	78,873	0
Passed	26,665	26,717	+52
Ignored	15,604	15,604	0
Failed	36,604	36,552	-52
Panics	14	16	+2
Conformance	33.81%	33.87%	+0.07%

Fixed tests:

test/language/line-terminators/7.3-5.js [strict mode] (previously Failed)
test/language/line-terminators/7.3-5.js (previously Failed)
test/language/line-terminators/7.3-15.js [strict mode] (previously Failed)
test/language/line-terminators/7.3-15.js (previously Failed)
test/language/line-terminators/7.3-6.js [strict mode] (previously Failed)
test/language/line-terminators/7.3-6.js (previously Failed)
test/harness/decimalToHexString.js [strict mode] (previously Failed)
test/harness/decimalToHexString.js (previously Failed)
test/built-ins/Object/getOwnPropertyDescriptor/primitive-string.js [strict mode] (previously Failed)
test/built-ins/Object/getOwnPropertyDescriptor/primitive-string.js (previously Failed)
test/built-ins/Object/getOwnPropertyDescriptor/15.2.3.3-3-14.js [strict mode] (previously Failed)
test/built-ins/Object/getOwnPropertyDescriptor/15.2.3.3-3-14.js (previously Failed)
test/built-ins/Date/prototype/toISOString/15.9.5.43-0-5.js [strict mode] (previously Failed)
test/built-ins/Date/prototype/toISOString/15.9.5.43-0-5.js (previously Failed)
test/built-ins/Array/prototype/filter/15.4.4.20-2-18.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/filter/15.4.4.20-2-18.js (previously Failed)
test/built-ins/Array/prototype/filter/15.4.4.20-1-8.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/filter/15.4.4.20-1-8.js (previously Failed)
test/built-ins/Array/prototype/filter/15.4.4.20-1-7.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/filter/15.4.4.20-1-7.js (previously Failed)
test/built-ins/Array/prototype/indexOf/15.4.4.14-1-8.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/indexOf/15.4.4.14-1-8.js (previously Failed)
test/built-ins/Array/prototype/indexOf/15.4.4.14-2-18.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/indexOf/15.4.4.14-2-18.js (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-9-c-i-28.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-9-c-i-28.js (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-2-18.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-2-18.js (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-1-7.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-1-7.js (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-1-8.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-1-8.js (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-8-b-iii-1-28.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-8-b-iii-1-28.js (previously Failed)
test/built-ins/Array/prototype/lastIndexOf/15.4.4.15-1-8.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/lastIndexOf/15.4.4.15-1-8.js (previously Failed)
test/built-ins/Array/prototype/lastIndexOf/15.4.4.15-2-18.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/lastIndexOf/15.4.4.15-2-18.js (previously Failed)
test/built-ins/Array/prototype/every/15.4.4.16-2-18.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/every/15.4.4.16-2-18.js (previously Failed)
test/built-ins/Array/prototype/some/15.4.4.17-2-18.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/some/15.4.4.17-2-18.js (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-2-18.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-2-18.js (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-1-7.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-1-7.js (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-9-c-i-28.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-9-c-i-28.js (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-8-b-iii-1-28.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-8-b-iii-1-28.js (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-1-8.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-1-8.js (previously Failed)

New panics:

test/built-ins/Array/prototype/concat/Array.prototype.concat_spreadable-string-wrapper.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/concat/Array.prototype.concat_spreadable-string-wrapper.js (previously Failed)

boa/src/object/internal_methods.rs

Razican

Looking good, I would just change a couple of things.

boa/src/object/internal_methods.rs

Co-authored-by: Iban Eguia <razican@protonmail.ch>

jarkonik · 2021-05-29T20:36:03Z

I've managed to fix the new panics, they were caused by emoji character which consists of multiple utf-16 scalar values. v8 in this case just returns string with one of escaped single scalar value(like below) in each index. I've replicated this behaviour in a new commit instead of unwrapping.

const str = new String("😀");
str[0]; // "\ud83d"
str[1]; // "\ude00"

P.S. Similar issue exists in string builtin method charAt

boa/src/object/internal_methods.rs

Razican

Looks good to me! Let's see the test results :)

RageKnify

Numbers look great now.

Test262 conformance changes:

Test result	master count	PR count	difference
Total	78,873	78,873	0
Passed	26,667	26,719	+52
Ignored	15,604	15,604	0
Failed	36,602	36,550	-52
Panics	14	14	0
Conformance	33.81%	33.88%	+0.07%

Fixed tests:

test/language/line-terminators/7.3-5.js [strict mode] (previously Failed)
test/language/line-terminators/7.3-5.js (previously Failed)
test/language/line-terminators/7.3-15.js [strict mode] (previously Failed)
test/language/line-terminators/7.3-15.js (previously Failed)
test/language/line-terminators/7.3-6.js [strict mode] (previously Failed)
test/language/line-terminators/7.3-6.js (previously Failed)
test/harness/decimalToHexString.js [strict mode] (previously Failed)
test/harness/decimalToHexString.js (previously Failed)
test/built-ins/Object/getOwnPropertyDescriptor/primitive-string.js [strict mode] (previously Failed)
test/built-ins/Object/getOwnPropertyDescriptor/primitive-string.js (previously Failed)
test/built-ins/Object/getOwnPropertyDescriptor/15.2.3.3-3-14.js [strict mode] (previously Failed)
test/built-ins/Object/getOwnPropertyDescriptor/15.2.3.3-3-14.js (previously Failed)
test/built-ins/Date/prototype/toISOString/15.9.5.43-0-5.js [strict mode] (previously Failed)
test/built-ins/Date/prototype/toISOString/15.9.5.43-0-5.js (previously Failed)
test/built-ins/Array/prototype/filter/15.4.4.20-2-18.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/filter/15.4.4.20-2-18.js (previously Failed)
test/built-ins/Array/prototype/filter/15.4.4.20-1-8.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/filter/15.4.4.20-1-8.js (previously Failed)
test/built-ins/Array/prototype/filter/15.4.4.20-1-7.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/filter/15.4.4.20-1-7.js (previously Failed)
test/built-ins/Array/prototype/indexOf/15.4.4.14-1-8.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/indexOf/15.4.4.14-1-8.js (previously Failed)
test/built-ins/Array/prototype/indexOf/15.4.4.14-2-18.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/indexOf/15.4.4.14-2-18.js (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-9-c-i-28.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-9-c-i-28.js (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-2-18.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-2-18.js (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-1-7.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-1-7.js (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-1-8.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-1-8.js (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-8-b-iii-1-28.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduce/15.4.4.21-8-b-iii-1-28.js (previously Failed)
test/built-ins/Array/prototype/lastIndexOf/15.4.4.15-1-8.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/lastIndexOf/15.4.4.15-1-8.js (previously Failed)
test/built-ins/Array/prototype/lastIndexOf/15.4.4.15-2-18.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/lastIndexOf/15.4.4.15-2-18.js (previously Failed)
test/built-ins/Array/prototype/every/15.4.4.16-2-18.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/every/15.4.4.16-2-18.js (previously Failed)
test/built-ins/Array/prototype/some/15.4.4.17-2-18.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/some/15.4.4.17-2-18.js (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-2-18.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-2-18.js (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-1-7.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-1-7.js (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-9-c-i-28.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-9-c-i-28.js (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-8-b-iii-1-28.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-8-b-iii-1-28.js (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-1-8.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/reduceRight/15.4.4.22-1-8.js (previously Failed)

jarkonik force-pushed the exotic-string-get-own-property branch from 71e1952 to eed83aa Compare May 28, 2021 18:28

Support GetOwnProperty for string exotic object

8e426f1

jarkonik force-pushed the exotic-string-get-own-property branch from eed83aa to 8e426f1 Compare May 28, 2021 18:28

Razican added bug Something isn't working builtins PRs and Issues related to builtins/intrinsics labels May 28, 2021

Razican requested review from RageKnify, HalidOdat, tofpie and Razican May 28, 2021 20:03

Razican added this to the v0.12.0 milestone May 28, 2021

Fix clippy error

a6801a3

RageKnify suggested changes May 29, 2021

View reviewed changes

boa/src/object/internal_methods.rs Outdated Show resolved Hide resolved

boa/src/object/internal_methods.rs Outdated Show resolved Hide resolved

jarkonik added 2 commits May 29, 2021 21:01

change var name

10d89cd

dont load from_u32 to namespace

c7f158a

Razican requested changes May 29, 2021

View reviewed changes

boa/src/object/internal_methods.rs Outdated Show resolved Hide resolved

boa/src/object/internal_methods.rs Outdated Show resolved Hide resolved

jarkonik and others added 3 commits May 29, 2021 21:27

be explicit about attributes

821378c

Update boa/src/object/internal_methods.rs

859022d

Co-authored-by: Iban Eguia <razican@protonmail.ch>

fix utf16 emoji panic

04043c6

Razican reviewed May 29, 2021

View reviewed changes

boa/src/object/internal_methods.rs Outdated Show resolved Hide resolved

use map and ? operator

4dca641

Razican approved these changes May 30, 2021

View reviewed changes

Razican requested a review from RageKnify May 30, 2021 07:17

RageKnify approved these changes May 30, 2021

View reviewed changes

Razican merged commit 82e8a11 into boa-dev:master May 30, 2021

RageKnify mentioned this pull request Jun 6, 2021

Indexing strings #602

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support GetOwnProperty for string exotic object #1291

Support GetOwnProperty for string exotic object #1291

jarkonik commented May 28, 2021

RageKnify left a comment •

edited by Razican

Loading

Razican left a comment

jarkonik commented May 29, 2021 •

edited

Loading

Razican left a comment

RageKnify left a comment

Support GetOwnProperty for string exotic object #1291

Support GetOwnProperty for string exotic object #1291

Conversation

jarkonik commented May 28, 2021

RageKnify left a comment • edited by Razican Loading

Choose a reason for hiding this comment

Test262 conformance changes:

Razican left a comment

Choose a reason for hiding this comment

jarkonik commented May 29, 2021 • edited Loading

Razican left a comment

Choose a reason for hiding this comment

RageKnify left a comment

Choose a reason for hiding this comment

Test262 conformance changes:

RageKnify left a comment •

edited by Razican

Loading

jarkonik commented May 29, 2021 •

edited

Loading