[CMake][libcxx] Use target_include_directories for libc++ headers

[phosek]

Summary

This is the idiomatic way to handle include directories in CMake.

[urnathan]

runtimes/CMakeLists.txt

95

maybe put a precis of 19227 here (as well as the reference), I like comments but I am the new kid :)
IIUC it's because were doing the clang equivalent of gcc's -nostdinc?

[urnathan]

thanks!

[ldionne]

IIUC, CMake was basically ignoring the target_include_directories(cxx-headers INTERFACE <path>) previously because <path> was part of ${CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES}? Hmm, that's a good find.

LGTM but can you please rebase onto main to get a clean CI run?

[phosek]

Rebase to trigger bots.

[ronlieb]

Hi Peter

i am seeing a buildbot failure in https://lab.llvm.org/buildbot/#/builders/193/builds/11528
after this patch landed. backing up to predecessor patch builds ok. This is during our build of the openmp project for amdgpu.

suggestions please ?

llvm-project/openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
In file included from /home/rlieberm/mono-repo/llvm-project/openmp/libomptarget/plugins/amdgpu/src/rtl.cpp:13:
In file included from /usr/lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/algorithm:62:
In file included from /usr/lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/stl_algo.h:59:
/usr/lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/cstdlib:75:15: fatal error: 'stdlib.h' file not found
#include_next <stdlib.h>

^~~~~~~~~~

1 error generated.

[phosek]

I'll need some time to properly investigate this so I'm going to revert this change in the meantime.

libcxx/include/CMakeLists.txt

add_custom_command(OUTPUT ${dst}
     COMMENT "Copying CXX header ${f}")
   list(APPEND _all_includes "${dst}")
 endforeach()
 
 add_custom_target(generate-cxx-headers ALL DEPENDS ${_all_includes})
 
 add_library(cxx-headers INTERFACE)
 add_dependencies(cxx-headers generate-cxx-headers ${LIBCXX_CXX_ABI_HEADER_TARGET})
-# TODO: Use target_include_directories once we figure out why that breaks the runtimes build
-if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC" OR "${CMAKE_CXX_SIMULATE_ID}" STREQUAL "MSVC")
-  target_compile_options(cxx-headers INTERFACE /I${LIBCXX_GENERATED_INCLUDE_DIR}
-                                     INTERFACE /I${LIBCXX_GENERATED_INCLUDE_TARGET_DIR})
-else()
-  target_compile_options(cxx-headers INTERFACE -I${LIBCXX_GENERATED_INCLUDE_DIR}
-                                     INTERFACE -I${LIBCXX_GENERATED_INCLUDE_TARGET_DIR})
-endif()
+target_include_directories(cxx-headers INTERFACE ${LIBCXX_GENERATED_INCLUDE_DIR}
+                                                 ${LIBCXX_GENERATED_INCLUDE_TARGET_DIR})
 
 if (LIBCXX_INSTALL_HEADERS)
   foreach(file ${files})
     get_filename_component(dir ${file} DIRECTORY)
     install(FILES ${file}
       DESTINATION "${LIBCXX_INSTALL_INCLUDE_DIR}/${dir}"
       COMPONENT cxx-headers
       PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ

runtimes/CMakeLists.txt

 # This variable is used by individual runtimes to locate LLVM files.
 set(LLVM_PATH ${CMAKE_CURRENT_SOURCE_DIR}/../llvm)
 
 include(CheckLibraryExists)
 include(LLVMCheckCompilerLinkerFlag)
 include(CheckCCompilerFlag)
 include(CheckCXXCompilerFlag)
 
+# CMake omits default compiler include paths, but in runtimes build, we use
+# -nostdinc and -nostdinc++ and control include paths manually so this behavior
+# is undesirable. Resetting CMAKE_{LANG}_IMPLICIT_INCLUDE_DIRECTORIES disables
+# this behavior. See https://gitlab.kitware.com/cmake/cmake/-/issues/19227 for
+# further details no this topic.
+#
+# TODO: We may want to consider filtering out only the paths that are within
+# the toolchain installation and leaving the system paths.
+set(CMAKE_C_IMPLICIT_INCLUDE_DIRECTORIES "")
+set(CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES "")
+set(CMAKE_ASM_IMPLICIT_INCLUDE_DIRECTORIES "")
 
 check_c_compiler_flag("" LLVM_RUNTIMES_LINKING_WORKS)
 if (NOT LLVM_RUNTIMES_LINKING_WORKS)
   # The compiler driver may be implicitly trying to link against libunwind,
   # which might not work if libunwind doesn't exist yet. Try to check if
   # --unwindlib=none is supported, and use that if possible.
   # Don't add this if not necessary to fix linking, as it can break using
   # e.g. ASAN/TSAN.