tra 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));
----------------
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?



================
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"
+
----------------
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


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

Reply via email to