[INTL] Remove INTL sub-feature compile flags
Created attachment 345877 [details] Patch
There was an ENABLE_JAVASCRIPT_I18N_API flag I couldn't find actually used anywhere, and the ENABLE_INTL_NUMBER_FORMAT_TO_PARTS and ENABLE_INTL_PLURAL_RULES only served to set the default for their respective runtime flags. Neither were used substantially to change compile behavior, so this removes them, and the runtime flags will be used exclusively. Hopefully this makes it easier for GTK to enable these features. https://bugs.webkit.org/show_bug.cgi?id=185714
Created attachment 345880 [details] Patch
Comment on attachment 345880 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=345880&action=review Yay, fewer build flags! This looks good, except perhaps for one bit I have questions about: > Source/JavaScriptCore/runtime/Options.h:115 > -#if ENABLE(INTL_NUMBER_FORMAT_TO_PARTS) && (!PLATFORM(COCOA) || ENABLE(EXPERIMENTAL_FEATURES)) > +#if (!PLATFORM(COCOA) || ENABLE(EXPERIMENTAL_FEATURES)) So the intention is for these features to be enabled for Apple only if experimental features are enabled, but enabled on other ports always? Isn't that pretty weird? What's the reasoning behind this? Are the features expected to be more stable on non-Apple ports? Surely this is enabling the feature for WPE/GTK right here?
(In reply to Michael Catanzaro from comment #4) > Comment on attachment 345880 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=345880&action=review > > Yay, fewer build flags! This looks good, except perhaps for one bit I have > questions about: > > > Source/JavaScriptCore/runtime/Options.h:115 > > -#if ENABLE(INTL_NUMBER_FORMAT_TO_PARTS) && (!PLATFORM(COCOA) || ENABLE(EXPERIMENTAL_FEATURES)) > > +#if (!PLATFORM(COCOA) || ENABLE(EXPERIMENTAL_FEATURES)) > > So the intention is for these features to be enabled for Apple only if > experimental features are enabled, but enabled on other ports always? Isn't > that pretty weird? What's the reasoning behind this? Are the features > expected to be more stable on non-Apple ports? > > Surely this is enabling the feature for WPE/GTK right here? I could remove the constexpr here, and set it to default false below. Otherwise, as is, it is enabling it by default for all non-Apple ports. Is there another file where ports can enable runtime flags? I didn't add those conditions before, so I'm not positive they are correct without the INTL flags. That's why I haven't cq=? yet.
(In reply to Andy VanWagoner from comment #5) > I could remove the constexpr here, and set it to default false below. > Otherwise, as is, it is enabling it by default for all non-Apple ports. Is > there another file where ports can enable runtime flags? No, this is the right place for JavaScriptCore options. It was changed in bug #186319. > I didn't add those conditions before, so I'm not positive they are correct > without the INTL flags. That's why I haven't cq=? yet. I think it's not worse than before, but it does enable the feature for non-Cocoa ports that use ENABLE(INTL), so that should be mentioned in the ChangeLog... but, looking through bug #186319, I think we should just change the guard to: #if ENABLE(EXPERIMENTAL_FEATURES) and then close bug #185714. Then, whenever the ENABLE(EXPERIMENTAL(FEATURES) guard is removed, it will become available automatically for all ports with newer ICU.
Created attachment 345891 [details] Patch
Comment on attachment 345891 [details] Patch Clearing flags on attachment: 345891 Committed r234293: <https://trac.webkit.org/changeset/234293>
All reviewed patches have been landed. Closing bug.
<rdar://problem/42650262>