[libcxx-commits] [PATCH] D101206: [libc++] Remove UB in std::list

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

Quuxplusone added a comment.

LGTM at this point. So I guess the next question (besides how it looks to @ldionne!) is, should you do it for `forward_list` etc. in this same PR? Do you know already the complete list of types that need this kind of patch done for them? If so, I would think it's appropriate to do them all together in this PR.



================
Comment at: libcxx/include/list:274
+#ifndef _LIBCPP_CXX03_LANG
+  union { _Tp __value_; };
+#else
----------------
Nit: Indent 4 spaces to match the indentation everywhere else in this file. (I blame clang-format. ;))


================
Comment at: libcxx/include/list:282-285
+#ifndef _LIBCPP_CXX03_LANG
+    _LIBCPP_INLINE_VISIBILITY
+    __list_node() {}
+#endif
----------------
Any reason not to `=default` this constructor? (And I'd make it `explicit`, since there's no reason not to, and it might ease overload resolution just a tiny bit. In my codebases, the `explicit` keyword basically means "Look out, here comes a constructor!")


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101206