[Webkit-unassigned] [Bug 107406] Collapse PLATFORM(MAC)||PLATFORM(IOS) to just PLATFORM(MAC)

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=107406


David Kilzer (:ddkilzer) <ddkilzer at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #183694|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #10 from David Kilzer (:ddkilzer) <ddkilzer at webkit.org>  2013-01-26 10:39:08 PST ---
(From update of attachment 183694)
View in context: https://bugs.webkit.org/attachment.cgi?id=183694&action=review

r- to incorporate suggested changes outside of Platform.h.

> Source/JavaScriptCore/runtime/DatePrototype.cpp:65
> -#if PLATFORM(MAC) || PLATFORM(IOS) || (PLATFORM(WX) && OS(DARWIN)) || (PLATFORM(QT) && OS(DARWIN))
> +#if PLATFORM(MAC) || (PLATFORM(WX) && OS(DARWIN)) || (PLATFORM(QT) && OS(DARWIN))
>  #include <CoreFoundation/CoreFoundation.h>

This #if should probably be:  #if USE(CF)

However, this code would also now compile on the following platforms:
PLATFORM(WIN) && !OS(WINCE)
PLATFORM(CHROMIUM) && OS(DARWIN)

The Chromium-on-Mac macro doesn't matter since Chromium uses V8.
Not sure about !WinCE, though.

> Source/JavaScriptCore/runtime/DatePrototype.cpp:128
> -#if PLATFORM(MAC) || PLATFORM(IOS) || (PLATFORM(WX) && OS(DARWIN)) || (PLATFORM(QT) && OS(DARWIN))
> +#if PLATFORM(MAC) || (PLATFORM(WX) && OS(DARWIN)) || (PLATFORM(QT) && OS(DARWIN))

Ditto about switching to #if USE(CF) here.

> Source/WTF/wtf/Platform.h:473
> -#if PLATFORM(MAC) || PLATFORM(IOS)
> +#if PLATFORM(MAC)

I'm going to restate what's already been said, but we need to stop defining PLATFORM(MAC) when building PLATFORM(IOS) (and PLATFORM(IOS_SIMULATOR)) to make the decision explicit about whether a given piece of code supports both Mac and iOS, and to make Mac-only code not require "PLATFORM(MAC) && !PLATFORM(IOS)".  I'll file a follow-up bug.

The other thing I noticed about these uses of "PLATFORM(MAC) || PLATFORM(IOS)" in Platform.h is that' they're all defining other ENABLE() or USE() macros, which is exactly how they should be used since it makes it explicit about which platforms are defining each macro.

To put it another way, using "PLATFORM(MAC) || PLATFORM(IOS)" in Platform.h is good, but using it outside of Platform.h means we're probably missing an ENABLE() or USE() macro.

> Source/WebCore/config.h:148
>  // CoreAnimation is available to IOS, Mac and Windows if using CG
> -#if PLATFORM(MAC) || PLATFORM(IOS) || (PLATFORM(WIN) && USE(CG))
> +#if PLATFORM(MAC) || (PLATFORM(WIN) && USE(CG))
>  #define WTF_USE_CA 1
>  #endif

This exact code is already in Source/WTF/wtf/Platform.h, and <wtf/Platform.h> is included in config.h above.  It should just be removed.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.