r289018 - [Headers] Enable #include_next<float.h> on Darwin

[Aaron Ballman via cfe-commits]

On Tue, Apr 25, 2017 at 5:46 PM, Bruno Cardoso Lopes
<bruno.cardoso at gmail.com> wrote:
> On Tue, Apr 25, 2017 at 2:34 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
>> On Tue, Apr 25, 2017 at 5:33 PM, Bruno Cardoso Lopes
>> <bruno.cardoso at gmail.com> wrote:
>>> On Tue, Apr 25, 2017 at 6:29 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
>>>> On Wed, Dec 7, 2016 at 9:13 PM, Bruno Cardoso Lopes via cfe-commits
>>>> <cfe-commits at lists.llvm.org> wrote:
>>>>> Author: bruno
>>>>> Date: Wed Dec  7 20:13:56 2016
>>>>> New Revision: 289018
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=289018&view=rev
>>>>> Log:
>>>>> [Headers] Enable #include_next<float.h> on Darwin
>>>>>
>>>>> Allows darwin targets to provide additional definitions and
>>>>> implementation specifc values for float.h
>>>>>
>>>>> rdar://problem/21961491
>>>>>
>>>>> Added:
>>>>>     cfe/trunk/test/Headers/Inputs/usr/
>>>>>     cfe/trunk/test/Headers/Inputs/usr/include/
>>>>>     cfe/trunk/test/Headers/Inputs/usr/include/float.h
>>>>>     cfe/trunk/test/Headers/float-darwin.c
>>>>> Modified:
>>>>>     cfe/trunk/lib/Headers/float.h
>>>>
>>>> This commit appears to have caused a regression:
>>>> https://bugs.llvm.org//show_bug.cgi?id=31504
>>>>
>>>> I am running into this on a Snow Leopard system as well, where the
>>>> Integration tests are now failing because float.h cannot be found by
>>>> the system #include_next.
>>>
>>> What's actually happening here is that Snow Leopard (or any < 10.7)
>>> used to ship a /usr/include/float.h, which has by itself another
>>> include_next (which is the one failing), see:
>>>
>>> --
>>> /* This file exists soley to keep Metrowerks' compilers happy.  The version
>>>    used by GCC can be found in /usr/lib/gcc, although it's
>>>    not very informative.  */
>>>
>>> #ifndef _FLOAT_H_
>>>
>>> #if defined(__GNUC__)
>>> #include_next <float.h>
>>> --
>>>
>>> We don't have any reliable way to check for the SDK version at this
>>> level, the current macros we have tell a history about deployment
>>> target, but that won't always solve the issue here. Can you try this
>>> workaround below and let me know if works?
>>>
>>> --
>>> #if (defined(__APPLE__) || (defined(__MINGW32__) || defined(_MSC_VER))) && \
>>>    __STDC_HOSTED__ && __has_include_next(<float.h>)
>>>
>>> #ifdef __APPLE__
>>> #define _FLOAT_H_ // Avoid #include_next'ing float.h content on MacOSX < 10.7
>>> #endif
>>>
>>> #  include_next <float.h>
>>> --
>>>
>>>> I was thinking of modifying lib/Headers/float.h to use the SDK version as mentioned in the bug report, but I share the reporter's question: why was this change needed in the first place? I couldn't find a review for the commit, and don't have access to the rdar link.
>>>
>>> We need it to have flexibility to implement additional stuff for
>>> float.h on future SDKs. There was no good reason for not upstreaming
>>> it. I never got to the point of dealing with this specific issue
>>> though, thanks for pinging.
>>
>> What good timing. I was about to post this as a possible fix:
>>
>> Index: lib/Headers/float.h
>> ===================================================================
>> --- lib/Headers/float.h (revision 135915)
>> +++ lib/Headers/float.h (working copy)
>> @@ -31 +31 @@
>> - * Also fall back on Darwin to allow additional definitions and
>> + * Also fall back on Darwin 10.7 or later to allow additional definitions and
>> @@ -34 +34,4 @@
>> -#if (defined(__APPLE__) || (defined(__MINGW32__) || defined(_MSC_VER))) && \
>> +#if ((defined(__APPLE__) &&
>>          \
>> +      defined(__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__) &&
>>          \
>> +      __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 1070) ||
>>          \
>> +     (defined(__MINGW32__) || defined(_MSC_VER))) &&
>>          \
>>
>> Would this be an improvement over your suggestion?
>
> This would be fine for most cases I believe, but if you're trying to
> deploy to a target >= 1070 while using Snow Leopard you would still
> get the error.

Okay, that makes sense to me. Thank you! I've tested your patch and it
does work for us on both 10.6 and 10.12.

~Aaron

>
>> ~Aaron
>>
>>>
>>> --
>>> Bruno Cardoso Lopes
>>> http://www.brunocardoso.cc
>
>
>
> --
> Bruno Cardoso Lopes
> http://www.brunocardoso.cc