jhuber6 added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1230
+ llvm::sys::path::append(P, "llvm-libc");
+ CmdArgs.push_back("-c-isystem");
+ CmdArgs.push_back(Args.MakeArgString(P));
----------------
tra wrote:
> Ensuring the right include order will be tricky. Interaction between the
> headers provided by llvm-libc vs the system headers will be interesting if we
> end up mixing the headers. It may be less problematic compared to the C++
> standard library, but I doubt that mixing two implementations would work well
> here, either.
>
> So, the major question I have -- does adding include path here guarantees
> that we're not picking up the host headers? I do not see any changes that
> would exclude system headers from include paths.
> If we do have include paths leading to both llvm-libc and the host headers,
> what's expected to happen if user code ends up including a header that's not
> present in llvm-libc? Is that possible?
>
Right now I'm just kind of relying on an expectation that since this will be
the first `c-isystem` path set, then it will pull in these libraries first if
they exist. It's not captured by the tests, but compiling with `-v` shows this
path being used first in my experience. So, theoretically, if there is an
implementation of said header in this location, it will be picked up before
anything else. Otherwise it'll just search the other standard locations.
================
Comment at: clang/test/Driver/gpu-libc-headers.c:14
+// RUN: FileCheck %s --check-prefix=CHECK-HEADERS
+// CHECK-HEADERS: "-cc1"{{.*}}"-c-isystem" "{{.*}}include/llvm-libc"
+
----------------
tra wrote:
> I think here we want to test for not just the presence of
> `include/llvm-libc`, but also that it's in the correct position relative to
> other include paths.
>
> We may want something similar to what we have in
> clang/test/Driver/hip-include-path.hip
Yeah, I wasn't sure if there was a good way to guarantee a certain path since
those can change based on the system. Maybe `--sysroot`?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146973/new/
https://reviews.llvm.org/D146973
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
