[cfe-commits] [PATCH] Fixing include order for Windows

Aaron Ballman aaron at aaronballman.com
Sun Feb 19 15:09:12 PST 2012


On Sun, Feb 19, 2012 at 4:45 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
> On Sun, Feb 19, 2012 at 10:28 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
>> On Windows, the default search paths for includes would put the
>> CMake-found headers (<build dir>\lib\clang\3.1\include) before the OS
>> headers.  For the most part, this is fine.  However, there are some
>> header files that share names with the system headers, which are
>> incorrect.  For instance, immintrin.h is completely different from the
>> SDK include.  This causes miscompilations in surprising places because
>> a system header will attempt to include one of these duplicates, and
>> find the clang header instead of the other system header.
>>
>> A simple test case of: #include <map> when building from MSVC
>> demonstrates the issue.
>>
>> A user can work around this behavior by specifying -nobuiltininc on
>> the command line, but that seems rather obtuse for such normal use
>> cases.
>>
>> I've fixed this by changing the order that the headers are included
>> from the toolchain.  The SDK headers get added *before* the clang
>> headers, and this resolves the issue.
>>
>> This patch does cause one regression test to fail
>> (Modules\compiler_builtins.m).  This is because the platform SDK
>> intrinsics are differently structured from the ones that come with
>> clang.  The OS APIs expect the SDK intrinsics, but our tests seem to
>> require our own intrinsics.  I'm open to suggestions.
>
> If on Windows, we specifically need the OS version of immintrin.h, we
> should modify our version to forward to it; completely ignoring the
> builtin headers is a bad idea (for example,
> http://llvm.org/bugs/show_bug.cgi?id=11918 is a miscompile caused by
> pulling in the wrong version of stdarg.h).

The patch doesn't completely ignore the builtin headers though -- it
simply prefers the OS headers over the builtin headers.  That being
said, I am not against forwarding our version.  What would be a good
approach to it?

#ifndef(_MSC_VER)
  // The builtin includes here
#else
  #include <immintrin.h>
#endif

?

~Aaron




More information about the cfe-commits mailing list