1249588 - Update RegExpObject group type information to follow ES6 change.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Tooru Fujisawa [:arai]]

https://dxr.mozilla.org/mozilla-central/rev/d5daf10d3b74b04e8fa63c4e5429de8a0adf79f8/js/src/vm/ObjectGroup.cpp#586
>         if (obj->is<RegExpObject>()) {
>             AddTypePropertyId(cx, group, nullptr, NameToId(names.source), TypeSet::StringType());
>             AddTypePropertyId(cx, group, nullptr, NameToId(names.global), TypeSet::BooleanType());
>             AddTypePropertyId(cx, group, nullptr, NameToId(names.ignoreCase), TypeSet::BooleanType());
>             AddTypePropertyId(cx, group, nullptr, NameToId(names.multiline), TypeSet::BooleanType());
>             AddTypePropertyId(cx, group, nullptr, NameToId(names.sticky), TypeSet::BooleanType());
>             AddTypePropertyId(cx, group, nullptr, NameToId(names.lastIndex), TypeSet::Int32Type());
>         }

this list does not include unicode, and that is the reason why unicode flag is handled differently in bug 1249235.

we should either add unicode there, or remove source and flags from there.

Comment 1

[Tooru Fujisawa [:arai]]

jandem, what does the code do actually?
should we call AddTypePropertyId only for own data properties, or including getters in prototype?

Comment 2

[Jan de Mooij [:jandem]]

(In reply to Tooru Fujisawa [:arai] from comment #1)
> jandem, what does the code do actually?
> should we call AddTypePropertyId only for own data properties, or including
> getters in prototype?

We have to do this TI stuff for the properties we set in RegExpObject::assignInitialShape. The idea is that usually we define properties manually on new objects, and in that case the type information is automatically updated. But here we give the object an initial shape that already includes some properties and because we don't explicitly define/add the property on each object, type information has to be updated manually.

Since most of these properties are now getters on the prototype, I think you can remove those from ObjectGroup.cpp. Except for lastIndex, that one is still set in RegExpObject::assignInitialShape.

Comment 3

[Tooru Fujisawa [:arai]]

Attachment: Remove unnecessary type information from RegExpObject.

Thank you for detailed explanation :D

With this, the patch in bug 1249235 works as expected, all flag accesses in RegExp.prototype.flags is folded into single load in GVN.

Comment 4

[Jan de Mooij [:jandem]]

Comment on attachment 8721357 [details] [diff] [review]
Remove unnecessary type information from RegExpObject.

Review of attachment 8721357 [details] [diff] [review]:
-----------------------------------------------------------------

\o/

Comment 5

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/36fdf01130f9d403fd8f03fc3c21c909a4c373f3
Bug 1249588 - Remove unnecessary type information from RegExpObject. r=jandem

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/36fdf01130f9