[webkit-dev] Removing BUILDING_ON / TARGETING macros in favor of system availability macros

[Mark Mentovai]

Tony brought me in to comment on what impact this might have on the
Chromium Mac build. It shouldn’t have any impact. Any use of the
compiler-defined macros is fine.

In Chrome code, we usually use MAC_OS_X_VERSION_MAX_ALLOWED and
MAC_OS_X_VERSION_MIN_REQUIRED from <AvailabilityMacros.h>, along with
symbolic macros like MAC_OS_X_VERSION_10_7 instead of 1070. This has an
annoying disadvantage that older SDKs (like the 10.6 SDK) don’t define
MAC_OS_X_VERSION_10_7, so we’re stuck testing things like
defined(MAC_OS_X_VERSION_10_7) && MAC_OS_X_VERSION_MAX_ALLOWED >=
MAC_OS_X_VERSION_10_7. The same applies when using <Availability.h>, where
the symbolic names are of the form __MAC_10_7, but you seem to have chosen
not to use those, thus condensing the macro logic.

We used <AvailabilityMacros.h>’s MAC_OS_X_VERSION_MAX_ALLOWED and
MAC_OS_X_VERSION_MIN_REQUIRED instead of <Availability.h>’s
__MAC_OS_X_VERSION_MAX_ALLOWED and __MAC_OS_X_VERSION_MIN_REQUIRED because
the latter header’s comments discuss how it’s appropriate for OS headers,
while the former documents its use for user (non-OS) code. TN2064 also
recommends <AvailabilityMacros.h>.

I never liked the WebKit-specific BUILDING_ON_*/TARGETING_* macros and am
happy to see them go.

My one real gripe with the macros from both <Availability.h> and
<AvailabilityMacros.h> is that it’s not terribly obvious which one refers
to the SDK (it’s MAX_ALLOWED) and which refers to the deployment target
(it’s MIN_REQUIRED). Until you’re familiar with the macros, I think it’s
unclear that “allowed” refers to APIs allowed to be called because they’re
present in the SDK, and unclear that “required” refers to the runtime
requirement. This probably leads to more misuse and abuse than if they had
SDK and DT in their names. I guess BUILDING_ON_* and TARGETING_* were a
little better in that regard, but they were misused and abused too. SDKs
and DTs are probably just too confusing to begin with.


On Tue, Jul 10, 2012 at 7:24 PM, Mark Rowe <mrowe at apple.com> wrote:

> I would like to propose removing the BUILDING_ON and TARGETING family of
> macros that are used to build code conditionally for different versions of
> OS X. I propose this in order to address several problems:
>
>
>    - The checks are verbose, and getting worse.
>
>
> For instance, in order to write code targeting OS X 10.8 and newer I have
> to enumerate all other supported OS versions:
>
> #if !defined(TARGETING_LEOPARD) && !defined(TARGETING_SNOW_LEOPARD) &&
> !defined(TARGETING_LION)
> …
> #endif
>
>
> This problem has become worse over time as the number of supported OS X
> versions in the WebKit code base has increased.
>
>
>
>    - The nature of the version checks are often not obvious at first
>    glance.
>
>
> In order to understand the checks you have to first remember the marketing
> names of the various OS X releases. You must then reason about the
> conditional logic of the check itself, which will often contains multiple
> negated clauses.
>
>
>
>    - Almost all current uses are incorrect in the context of SDKs.
>
>
> The vast majority of the checks in WebKit use the BUILDING_ON macros where
> the TARGETING macros would be more appropriate. This hasn't cause many
> problems to date since builds of WebKit on OS X for the most part do not
> use SDKs. This means that they build with __MAC_OS_X_VERSION_MIN_REQUIRED
> == __MAC_OS_X_VERSION_MAX_ALLOWED, resulting in the BUILDING_ON and
> TARGETING macros being equivalent. However, when building with a deployment
> target of an older OS release than the SDK you're building against, the
> BUILDING_ON and TARGETING macros will have different behavior. The result
> is that WebKit fails to build against an SDK when targeting an older OS
> release.
>
>
> My proposed solution to these problems is to remove the BUILDING_ON and
> TARGETING macros. The vast majority of the BUILDING_ON uses and all of the
> TARGETING uses would be replaced with tests against
> __MAC_OS_X_VERSION_MIN_REQUIRED. The small number of uses of BUILDING_ON
> that are being used correctly would be replaced with tests against
> __MAC_OS_X_VERSION_MAX_ALLOWED.
>
> The example above of code targeting OS X 10.8 and newer would become:
>
> #if __MAC_OS_X_VERSION_MIN_REQUIRED >= 1080
> …
> #endif
>
> Code that wishes to target only OS X 10.6 and older would become:
>
> #if __MAC_OS_X_VERSION_MIN_REQUIRED <= 1060
> …
> #endif
>
> This is much more concise and understandable than the current approach.
>
>
> I'm open to feedback on this proposal, but I'd like to move forward with
> this change in the next day or two if no one objects.
>
> - Mark
>
>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20120711/c4af823f/attachment.html>