[libcxx-commits] [PATCH] D128146: [libc++] Use uninitialized algorithms for vector

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik marked an inline comment as done.
philnik added inline comments.


================
Comment at: libcxx/include/__memory/uninitialized_algorithms.h:501
 
+template <class _Alloc, class _Iter1, class _Sent1, class _Iter2>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX17 _Iter2
----------------
Mordante wrote:
> Is this moved too? For reviewing I prefer a stack of 2 (or more) commits where the moving of code is separated from the real changes.
No, this isn't just moved. Here the names and behaviour changed. The algorithms now destroy the elements again if an exception has been thrown.


================
Comment at: libcxx/include/__utility/move.h:31
+template <class _Tp>
+using __move_if_noexcept_result_t = _Tp&&;
+#else
----------------
Mordante wrote:
> Is this needed? I assume the original code also resulted in this type when exceptions were disabled.
https://godbolt.org/z/vsPqYa99h I also found it weird, but `noexcept` is still correctly checked with `-fno-exceptions`.


================
Comment at: libcxx/test/libcxx/containers/sequences/vector/asan_throw.pass.cpp:102
   } catch (int e) {
-    assert(v.size() == 3);
+    assert(v.size() == 2);
   }
----------------
Mordante wrote:
> Is this a behaviour change or a bug fix? I don't understand this change. Maybe add a description to the patch that explains this behaviour change. That way when we look at the history we know why this was done.
https://eel.is/c++draft/vector#modifiers-2 is the relevant paragraph I think. I'm not entirely sure if this is just a behaviour change or if it's a bugfix. I //think// it's a bugfix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128146