[webkit-reviews] review denied: [Bug 234540] Upstream some HAVE/ENABLE/USE macros : [Attachment 447674] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has denied Tim Horton <thorton at apple.com>'s
request for review:
Bug 234540: Upstream some HAVE/ENABLE/USE macros
https://bugs.webkit.org/show_bug.cgi?id=234540

Attachment 447674: Patch

https://bugs.webkit.org/attachment.cgi?id=447674&action=review




--- Comment #2 from Alexey Proskuryakov <ap at webkit.org> ---
Comment on attachment 447674
  --> https://bugs.webkit.org/attachment.cgi?id=447674
Patch

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

Seems like too many red EWSes to r_ as is.

> Source/WTF/wtf/PlatformEnableCocoa.h:554
> +    && ((PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 150000) \

__IPHONE_OS_VERSION_MIN_REQUIRED is always >= 150000

This is repeated many times in the patch, I'll only comment once.

> Source/WTF/wtf/PlatformEnableCocoa.h:589
> +    && (((PLATFORM(IOS) || PLATFORM(MACCATALYST)) &&
__IPHONE_OS_VERSION_MIN_REQUIRED >= 150000) \

Does PLATFORM(MACCATALYST) get a __IPHONE_OS_VERSION_MIN_REQUIRED? One way or
another, version check is not needed any more.

> Source/WTF/wtf/PlatformEnableCocoa.h:590
> +    || (PLATFORM(WATCHOS) && __WATCH_OS_VERSION_MIN_REQUIRED >= 80000) \

Well, and same for watchOS/tvOS.

> Source/WTF/wtf/PlatformHave.h:1119
> +#if (PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 130000) ||
(PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101500)

And for __MAC_OS_X_VERSION_MIN_REQUIRED >= 101500