[Webkit-unassigned] [Bug 108132] Refactor the way HAVE_XXX macros are set

[bugzilla-daemon at webkit.org]

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


Benjamin Poulain <benjamin at webkit.org> changed:

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




--- Comment #6 from Benjamin Poulain <benjamin at webkit.org>  2013-02-07 18:39:50 PST ---
(From update of attachment 186709)
View in context: https://bugs.webkit.org/attachment.cgi?id=186709&action=review

I am not convinced you are making things simpler when splitting blocks.

E.g.:
#if PLATFORM(IOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= 1060
Foo
Bar
#endif

#if OS(DARWIN) && !OS(IOS) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 1060
MacFoobar
#endif

Against:

#if PLATFORM(IOS) || (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 1060)
Foo
Bar
#if !PLATFORM(IOS)
MacFooBar
#endf // !PLATFORM(IOS)
#endif

> Source/WTF/wtf/Platform.h:687
>  #endif

#endif // OS(UNIX)

> Source/WTF/wtf/Platform.h:721
> +#endif

#endif // OS(DARWIN)

> Source/WTF/wtf/Platform.h:723
> +#if OS(IOS) || (OS(DARWIN) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 1060)

It is probably better to avoid OS(IOS) until Dave decided the fate of PLATFORM(MAC) and PLATFORM(IOS).


This?
#if PLATFORM(IOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= 1060
or 
#if PLATFORM(IOS) || (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 1060)

Having DARWIN and __MAC_OS_X_VERSION_MIN_REQUIRED together is not adding readability because iOS is also a Darwin system.

> Source/WTF/wtf/Platform.h:730
> +#if OS(DARWIN) && !OS(IOS) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 1060

PLATFORM(IOS)

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