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

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

Thanks a lot, this LGTM.



================
Comment at: libcxx/include/vector:1048
 template <class _Tp, class _Allocator>
 vector<_Tp, _Allocator>::vector(size_type __n)
 {
----------------
I think it's weird that we're default initializing the allocator instead of value initializing it here, but it's definitely consistent with what we did before this patch.

Actually, this leads me to believe that we don't implement the C++20 `vector` properly. Indeed, in C++20, this constructor is specified as:

```
constexpr explicit vector(size_type n, const Allocator& = Allocator());
```

Unless mistake, this means that we should be value-initializing the allocator here to be equivalent to the default argument. This seems to have changed in C++14 (https://en.cppreference.com/w/cpp/container/vector/vector). @Quuxplusone, do you happen to know what paper changed this? Does this look suspicious to you too?

This comment doesn't affect this review since it's not a behavior change with this patch, but we should rectify the situation in a follow-up patch if my suspicion above is confirmed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117108