[libcxxabi] Fewer assumptions about path from libcxx to libcxxabi

[thakis]

Summary

This is useful for projects that pull in libcxx and libcxxabi and build
them using out-of-tree build files, but don't make them sibling
directories (or don't call the sibling directories libcxx and libcxxabi
for some reason).

Fixes PR49313.

[thakis]

(Alternatively, the include could say "../src/include/refstring.h"
since libcxx/include is already added as an include path, but it
sounds like there's a desire to make it get includes from the
build directory instead of the source directory in the future.
So I didn't go with that.)

[rnk]

This looks fine to me, but I feel like we should get approval of this from @ldionne.

[ldionne]

I'm fine with this change per-se, however the problem I have is with Chrome swimming against the current in terms of having the runtimes in non-standard locations.

We introduced this requirement a year ago: https://lists.llvm.org/pipermail/libcxx-dev/2020-March/000714.html. There were even suggestions about merging libc++ and libc++abi, which is something I know Eric has been wanting to do for a long time.

We can make this change, but it would be best for Chrome to change the way it's currently ingesting libc++ and libc++abi to account for those requirements. By the way, I think we had an explicit error at the top of the CMake that would have fired for using libc++/libc++abi in non-standard locations relative to each other, but I removed it some time ago (apparently before you updated your version of the libs).

The bottom line is that we want to be able to assume that we are in a monorepo-like layout in libc++ and libc++abi because it simplifies things, and in the future we might very well depend on some things like a common set of CMake utilities outside of either libcxx/ or libcxxabi/. If you change how you ingest libcxx/libcxxabi right now and ingest the monorepo instead (or at least a subset of the directories in the monorepo, right now only libcxx/ and libcxxabi/ are needed to build either), you won't risk being broken again in the future.

[thakis]

Thanks!

In D97379#2588370, @ldionne wrote:

I'm fine with this change per-se, however the problem I have is with Chrome swimming against the current in terms of having the runtimes in non-standard locations.

We introduced this requirement a year ago: https://lists.llvm.org/pipermail/libcxx-dev/2020-March/000714.html. There were even suggestions about merging libc++ and libc++abi, which is something I know Eric has been wanting to do for a long time.

We can make this change, but it would be best for Chrome to change the way it's currently ingesting libc++ and libc++abi to account for those requirements. By the way, I think we had an explicit error at the top of the CMake that would have fired for using libc++/libc++abi in non-standard locations relative to each other, but I removed it some time ago (apparently before you updated your version of the libs).

Understood.

From what I understand, the main benefits from assuming this is that it makes the cmake stuff easier since you can share more cmake modules etc. Chromium doesn't use the cmake files for libc++(abi), so if the cmake files make this assumption that isn't a problem for us. (Just to be clear: Chromium also doesn't use the gn files in llvm's tree, instead it has some build files downstream for libc++(abi), like for all its other third-party dependencies). I think it's fairly common for projects to deps in source code and have custom build bits. I think that's true for Firefox and Android too, for example. So I think as long as libc++ and libc++abi are distinct projects, making it possible to compile the source files of each in distinct locations is generally useful (as long as it doesn't require big contortions, which in this case it doesn't).

If libc++ and libc++abi would merge into a single repo, that wouldn't be a problem for us either.

The bottom line is that we want to be able to assume that we are in a monorepo-like layout in libc++ and libc++abi because it simplifies things, and in the future we might very well depend on some things like a common set of CMake utilities outside of either libcxx/ or libcxxabi/. If you change how you ingest libcxx/libcxxabi right now and ingest the monorepo instead (or at least a subset of the directories in the monorepo, right now only libcxx/ and libcxxabi/ are needed to build either), you won't risk being broken again in the future.

Making this assumption in cmake files isn't a problem for us. Just making that assumption in source files.

Thanks again!

libcxxabi/src/CMakeLists.txt

if (MSVC_IDE OR XCODE)
   # project scope because they don't have extensions.
   if (MSVC_IDE)
     source_group("Header Files" FILES ${LIBCXXABI_HEADERS})
   endif()
 endif()
 
 include_directories("${LIBCXXABI_LIBCXX_INCLUDES}")
 
+# stdlib_stdexcept.cpp depends on libc++ internals.
+include_directories("${LIBCXXABI_LIBCXX_PATH}")
+
 if (LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL)
   add_definitions(-DHAVE___CXA_THREAD_ATEXIT_IMPL)
 endif()
 
 if (LIBCXXABI_ENABLE_FORGIVING_DYNAMIC_CAST)
   add_definitions(-D_LIBCXXABI_FORGIVING_DYNAMIC_CAST)
 endif()
 

libcxxabi/src/stdlib_stdexcept.cpp

 //===------------------------ stdexcept.cpp -------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "../../libcxx/src/include/refstring.h"
 #include "stdexcept"
 #include "new"
 #include <cstdlib>
 #include <cstring>
 #include <cstdint>
 #include <cstddef>
 
+// This includes an implementation file from libc++.
+#include "src/include/refstring.h"
+
 static_assert(sizeof(std::__libcpp_refstring) == sizeof(const char *), "");
 
 namespace std  // purposefully not using versioning namespace
 {
 
 logic_error::~logic_error() _NOEXCEPT {}
 
 const char*

llvm/utils/gn/secondary/libcxxabi/src/BUILD.gn

 if (target_os == "linux" || target_os == "fuchsia") {
   cxxabi_sources += [ "cxa_thread_atexit.cpp" ]
 }
 
 config("cxxabi_config") {
   include_dirs = [
     "//libcxxabi/include",
     "//libcxx/include",
+
+    # stdlib_stdexcept.cpp depends on libc++ internals.
+    "//libcxx",
   ]
   cflags_cc = [ "-nostdinc++" ]
   defines = [ "_LIBCXXABI_BUILDING_LIBRARY" ]
   if (target_os == "win") {
     defines += [ "_LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS" ]
   }
 }