libc++abi should be responsible for installing its own headers, it
doesn't make sense for libc++ to be responsible for it.
Details
- Reviewers
phosek thakis - Group Reviewers
Restricted Project Restricted Project - Commits
- rG6dfdf79b8c48: [libc++abi] Install the libc++abi headers from libc++abi
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
LGTM
libcxx/cmake/Modules/HandleLibCXXABI.cmake | ||
---|---|---|
15 | We should also consider inlining this macro, I don't think it's getting us anything and I always have to lookup what each argument does so at least in my experience it harms readability. |
Makes sense to me.
Just to confirm: Even back when clang assumed that libc++ headers shipped with the compiler instead of the sdk on macOS (which I think is still the case in currently shipping xcode), it didn't assume that the libc++abi headers ship with the compiler, right? (I patched this in and clang still seems to compile a program that includes <iostream> fine, so it looks like it.)
Is there anything else that installs this header? We don't need it, but someone might?
Fact: Clang used to assume that libc++ was shipped in <toolchain>/include/c++/v1, and nowadays it looks in <sdk>/include/c++/v1 first, and only falls back to <toolchain>/include/c++/v1.
Fact: Recent SDKs have the libc++ headers in <sdk>/usr/include/c++/v1.
Fact: The <cxxabi.h> header is present in both <sdk>/usr/include/c++/v1 and <sdk>/usr/include (and also in <toolchain>/usr/include/c++/v1 until we remove all traces of libc++/libc++abi from the toolchain). It looks like they are always the same thing. It's harmless but silly, since we should really have only one copy. It turns out I don't understand how the copy in <sdk>/usr/include ends up there, so I'll investigate that.
Your main question: I'm pretty sure Clang used to expect <cxxabi.h> in <toolchain>/usr/include/c++/v1, alongside the rest of libc++.
I actually hadn't realized that, and it does change what I was going to do. Instead of always installing <cxxabi.h> to <install>/usr/include, I think the correct thing to do would be to always install it to <install>/usr/include/c++/v1, but to do that directly from libc++abi. My next update will reflect that.
Edit I just found out where <sdk>/usr/include/cxxabi.h comes from. We copy it explicitly here: https://github.com/llvm/llvm-project/blob/main/libcxx/utils/ci/apple-install-libcxx.sh#L159. I wrote that code, I should have known!
libcxx/cmake/Modules/HandleLibCXXABI.cmake | ||
---|---|---|
15 | Oh yeah, totally, I want to clean up this mess, one step at a time. |
Install <cxxabi.h> from libc++abi.
Pinging @dim since I think you folks use a different ABI library - you should make
sure that your ABI library is installing <cxxabi.h> in a place where users can find
it on its own.
Drop the gn changes.
llvm/utils/gn/secondary/libcxx/include/BUILD.gn | ||
---|---|---|
271–281 ↗ | (On Diff #341195) | @thakis I suspect you'll have more fixing up to do since you need to install the libc++abi headers from the libc++abi build. I'll just drop this GN change to avoid muddying the waters, but I'm still giving you this heads up. |
Rebase onto main, and don't touch any logic inside libc++. In other words, only start installing
the headers from libc++abi. I'll keep any potentially-breaking change in a separate patch.
Rebase onto main -- I am unable to reproduce the CI issue with GCC, and it looks like a fluke to me.
We should also consider inlining this macro, I don't think it's getting us anything and I always have to lookup what each argument does so at least in my experience it harms readability.