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

[Chandler Carruth]

On Sun, Feb 19, 2012 at 3:09 PM, Aaron Ballman <aaron at aaronballman.com>wrote:

> 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 doesn't work in the general case though -- look at all the places in
our builtin headers where we specifically forward *to* the system headers.
If the builtin doesn't come first, lots of things go wrong here.

We should make our builtins always come first, and always be correct when
they come first.

That being
> said, I am not against forwarding our version.  What would be a good
> approach to it?
>

Look at the other builtin headers that use '__has_include_next' and
'include_next' to fold together a Clang builtin header and a system builtin
header.

I suspect you will have to essentially tack on the system 'immintrin.h'
header to the end of the builtin, as we do expect #include <immintrin.h> to
pull in the Clang builtin bits... Essentially, this doesn't seem like it
should be either-or, it should be both.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120219/157ee239/attachment.html>