[libcxx-commits] [PATCH] D153672: [libc++] Remove the legacy debug mode.

Mon Jul 3 03:01:50 PDT 2023

hans added inline comments.

================
Comment at: libcxx/include/__tree:379-382
+    // TODO(varconst): make this work with the new debug mode (or remove).
+#ifdef _LIBCPP_ENABLE_DEBUG_MODE
+    _LIBCPP_ASSERT(std::__tree_invariant(__root), "The tree invariants should hold");
+#endif
----------------
ldionne wrote:
> I think this should just be a `_LIBCPP_UNCATEGORIZED_ASSERT` for now. It would work today.
> 
> Once we categorize this, this could become something like `_LIBCPP_INTERNAL_ASSERTION` or something like that since it's effectively an internal consistency check to ensure that our implementation isn't broken. We have other instances of that, and we had thought about adding another assertion macro to handle those back then.
This caused a few of Chromium's tests to fail.

Since we set `_LIBCPP_ENABLE_ASSERTIONS` but not `_LIBCPP_ENABLE_DEBUG_MODE`, this made `std::tree<>::erase` `O(n)` instead of `O(log n)`, causing a few of our tests to time out. (crbug.com/1459922)

It seems this kind of check is too expensive for the regular assert mode. Could this check be disabled for now, or is there some alternative "expensive checks" mode it could be part of?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153672/new/

https://reviews.llvm.org/D153672