On Tue, Apr 25, 2017 at 5:46 PM, Bruno Cardoso Lopes <bruno.card...@gmail.com> wrote: > On Tue, Apr 25, 2017 at 2:34 PM, Aaron Ballman <aa...@aaronballman.com> wrote: >> On Tue, Apr 25, 2017 at 5:33 PM, Bruno Cardoso Lopes >> <bruno.card...@gmail.com> wrote: >>> On Tue, Apr 25, 2017 at 6:29 AM, Aaron Ballman <aa...@aaronballman.com> >>> wrote: >>>> On Wed, Dec 7, 2016 at 9:13 PM, Bruno Cardoso Lopes via cfe-commits >>>> <cfe-commits@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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
