Bug 213587 – [Intl] Disprefer using ICU enums directly as instance variables

• Component: New Bugs
• Status: RESOLVED FIXED

[Ross Kirsling]

[Intl] Disprefer using ICU enums directly as instance variables

[Ross Kirsling]

Created attachment 402707 [details]
Patch

[Ross Kirsling]

Here's the patch whether we choose to use it or not.

Either way, IntlPluralRules is 72 bytes (padding 6 → 9) and IntlRelativeTimeFormat is 56 bytes (padding 3 → 6).

[Yusuke Suzuki]

Comment on attachment 402707 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=402707&action=review

r=me

> Source/JavaScriptCore/runtime/IntlPluralRules.h:83
> +    bool m_isOrdinal { false };

I think `enum class OrdinalType : uint8_t { Ordinal, Cardinal };` would be better.

[Ross Kirsling]

Created attachment 402708 [details]
Patch for landing

[EWS]

Committed r263497: <https://trac.webkit.org/changeset/263497>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 402708 [details].

[Darin Adler]

Comment on attachment 402707 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=402707&action=review

>> Source/JavaScriptCore/runtime/IntlPluralRules.h:83
>> +    bool m_isOrdinal { false };
> 
> I think `enum class OrdinalType : uint8_t { Ordinal, Cardinal };` would be better.

Why not "enum class OrdinalType : bool { Cardinal, Ordinal };"?

[Ross Kirsling]

(In reply to Darin Adler from comment #6)
> Comment on attachment 402707 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=402707&action=review
> 
> >> Source/JavaScriptCore/runtime/IntlPluralRules.h:83
> >> +    bool m_isOrdinal { false };
> > 
> > I think `enum class OrdinalType : uint8_t { Ordinal, Cardinal };` would be better.
> 
> Why not "enum class OrdinalType : bool { Cardinal, Ordinal };"?

I landed it as Type : bool.

[Radar WebKit Bug Importer]

<rdar://problem/64781188>