Add C++23 stacktrace (P0881R7) #136528
Draft
Add C++23 stacktrace (P0881R7) #136528
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Steve O'Brien (elsteveogrande) ChangesFirst pass at Some remaining TODOs, which I marked with
Patch is 203.60 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/136528.diff 78 Files Affected:
diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index ebaa6e9fd0e97..b12bf2ead76e5 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -131,6 +131,11 @@ option(LIBCXX_ENABLE_VENDOR_AVAILABILITY_ANNOTATIONS
the shared library they shipped should turn this on and see `include/__configuration/availability.h`
for more details." OFF)
+option(LIBCPP_STACKTRACE_ALLOW_TOOLS_AT_RUNTIME
+ "For C++23 <stacktrace>: whether to allow invocation of `addr2line`, `llvm-addr2line` or `atos`
+ at runtime (if it's available in PATH) to resolve call-chain addresses in the stacktrace
+ into source locations, if other methods are not available." ON)
+
if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
set(LIBCXX_DEFAULT_TEST_CONFIG "llvm-libc++-shared-gcc.cfg.in")
elseif(MINGW)
@@ -757,6 +762,7 @@ config_define(${LIBCXX_ENABLE_UNICODE} _LIBCPP_HAS_UNICODE)
config_define(${LIBCXX_ENABLE_WIDE_CHARACTERS} _LIBCPP_HAS_WIDE_CHARACTERS)
config_define(${LIBCXX_ENABLE_TIME_ZONE_DATABASE} _LIBCPP_HAS_TIME_ZONE_DATABASE)
config_define(${LIBCXX_ENABLE_VENDOR_AVAILABILITY_ANNOTATIONS} _LIBCPP_HAS_VENDOR_AVAILABILITY_ANNOTATIONS)
+config_define(${LIBCPP_STACKTRACE_ALLOW_TOOLS_AT_RUNTIME} _LIBCPP_STACKTRACE_ALLOW_TOOLS_AT_RUNTIME)
# TODO: Remove in LLVM 21. We're leaving an error to make this fail explicitly.
if (LIBCXX_ENABLE_ASSERTIONS)
diff --git a/libcxx/docs/UserDocumentation.rst b/libcxx/docs/UserDocumentation.rst
index 4a11a10224ae9..649b7551eb746 100644
--- a/libcxx/docs/UserDocumentation.rst
+++ b/libcxx/docs/UserDocumentation.rst
@@ -70,6 +70,7 @@ when ``-fexperimental-library`` is passed:
* The parallel algorithms library (``<execution>`` and the associated algorithms)
* ``std::chrono::tzdb`` and related time zone functionality
+* ``<stacktrace>``
* ``<syncstream>``
.. note::
diff --git a/libcxx/docs/VendorDocumentation.rst b/libcxx/docs/VendorDocumentation.rst
index 959a28607d75d..b05d494db4f9b 100644
--- a/libcxx/docs/VendorDocumentation.rst
+++ b/libcxx/docs/VendorDocumentation.rst
@@ -185,6 +185,14 @@ General purpose options
ship the IANA time zone database. When time zones are not supported,
time zone support in <chrono> will be disabled.
+.. option:: LIBCPP_STACKTRACE_ALLOW_TOOLS_AT_RUNTIME:BOOL
+
+ **Default**: ``OFF``
+
+ For C++23 <stacktrace>: whether to allow invocation of ``addr2line`` or ``llvm-addr2line``
+ at runtime (if it's available in PATH) to resolve call-chain addresses in the stacktrace
+ into source locations, if other methods are not available.
+
.. option:: LIBCXX_INSTALL_LIBRARY_DIR:PATH
**Default**: ``lib${LIBCXX_LIBDIR_SUFFIX}``
diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index f1bdf684a8549..ef091e40b9ec8 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -991,10 +991,17 @@ set(files
experimental/__simd/traits.h
experimental/__simd/utility.h
experimental/__simd/vec_ext.h
+ experimental/__stacktrace/basic_stacktrace.h
+ experimental/__stacktrace/detail/alloc.h
+ experimental/__stacktrace/detail/context.h
+ experimental/__stacktrace/detail/entry.h
+ experimental/__stacktrace/detail/to_string.h
+ experimental/__stacktrace/stacktrace_entry.h
experimental/iterator
experimental/memory
experimental/propagate_const
experimental/simd
+ experimental/stacktrace
experimental/type_traits
experimental/utility
ext/__hash
diff --git a/libcxx/include/__config b/libcxx/include/__config
index e14632f65b877..071961cf73438 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -977,6 +977,33 @@ typedef __char32_t char32_t;
# define _LIBCPP_NOINLINE
# endif
+// Some functions, e.g. std::stacktrace::current, need to avoid being
+// tail-called by (and tail-calling other) functions, for proper enumeration of
+// call-stack frames.
+// clang-format off
+
+// Disables tail-call optimization for "outbound" calls
+// performed in the function annotated with this attribute.
+# if __has_cpp_attribute(_Clang::__disable_tail_calls__)
+# define _LIBCPP_NO_TAIL_CALLS_OUT [[_Clang::__disable_tail_calls__]]
+# elif __has_cpp_attribute(__gnu__::__optimize__)
+# define _LIBCPP_NO_TAIL_CALLS_OUT [[__gnu__::__optimize__("no-optimize-sibling-calls")]]
+# else
+# define _LIBCPP_NO_TAIL_CALLS_OUT
+# endif
+
+// Disables tail-call optimization for "inbound" calls -- that is,
+// calls from some other function calling the one having this attribute.
+# if __has_cpp_attribute(_Clang::__not_tail_called__)
+# define _LIBCPP_NO_TAIL_CALLS_IN [[_Clang::__not_tail_called__]]
+# else
+# define _LIBCPP_NO_TAIL_CALLS_IN
+# endif
+
+// Disable TCO for calls into, and out from, the annotated function.
+# define _LIBCPP_NO_TAIL_CALLS _LIBCPP_NO_TAIL_CALLS_IN _LIBCPP_NO_TAIL_CALLS_OUT
+// clang-format on
+
// We often repeat things just for handling wide characters in the library.
// When wide characters are disabled, it can be useful to have a quick way of
// disabling it without having to resort to #if-#endif, which has a larger
diff --git a/libcxx/include/__config_site.in b/libcxx/include/__config_site.in
index fc01aaf2d8746..4e25ed040c9d0 100644
--- a/libcxx/include/__config_site.in
+++ b/libcxx/include/__config_site.in
@@ -33,6 +33,7 @@
#cmakedefine _LIBCPP_HAS_NO_STD_MODULES
#cmakedefine01 _LIBCPP_HAS_TIME_ZONE_DATABASE
#cmakedefine01 _LIBCPP_INSTRUMENTED_WITH_ASAN
+#cmakedefine01 _LIBCPP_STACKTRACE_ALLOW_TOOLS_AT_RUNTIME
// PSTL backends
#cmakedefine _LIBCPP_PSTL_BACKEND_SERIAL
diff --git a/libcxx/include/__ostream/basic_ostream.h b/libcxx/include/__ostream/basic_ostream.h
index f7473a36d8ccc..891dddbf79c42 100644
--- a/libcxx/include/__ostream/basic_ostream.h
+++ b/libcxx/include/__ostream/basic_ostream.h
@@ -570,7 +570,7 @@ _LIBCPP_HIDE_FROM_ABI _Stream&& operator<<(_Stream&& __os, const _Tp& __x) {
}
template <class _CharT, class _Traits, class _Allocator>
-basic_ostream<_CharT, _Traits>&
+_LIBCPP_HIDE_FROM_ABI basic_ostream<_CharT, _Traits>&
operator<<(basic_ostream<_CharT, _Traits>& __os, const basic_string<_CharT, _Traits, _Allocator>& __str) {
return std::__put_character_sequence(__os, __str.data(), __str.size());
}
diff --git a/libcxx/include/experimental/__stacktrace/basic_stacktrace.h b/libcxx/include/experimental/__stacktrace/basic_stacktrace.h
new file mode 100644
index 0000000000000..8ac1de82b1c63
--- /dev/null
+++ b/libcxx/include/experimental/__stacktrace/basic_stacktrace.h
@@ -0,0 +1,306 @@
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _LIBCPP_EXPERIMENTAL_BASIC_STACKTRACE
+#define _LIBCPP_EXPERIMENTAL_BASIC_STACKTRACE
+
+#include <experimental/__stacktrace/detail/entry.h>
+#include <experimental/__stacktrace/stacktrace_entry.h>
+
+#include <__config>
+#include <__format/formatter.h>
+#include <__functional/function.h>
+#include <__functional/hash.h>
+#include <__fwd/format.h>
+#include <__fwd/sstream.h>
+#include <__iterator/iterator.h>
+#include <__iterator/iterator_traits.h>
+#include <__iterator/reverse_access.h>
+#include <__iterator/reverse_iterator.h>
+#include <__memory/allocator.h>
+#include <__memory/allocator_traits.h>
+#include <__memory_resource/memory_resource.h>
+#include <__memory_resource/polymorphic_allocator.h>
+#include <__ostream/basic_ostream.h>
+#include <__utility/move.h>
+#include <__vector/pmr.h>
+#include <__vector/swap.h>
+#include <__vector/vector.h>
+#include <cstddef>
+#include <list>
+#include <memory>
+#include <string>
+
+#include <experimental/__stacktrace/detail/alloc.h>
+#include <experimental/__stacktrace/detail/context.h>
+#include <experimental/__stacktrace/detail/to_string.h>
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+// (19.6.4)
+// Class template basic_stacktrace [stacktrace.basic]
+
+class stacktrace_entry;
+
+template <class _Allocator>
+class _LIBCPP_EXPORTED_FROM_ABI basic_stacktrace {
+ friend struct hash<basic_stacktrace<_Allocator>>;
+ friend struct __stacktrace::__to_string;
+
+ using _ATraits _LIBCPP_NODEBUG = allocator_traits<_Allocator>;
+ constexpr static bool __kPropOnCopy = _ATraits::propagate_on_container_copy_assignment::value;
+ constexpr static bool __kPropOnMove = _ATraits::propagate_on_container_move_assignment::value;
+ constexpr static bool __kPropOnSwap = _ATraits::propagate_on_container_swap::value;
+ constexpr static bool __kAlwaysEqual = _ATraits::is_always_equal::value;
+ constexpr static bool __kNoThrowDflConstruct = is_nothrow_default_constructible_v<_Allocator>;
+ constexpr static bool __kNoThrowAlloc =
+ noexcept(noexcept(_Allocator().allocate(1)) && noexcept(_Allocator().allocate_at_least(1)));
+
+ using __entry_vec _LIBCPP_NODEBUG = vector<stacktrace_entry, _Allocator>;
+
+ [[no_unique_address]] _Allocator __alloc_;
+ __entry_vec __entries_;
+
+ _LIBCPP_HIDE_FROM_ABI basic_stacktrace(const _Allocator& __alloc, std::pmr::list<__stacktrace::entry>&& __vec)
+ : __alloc_(__alloc), __entries_(__alloc) {
+ __entries_.reserve(__vec.size());
+ for (auto& __entry : __vec) {
+ __entries_.emplace_back(std::move(__entry));
+ }
+ }
+
+public:
+ // (19.6.4.1)
+ // Overview [stacktrace.basic.overview]
+
+ using value_type = stacktrace_entry;
+ using const_reference = const value_type&;
+ using reference = value_type&;
+ using difference_type = ptrdiff_t;
+ using size_type = size_t;
+ using allocator_type = _Allocator;
+ using const_iterator = decltype(__entries_)::const_iterator;
+ using iterator = const_iterator;
+
+ using reverse_iterator = std::reverse_iterator<basic_stacktrace::iterator>;
+ using const_reverse_iterator = std::reverse_iterator<basic_stacktrace::const_iterator>;
+
+ // (19.6.4.2)
+ // Creation and assignment [stacktrace.basic.cons]
+
+ _LIBCPP_NO_TAIL_CALLS _LIBCPP_NOINLINE _LIBCPP_EXPORTED_FROM_ABI static basic_stacktrace
+ current(const allocator_type& __caller_alloc = allocator_type()) noexcept(__kNoThrowAlloc) {
+ __stacktrace::alloc __alloc(__caller_alloc);
+ __stacktrace::context __tr{__alloc};
+ __tr.do_stacktrace(1, /* infinite max_depth */ ~0);
+ return {__caller_alloc, std::move(__tr.__entries_)};
+ }
+
+ _LIBCPP_NO_TAIL_CALLS _LIBCPP_NOINLINE _LIBCPP_EXPORTED_FROM_ABI static basic_stacktrace
+ current(size_type __skip, const allocator_type& __caller_alloc = allocator_type()) noexcept(__kNoThrowAlloc) {
+ __stacktrace::alloc __alloc(__caller_alloc);
+ __stacktrace::context __tr{__alloc};
+ __tr.do_stacktrace(__skip + 1, /* infinite max_depth */ ~0);
+ return {__caller_alloc, std::move(__tr.__entries_)};
+ }
+
+ _LIBCPP_NO_TAIL_CALLS _LIBCPP_NOINLINE _LIBCPP_EXPORTED_FROM_ABI static basic_stacktrace
+ current(size_type __skip,
+ size_type __max_depth,
+ const allocator_type& __caller_alloc = allocator_type()) noexcept(__kNoThrowAlloc) {
+ __stacktrace::alloc __alloc(__caller_alloc);
+ __stacktrace::context __tr{__alloc};
+ __tr.do_stacktrace(__skip + 1, __max_depth);
+ return {__caller_alloc, std::move(__tr.__entries_)};
+ }
+
+ _LIBCPP_EXPORTED_FROM_ABI constexpr ~basic_stacktrace() = default;
+
+ _LIBCPP_EXPORTED_FROM_ABI basic_stacktrace() noexcept(__kNoThrowDflConstruct) : basic_stacktrace(allocator_type()) {}
+
+ _LIBCPP_EXPORTED_FROM_ABI explicit basic_stacktrace(const allocator_type& __alloc) noexcept
+ : __alloc_(__alloc), __entries_(__alloc_) {}
+
+ _LIBCPP_EXPORTED_FROM_ABI basic_stacktrace(basic_stacktrace const& __other) = default;
+
+ _LIBCPP_EXPORTED_FROM_ABI basic_stacktrace(basic_stacktrace&& __other) noexcept = default;
+
+ _LIBCPP_EXPORTED_FROM_ABI basic_stacktrace(basic_stacktrace const& __other, allocator_type const& __alloc)
+ : __alloc_(__alloc), __entries_(__other.__entries_, __alloc) {}
+
+ _LIBCPP_EXPORTED_FROM_ABI basic_stacktrace(basic_stacktrace&& __other, allocator_type const& __alloc)
+ : __alloc_(__alloc) {
+ if (__kAlwaysEqual || __alloc_ == __other.__alloc_) {
+ __entries_ = std::move(__other.__entries_);
+ } else {
+ // "moving" from a container with a different allocator; we're forced to copy items instead
+ for (auto const& __entry : __other.__entries_) {
+ __entries_.push_back(__entry);
+ }
+ }
+ }
+
+ _LIBCPP_EXPORTED_FROM_ABI basic_stacktrace& operator=(const basic_stacktrace& __other) {
+ if (this == std::addressof(__other)) {
+ return *this;
+ }
+ if (__kPropOnCopy) {
+ __alloc_ = __other.__alloc_;
+ }
+ __entries_ = {__other.__entries_, __alloc_};
+ return *this;
+ }
+
+ _LIBCPP_EXPORTED_FROM_ABI basic_stacktrace&
+ operator=(basic_stacktrace&& __other) noexcept(__kPropOnMove || __kAlwaysEqual) {
+ if (this == std::addressof(__other)) {
+ return *this;
+ }
+ if (__kPropOnMove) {
+ __alloc_ = __other.__alloc_;
+ __entries_ = std::move(__other.__entries_);
+ } else {
+ auto __allocs_eq = __kAlwaysEqual || __alloc_ == __other.__alloc_;
+ if (__allocs_eq) {
+ __entries_ = std::move(__other.__entries_);
+ } else {
+ // "moving" from a container with a different allocator;
+ // we're forced to copy items instead
+ for (auto const& __entry : __other.__entries_) {
+ __entries_.push_back(__entry);
+ }
+ }
+ }
+ return *this;
+ }
+
+ // clang-format on
+
+ // (19.6.4.3)
+ // [stacktrace.basic.obs], observers
+
+ [[nodiscard]] _LIBCPP_EXPORTED_FROM_ABI allocator_type get_allocator() const noexcept { return __alloc_; }
+
+ [[nodiscard]] _LIBCPP_EXPORTED_FROM_ABI const_iterator begin() const noexcept { return __entries_.begin(); }
+ [[nodiscard]] _LIBCPP_EXPORTED_FROM_ABI const_iterator end() const noexcept { return __entries_.end(); }
+ [[nodiscard]] _LIBCPP_EXPORTED_FROM_ABI const_reverse_iterator rbegin() const noexcept { return __entries_.rbegin(); }
+ [[nodiscard]] _LIBCPP_EXPORTED_FROM_ABI const_reverse_iterator rend() const noexcept { return __entries_.rend(); }
+
+ [[nodiscard]] _LIBCPP_EXPORTED_FROM_ABI const_iterator cbegin() const noexcept { return __entries_.cbegin(); }
+ [[nodiscard]] _LIBCPP_EXPORTED_FROM_ABI const_iterator cend() const noexcept { return __entries_.cend(); }
+ [[nodiscard]] _LIBCPP_EXPORTED_FROM_ABI const_reverse_iterator crbegin() const noexcept {
+ return __entries_.crbegin();
+ }
+ [[nodiscard]] _LIBCPP_EXPORTED_FROM_ABI const_reverse_iterator crend() const noexcept { return __entries_.crend(); }
+
+ [[nodiscard]] _LIBCPP_EXPORTED_FROM_ABI bool empty() const noexcept { return __entries_.empty(); }
+ [[nodiscard]] _LIBCPP_EXPORTED_FROM_ABI size_type size() const noexcept { return __entries_.size(); }
+ [[nodiscard]] _LIBCPP_EXPORTED_FROM_ABI size_type max_size() const noexcept { return __entries_.max_size(); }
+
+ [[nodiscard]] _LIBCPP_EXPORTED_FROM_ABI const_reference operator[](size_type __i) const { return __entries_[__i]; }
+ [[nodiscard]] _LIBCPP_EXPORTED_FROM_ABI const_reference at(size_type __i) const { return __entries_.at(__i); }
+
+ // (19.6.4.4)
+ // [stacktrace.basic.cmp], comparisons
+
+ template <class _Allocator2>
+ _LIBCPP_EXPORTED_FROM_ABI friend bool
+ operator==(const basic_stacktrace& __x, const basic_stacktrace<_Allocator2>& __y) noexcept {
+ if (__x.size() != __y.size()) {
+ return false;
+ }
+ auto __xi = __x.begin();
+ auto __yi = __y.begin();
+ auto __xe = __x.end();
+ while (__xi != __xe) {
+ if (*__xi++ != *__yi++) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ template <class _Allocator2>
+ _LIBCPP_EXPORTED_FROM_ABI friend strong_ordering
+ operator<=>(const basic_stacktrace& __x, const basic_stacktrace<_Allocator2>& __y) noexcept {
+ auto __ret = __x.size() <=> __y.size();
+ if (__ret != std::strong_ordering::equal) {
+ return __ret;
+ }
+ auto __xi = __x.begin();
+ auto __yi = __y.begin();
+ auto __xe = __x.end();
+ while ((__ret == std::strong_ordering::equal) && __xi != __xe) {
+ __ret = *__xi++ <=> *__yi++;
+ }
+ return __ret;
+ }
+
+ // (19.6.4.5)
+ // [stacktrace.basic.mod], modifiers
+
+ _LIBCPP_EXPORTED_FROM_ABI void swap(basic_stacktrace<_Allocator>& __other) noexcept {
+ std::swap(__entries_, __other.__entries_);
+ if (__kPropOnSwap) {
+ std::swap(__alloc_, __other.__alloc_);
+ }
+ }
+};
+
+using stacktrace = basic_stacktrace<allocator<stacktrace_entry>>;
+
+namespace pmr {
+using stacktrace = basic_stacktrace<polymorphic_allocator<stacktrace_entry>>;
+} // namespace pmr
+
+// (19.6.4.6)
+// Non-member functions [stacktrace.basic.nonmem]
+
+template <class _Allocator>
+_LIBCPP_EXPORTED_FROM_ABI inline void
+swap(basic_stacktrace<_Allocator>& __a, basic_stacktrace<_Allocator>& __b) noexcept(noexcept(__a.swap(__b))) {
+ __a.swap(__b);
+}
+
+template <class _Allocator>
+_LIBCPP_EXPORTED_FROM_ABI inline string to_string(const basic_stacktrace<_Allocator>& __stacktrace) {
+ return __stacktrace::__to_string()(__stacktrace);
+}
+
+template <class _Allocator>
+_LIBCPP_EXPORTED_FROM_ABI inline ostream& operator<<(ostream& __os, const basic_stacktrace<_Allocator>& __stacktrace) {
+ auto __str = __stacktrace::__to_string()(__stacktrace);
+ return __os << __str;
+}
+
+// (19.6.5)
+// Formatting support [stacktrace.format]
+
+// TODO(stacktrace23): needs `formatter`
+template <class _Allocator>
+struct _LIBCPP_EXPORTED_FROM_ABI formatter<basic_stacktrace<_Allocator>>;
+
+// (19.6.6)
+// Hash support [stacktrace.basic.hash]
+
+template <class _Allocator>
+struct _LIBCPP_EXPORTED_FROM_ABI hash<basic_stacktrace<_Allocator>> {
+ [[nodiscard]] size_t operator()(basic_stacktrace<_Allocator> const& __context) const noexcept {
+ size_t __ret = 1;
+ for (auto const& __entry : __context.__entries_) {
+ __ret += hash<uintptr_t>()(__entry.native_handle());
+ __ret *= 3001;
+ }
+ return __ret;
+ }
+};
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP_EXPERIMENTAL_BASIC_STACKTRACE
diff --git a/libcxx/include/experimental/__stacktrace/detail/alloc.h b/libcxx/include/experimental/__stacktrace/detail/alloc.h
new file mode 100644
index 0000000000000..a0949b5751899
--- /dev/null
+++ b/libcxx/include/experimental/__stacktrace/detail/alloc.h
@@ -0,0 +1,103 @@
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _LIBCPP_EXPERIMENTAL_STACKTRACE_ALLOC
+#define _LIBCPP_EXPERIMENTAL_STACKTRACE_ALLOC
+
+#include <__config>
+#include <__functional/function.h>
+#include <__memory/allocator_traits.h>
+#include <__memory_resource/memory_resource.h>
+#include <__memory_resource/polymorphic_allocator.h>
+#include <cstddef>
+#include <list>
+#include <memory>
+#include <string>
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+class stacktrace_entry;
+
+namespace __stacktrace {
+
+/** Per-stacktrace-invocation allocator which wraps a caller-provided allocator of any type.
+This is intended to be used with `std::pmr::` containers and strings throughout the stacktrace
+creation process. */
+struct alloc final : std::pmr::memory_resource {
+ template <class _Allocator>
+ _LIBCPP_HIDE_FROM_ABI explicit alloc(_Allocator const& __a) {
+ // Take the given allocator type, and rebind with a new type having <byte> as the template arg
+ using _AT = std::allocator_traits<_Allocator>;
+ using _BA = typename _AT::template rebind_alloc<std::byte>;
+ auto __ba = _BA(__a);
+ __alloc_func_ = [__ba](size_t __sz) mutable { return __ba.allocate(__sz); };
+ __dealloc_func_ = [__ba](void* __ptr, size_t __sz) mutable { return __ba.deallocate((std::byte*)__ptr, __sz); };
+ __alloc_opaque_ = std::addressof(__a);
+ }
+
+ _LIBCPP_HIDE_FROM_ABI_VIRTUAL ~alloc() override = default;
+
+ _LIBCPP_HIDE_FROM_ABI_VIRTUAL vi...
[truncated]
|
|
✅ With the latest revision this PR passed the Python code formatter. |
elsteveogrande
changed the title
frederick-vs-ja
requested changes
Apr 21, 2025
|
This PR should have
IMO this can be done in a later PR... |
|
@elsteveogrande Thanks for working on this. It's great to see progress on Just a few nits: as @frederick-vs-ja pointed out we have a few unwritten conventions for PRs, which while not required strictly is nice to follow for consistency, such as naming PR, and putting relevant info in the commit message, using the github syntax to link PR to issues. |
Zingam
reviewed
Apr 21, 2025
Zingam
reviewed
Apr 21, 2025
Zingam
reviewed
Apr 21, 2025
| //===----------------------------------------------------------------------===// | ||
|
|
||
| #ifndef _LIBCPP_STACKTRACE_OSX_H | ||
| #define _LIBCPP_STACKTRACE_OSX_H |
There was a problem hiding this comment.
I'm pretty sure that for the macOS you should implement the availability macros: https://github.com/llvm/llvm-project/blob/main/libcxx/include/__configuration/availability.h
There was a problem hiding this comment.
Thanks again for the feedback @Zingam -- I will revisit this (I don't understand this fully yet :) )
Zingam
reviewed
Apr 21, 2025
Zingam
reviewed
Apr 21, 2025
Comment on lines
94
to
95
| std::function<std::byte*(size_t)> __alloc_func_; | ||
| std::function<void(std::byte*, size_t)> __dealloc_func_; |
There was a problem hiding this comment.
Isn't this too heavy and/or overly type-erasing?
Also, it seems to me that internal use of pmr containers will cause the polymorphic_allocator to be stored too many times.
There was a problem hiding this comment.
I was doubting whether I should have been using pmr here. While convenient for me, it might not have been appropriate to apply here. (I don't see any pmr usage inside libcxx anywhere else...)
There is need to do allocations within stacktrace though (other than for stacktrace_entry for which the caller may have provided their allocator). So I think I'll still need alloc/dealloc functions, which delegate to the caller's allocator. But if there are other better ways I'm happy to consider those :)
There was a problem hiding this comment.
I found that other implementations (MSVC STL & libstdc++) just pass pointers to allocating and deallcating functions and contexts to the separately compiled library, and the allocation strategy is controlled in the header. IMO pmr should be avoided (except for the standard-required typedef name std::pmr::stacktrace).
Zingam
reviewed
Apr 21, 2025
Zingam
reviewed
Apr 21, 2025
Comment on lines
11
to
25
| #include <stacktrace> | ||
|
|
||
| #include <cassert> | ||
|
|
||
| /* | ||
| (19.6.4.4) Comparisons [stacktrace.basic.cmp] | ||
|
|
||
| template<class Allocator2> | ||
| friend bool operator==(const basic_stacktrace& x, | ||
| const basic_stacktrace<Allocator2>& y) noexcept; | ||
|
|
||
| template<class Allocator2> | ||
| friend strong_ordering operator<=>(const basic_stacktrace& x, | ||
| const basic_stacktrace<Allocator2>& y) noexcept; | ||
| */ |
There was a problem hiding this comment.
I think the current practice is to test a single function, method in a test, e.g.
stacktrace/basic.cmp/equality.pass.pass for operator==
stacktrace/basic.cmp/three_way_comparison.pass.cpp for spaceship
you need to put the tests in subdirectories named after the C++ standard sections
Then add the synopsis and then the includes sorted.
Suggested change
| #include <stacktrace> | |
| #include <cassert> | |
| /* | |
| (19.6.4.4) Comparisons [stacktrace.basic.cmp] | |
| template<class Allocator2> | |
| friend bool operator==(const basic_stacktrace& x, | |
| const basic_stacktrace<Allocator2>& y) noexcept; | |
| template<class Allocator2> | |
| friend strong_ordering operator<=>(const basic_stacktrace& x, | |
| const basic_stacktrace<Allocator2>& y) noexcept; | |
| */ | |
| // template<class Allocator> | |
| // class basic_stacktrace | |
| // template<class Allocator2> | |
| // friend bool operator==(const basic_stacktrace& x, | |
| // const basic_stacktrace<Allocator2>& y) noexcept; | |
| #include <cassert> | |
| #include <stacktrace> |
Did you have a test for noexcept?
There was a problem hiding this comment.
I'll break these up, reformat, etc.
There was a problem hiding this comment.
Oh, and also ensure I have something testing noexcept :)
There was a problem hiding this comment.
I broke up the tests but still need one for noexcept
elsteveogrande
force-pushed
the
stacktrace23
branch
from
a5e2ba7 to
c199a84
Compare
|
@elsteveogrande BTW It's better to use "merge" after the review started because rebasing affects the comments on GH. |
elsteveogrande
force-pushed
the
stacktrace23
branch
2 times, most recently
from
aeb5673 to
12636b4
Compare
Argh, yeah I see my re-stacking and force-pushing to my branch messed things up a little; will avoid rebasing |
elsteveogrande
commented
Apr 23, 2025
Comment on lines
11
to
25
| #include <stacktrace> | ||
|
|
||
| #include <cassert> | ||
|
|
||
| /* | ||
| (19.6.4.4) Comparisons [stacktrace.basic.cmp] | ||
|
|
||
| template<class Allocator2> | ||
| friend bool operator==(const basic_stacktrace& x, | ||
| const basic_stacktrace<Allocator2>& y) noexcept; | ||
|
|
||
| template<class Allocator2> | ||
| friend strong_ordering operator<=>(const basic_stacktrace& x, | ||
| const basic_stacktrace<Allocator2>& y) noexcept; | ||
| */ |
There was a problem hiding this comment.
I'll break these up, reformat, etc.
Comment on lines
11
to
25
| #include <stacktrace> | ||
|
|
||
| #include <cassert> | ||
|
|
||
| /* | ||
| (19.6.4.4) Comparisons [stacktrace.basic.cmp] | ||
|
|
||
| template<class Allocator2> | ||
| friend bool operator==(const basic_stacktrace& x, | ||
| const basic_stacktrace<Allocator2>& y) noexcept; | ||
|
|
||
| template<class Allocator2> | ||
| friend strong_ordering operator<=>(const basic_stacktrace& x, | ||
| const basic_stacktrace<Allocator2>& y) noexcept; | ||
| */ |
There was a problem hiding this comment.
Oh, and also ensure I have something testing noexcept :)
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Zingam
reviewed
Apr 24, 2025
| @@ -189,7 +189,7 @@ | |||
| "`LWG3028 <https://wg21.link/LWG3028>`__","Container requirements tables should distinguish ``const`` and non-``const`` variables","2022-11 (Kona)","","","" | |||
| "`LWG3118 <https://wg21.link/LWG3118>`__","``fpos`` equality comparison unspecified","2022-11 (Kona)","","","" | |||
| "`LWG3177 <https://wg21.link/LWG3177>`__","Limit permission to specialize variable templates to program-defined types","2022-11 (Kona)","|Nothing To Do|","","" | |||
| "`LWG3515 <https://wg21.link/LWG3515>`__","§[stacktrace.basic.nonmem]: ``operator<<`` should be less templatized","2022-11 (Kona)","","","" | |||
| "`LWG3515 <https://wg21.link/LWG3515>`__","§[stacktrace.basic.nonmem]: ``operator<<`` should be less templatized","2022-11 (Kona)","|Nothing To Do|","","" | |||
There was a problem hiding this comment.
I only had a quick look but I think the status should be |Complete| and you also need to add Closes #104998 .
We don't add an entry to Release Notes for LWG issues.
R-Goc
reviewed
Apr 24, 2025
libcxx/src/stacktrace/config.h
Outdated
| # define _LIBCPP_STACKTRACE_CAN_SPAWN_TOOLS | ||
| #endif | ||
|
|
||
| #if __has_include(<windows.h>) && __has_include(<dbghelp.h>) && __has_include(<psapi.h>) |
There was a problem hiding this comment.
Why not actually check for windows? Either with the usual macros, or ideally a libcxx specific one if it exists. It seems like _LIBCPP_WIN32API is the one used most often.
There was a problem hiding this comment.
Same goes for the other platforms
elsteveogrande
force-pushed
the
stacktrace23
branch
from
fbc27fa to
d4d8e56
Compare
Arthapz
reviewed
May 16, 2025
elsteveogrande
force-pushed
the
stacktrace23
branch
from
3c3585d to
b1f6f70
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
First pass at
<stacktrace>implementation.Some remaining TODOs, which I marked with
TODO(stacktrace23):select_on_container_copy_construction/select_on_container_swapStill needs support for(will cover in another PR for the relevant issue)formatterCloses #105131
Closes #105167