[cfe-commits] [PATCH] Fixing include order for Windows
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
>> 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?
// The builtin includes here
More information about the cfe-commits