[libcxx-commits] [PATCH] D110347: [libc++] Do not enable P1951 before C++23, since it's a breaking change

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

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

I'm happy if buildkite is happy. My comment about lines 116+117 is worth addressing one way or the other (either complicate the ctors, or remove the redundant assertions), but I don't really care which way.



================
Comment at: libcxx/test/std/utilities/utility/pairs/pairs.pair/U_V.pass.cpp:106-109
             constexpr TrackInit(TrackInit const&) : wasCopyInit(true) { }
             constexpr TrackInit(TrackInit&&) : wasMoveInit(true) { }
             bool wasMoveInit = false;
             bool wasCopyInit = false;
----------------
FWIW, the paired assertions (e.g. lines 116+117) seem like overkill, since right now obviously it's impossible ever to have `wasCopyInit && wasMoveInit`. I wonder if you meant to do, like,
```
constexpr TrackInit(TrackInit const& rhs) : wasCopyInit(true), wasMoveInit(rhs.wasMoveInit) { }
constexpr TrackInit(TrackInit&& rhs) : wasCopyInit(rhs.wasCopyInit), wasMoveInit(true) { }
```


================
Comment at: libcxx/test/std/utilities/utility/pairs/pairs.pair/ctor.brace-init.P1951.pass.cpp:34-35
+        int i = 0;
+        auto f = [&](std::pair<std::vector<int>, const A&>) { assert(i >= 1); };
+        f({{42, 43}, &i});
+    }
----------------
LGTM, although of course you could replace `vector<int>` with `int` and/or `{42,43}` with `{}` if you wanted to simplify it a bit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110347