[libcxx-commits] [libcxx] Simplify the __assertion_handler build logic. Be friendly to IDEs. (PR #93333)

[via libcxx-commits]

https://github.com/EricWF created https://github.com/llvm/llvm-project/pull/93333

Instead of having no default __assertion_handler in-tree and depending
on the build system to produce it, this change makes the default
assertion handler just be __assertion_handler.

Users can still override the file in the same way, by providing the
LIBCXX_ASSERTION_HANDLER_FILE Cmake variable, and everything still works
the same.

However, with this change we've

1. Simplified people reading and writing our headers. Having out-of-tree
generated headers make development harder for everyone.

2. Made the in-tree headers, which are the files developers edit,
   actually compileable so IDE's can (with a little more help) actually
consume these headers.

3. Decoupled the validity of the in-tree headers from the build system.

Ideally, we should be building using the in-tree headers with a
generated directory, similar to the target generated include dir, for
storing configure-time generated files. Historically there has been
push-back against this that has been addressed by better CI and testing.


>From 4d0f7c98edc6a98b1a38ab25500dce4645d8fab6 Mon Sep 17 00:00:00 2001
From: Eric Fiselier <eric at efcs.ca>
Date: Fri, 10 May 2024 14:55:02 -0400
Subject: [PATCH] Simplify the __assertion_handler build logic. Be friendly to
 IDEs.

Instead of having no default __assertion_handler in-tree and depending
on the build system to produce it, this change makes the default
assertion handler just be __assertion_handler.

Users can still override the file in the same way, by providing the
LIBCXX_ASSERTION_HANDLER_FILE Cmake variable, and everything still works
the same.

However, with this change we've

1. Simplified people reading and writing our headers. Having out-of-tree
generated headers make development harder for everyone.

2. Made the in-tree headers, which are the files developers edit,
   actually compileable so IDE's can (with a little more help) actually
consume these headers.

3. Decoupled the validity of the in-tree headers from the build system.

Ideally, we should be building using the in-tree headers with a
generated directory, similar to the target generated include dir, for
storing configure-time generated files. Historically there has been
push-back against this that has been addressed by better CI and testing.
---
 libcxx/CMakeLists.txt                               |  2 +-
 libcxx/docs/BuildingLibcxx.rst                      | 13 +++++++------
 .../__assertion_handler}                            |  0
 libcxx/include/module.modulemap                     |  1 +
 4 files changed, 9 insertions(+), 7 deletions(-)
 rename libcxx/{vendor/llvm/default_assertion_handler.in => include/__assertion_handler} (100%)

diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index f34cb178e076e..6c80dc2f66070 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -70,7 +70,7 @@ if (NOT "${LIBCXX_HARDENING_MODE}" IN_LIST LIBCXX_SUPPORTED_HARDENING_MODES)
     "Unsupported hardening mode: '${LIBCXX_HARDENING_MODE}'. Supported values are ${LIBCXX_SUPPORTED_HARDENING_MODES}.")
 endif()
 set(LIBCXX_ASSERTION_HANDLER_FILE
-  "${CMAKE_CURRENT_SOURCE_DIR}/vendor/llvm/default_assertion_handler.in"
+  "${CMAKE_CURRENT_SOURCE_DIR}/include/__assertion_handler"
   CACHE STRING
   "Specify the path to a header that contains a custom implementation of the
    assertion handler that gets invoked when a hardening assertion fails. If
diff --git a/libcxx/docs/BuildingLibcxx.rst b/libcxx/docs/BuildingLibcxx.rst
index e425b9dadfe7d..b4fed5ddec0db 100644
--- a/libcxx/docs/BuildingLibcxx.rst
+++ b/libcxx/docs/BuildingLibcxx.rst
@@ -417,7 +417,7 @@ libc++ Feature Options
 
 .. option:: LIBCXX_ASSERTION_HANDLER_FILE:PATH
 
-  **Default**:: ``"${CMAKE_CURRENT_SOURCE_DIR}/vendor/llvm/default_assertion_handler.in"``
+  **Default**:: ``"${CMAKE_CURRENT_SOURCE_DIR}/include/__assertion_handler"``
 
   Specify the path to a header that contains a custom implementation of the
   assertion handler that gets invoked when a hardening assertion fails. If
@@ -505,11 +505,12 @@ Under the hood, a hardening assertion will invoke the
 that contains a custom definition of this macro and specify the path to the
 header via the ``LIBCXX_ASSERTION_HANDLER_FILE`` CMake variable. If provided,
 this header will be included by the library and replace the default
-implementation. The header must not include any standard library headers
-(directly or transitively) because doing so will almost always create a circular
-dependency. The ``_LIBCPP_ASSERTION_HANDLER(message)`` macro takes a single
-parameter that contains an error message explaining the hardening failure and
-some details about the source location that triggered it.
+implementation. The header may only include `__config`, `__config_site`,
+and `__verbose_abort`, The custom header must not include any other standard
+library headers (directly or transitively) because doing so will almost always
+create a circular dependency. The ``_LIBCPP_ASSERTION_HANDLER(message)`` macro
+takes a single parameter that contains an error message explaining the hardening
+failure and some details about the source location that triggered it.
 
 When a hardening assertion fails, it means that the program is about to invoke
 library undefined behavior. For this reason, the custom assertion handler is
diff --git a/libcxx/vendor/llvm/default_assertion_handler.in b/libcxx/include/__assertion_handler
similarity index 100%
rename from libcxx/vendor/llvm/default_assertion_handler.in
rename to libcxx/include/__assertion_handler
diff --git a/libcxx/include/module.modulemap b/libcxx/include/module.modulemap
index 8bc94d71391ec..887ccc475a0fd 100644
--- a/libcxx/include/module.modulemap
+++ b/libcxx/include/module.modulemap
@@ -568,6 +568,7 @@ module std [system] {
 // must not be directly imported.
 module std_private_assert            [system] {
   header "__assert"
+  header "__assertion_handler"
   export *
 }
 module std_private_availability      [system] {