[libcxx-commits] [PATCH] D117108: [libc++] Remove vector base class

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone accepted this revision as: Quuxplusone.
Quuxplusone added a comment.

LGTM but possibly significant comment.



================
Comment at: libcxx/include/vector:649-652
+    pointer __begin_ = nullptr;
+    pointer __end_ = nullptr;
+    __compressed_pair<pointer, allocator_type> __end_cap_ =
+        __compressed_pair<pointer, allocator_type>(nullptr, allocator_type());
----------------
I'd feel slightly better if this unrelated NSDMI-ification were done in a separate commit; but if CI is happy then I'm happy.


================
Comment at: libcxx/src/vector.cpp:18-26
+template <>
+struct __vector_base_common<true> {
+  _LIBCPP_NORETURN _LIBCPP_EXPORTED_FROM_ABI void __throw_length_error() const {
+    _VSTD::__throw_length_error("vector");
+  }
+  _LIBCPP_NORETURN _LIBCPP_EXPORTED_FROM_ABI void __throw_out_of_range() const {
     _VSTD::__throw_out_of_range("vector");
----------------
These member functions are now implicitly inline, because they appear in the body of a class. FWLIW I'd feel vastly safer if they appeared outside the body of the class, so that they would not be `inline`.
I'm guessing that `_LIBCPP_EXPORTED_FROM_ABI` is some kind of magic "anti-inline" keyword that will cause them to be generated anyway, on relevant platforms? @ldionne you think this is OK (and better than defining them out-of-line)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117108