[libc-commits] [libc] [libc] improve error handling and compiler hints (PR #109026)

[Michael Jones via libc-commits]

================
@@ -9,43 +9,87 @@
 #ifndef LLVM_LIBC_SRC___SUPPORT_LIBC_ASSERT_H
 #define LLVM_LIBC_SRC___SUPPORT_LIBC_ASSERT_H
 
-#include "src/__support/macros/config.h"
-#if defined(LIBC_COPT_USE_C_ASSERT) || !defined(LIBC_FULL_BUILD)
-
-// The build is configured to just use the public <assert.h> API
-// for libc's internal assertions.
-
-#include <assert.h>
-
-#define LIBC_ASSERT(COND) assert(COND)
-
-#else // Not LIBC_COPT_USE_C_ASSERT
-
+#include "src/__support/CPP/string_view.h"
 #include "src/__support/OSUtil/exit.h"
 #include "src/__support/OSUtil/io.h"
+#include "src/__support/OSUtil/linux/io.h"
 #include "src/__support/integer_to_string.h"
 #include "src/__support/macros/attributes.h" // For LIBC_INLINE
+#include "src/__support/macros/config.h"
 
 namespace LIBC_NAMESPACE_DECL {
+LIBC_INLINE void report_assertion_failure(cpp::string_view assertion,
+                                          cpp::string_view filename,
+                                          cpp::string_view line,
+                                          cpp::string_view funcname) {
+  write_all_to_stderr({filename, ":", line, ": Assertion failed: '", assertion,
+                       "' in function: '", funcname, "'\n"});
+}
 
-// This is intended to be removed in a future patch to use a similar design to
-// below, but it's necessary for the external assert.
-LIBC_INLINE void report_assertion_failure(const char *assertion,
-                                          const char *filename, unsigned line,
-                                          const char *funcname) {
+LIBC_INLINE void report_assertion_failure(cpp::string_view assertion,
+                                          cpp::string_view filename,
+                                          unsigned line,
+                                          cpp::string_view funcname) {
   const IntegerToString<unsigned> line_buffer(line);
-  write_to_stderr(filename);
-  write_to_stderr(":");
-  write_to_stderr(line_buffer.view());
-  write_to_stderr(": Assertion failed: '");
-  write_to_stderr(assertion);
-  write_to_stderr("' in function: '");
-  write_to_stderr(funcname);
-  write_to_stderr("'\n");
+  report_assertion_failure(assertion, filename, line_buffer.view(), funcname);
+}
+
+// Direct calls to the unsafe_unreachable are highly discouraged.
+// Use the macro versions to provide at least a check in debug builds.
+LIBC_INLINE void unsafe_unreachable() {
+#if __has_builtin(__builtin_unreachable)
+  __builtin_unreachable();
+#endif
+}
----------------
michaelrj-google wrote:

given that `__builtin_unreachable` doesn't lower to any particular instructions, this should be a macro. I'm not certain how a given compiler will handle a function call that's just `__builtin_unreachable` but I wouldn't be surprised for it to simply be ignored.

https://github.com/llvm/llvm-project/pull/109026