[libcxx-commits] [PATCH] D116947: [libc++] Introduce __debug_db_insert_c()

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/__debug:15
 #include <iosfwd>
+#include <type_traits>
 
----------------
philnik wrote:
> Quuxplusone wrote:
> > Is this for `__libcpp_is_constant_evaluated`? Might it make sense to move `__libcpp_is_constant_evaluated` into `<__config>`?
> Yes. But I don't think it should be in `<__config>`. It's not really a config. Maybe in something like `__type_traits/is_contant_evaulated.h`?
I would definitely not move it to `__config`. If anything, I would put it in `__type_traits/is_constant_evaluated.h` as suggested.


================
Comment at: libcxx/include/__debug:273
+template <class _Tp>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX11 inline void __debug_db_insert_c(_Tp* __c) {
+#if _LIBCPP_DEBUG_LEVEL == 2
----------------
philnik wrote:
> Quuxplusone wrote:
> > and perhaps `_LIBCPP_ALWAYS_INLINE` as we discussed? (I'm still neutral/ambivalent — seems like an easy but also maybe negligible win.)
> I'd like to hear what @ldionne thinks about this. I'm not really sold on the idea unless we have some general guidelines when to use `_LIBCPP_ALWAYS_INLINE` or something similar.
I would avoid sprinkling `_LIBCPP_ALWAYS_INLINE` unless we have very good reasons to. It's just a bad habit to take. I am confident that the optimizer will see this and inline it when debugging is disabled. When debugging is enabled, it doesn't really matter as much.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116947