Bug 234540

Summary: Upstream some HAVE/ENABLE/USE macros
Product: WebKit Reporter: Tim Horton <thorton>
Component: New BugsAssignee: Tim Horton <thorton>
Status: RESOLVED CONFIGURATION CHANGED    
Severity: Normal CC: akeerthi, annevk, ap, benjamin, cdumez, cmarcelo, ews-watchlist, hi, megan_gardner, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch ap: review-, ews-feeder: commit-queue-

Description Tim Horton 2021-12-20 18:09:42 PST 
Upstream some HAVE/ENABLE/USE macros
Comment 1 Tim Horton 2021-12-20 18:16:56 PST 
Created attachment 447674 [details]
Patch
Comment 2 Alexey Proskuryakov 2021-12-21 15:57:12 PST 
Comment on attachment 447674 [details]
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
Comment 3 Tim Horton 2021-12-21 15:59:26 PST 
(In reply to Alexey Proskuryakov from comment #2)
> Comment on attachment 447674 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=447674&action=review
> 
> Seems like too many red EWSes to r_ as is.

Agreed, will need some SPI header changes!

> > 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.

Good point! I wasn't resolving anything down, just upstreaming, but this is probably the best time to do it...

> > 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.

It does (thankfully), but good point.

> > 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

Nice!
Comment 4 Radar WebKit Bug Importer 2021-12-27 18:10:18 PST 
<rdar://problem/86946926>
Comment 5 Anne van Kesteren 2024-03-15 04:10:33 PDT 
Looks like this got resolved elsewhere.