This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by phosek on Mar 28 2022, 2:14 PM.

Details

Summary

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

Diff Detail

Event Timeline

phosek created this revision.Mar 28 2022, 2:14 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 28 2022, 2:14 PM
Herald added a subscriber: mgorny. · View Herald Transcript
phosek requested review of this revision.Mar 28 2022, 2:14 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMar 28 2022, 2:14 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript
phosek updated this revision to Diff 418799.Mar 28 2022, 11:30 PM
urnathan added inline comments.
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?

phosek updated this revision to Diff 419870.Apr 1 2022, 3:14 PM
phosek marked an inline comment as done.
urnathan accepted this revision.Apr 6 2022, 4:25 AM

thanks!

ldionne accepted this revision.Apr 8 2022, 1:54 PM

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?

This revision is now accepted and ready to land.Apr 8 2022, 1:54 PM
phosek updated this revision to Diff 425419.Apr 26 2022, 11:18 PM

Rebase to trigger bots.

This revision was landed with ongoing or failed builds.May 6 2022, 2:06 PM
This revision was automatically updated to reflect the committed changes.
ronlieb added a subscriber: ronlieb.May 6 2022, 9:57 PM

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.

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

phosek reopened this revision.May 6 2022, 10:20 PM
This revision is now accepted and ready to land.May 6 2022, 10:20 PM
phosek updated this revision to Diff 436256.Jun 12 2022, 3:10 PM
This revision was landed with ongoing or failed builds.Jun 12 2022, 3:18 PM
This revision was automatically updated to reflect the committed changes.