[libcxx-commits] [PATCH] D117780: [libc++][test] add vector<bool>::reference tests

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

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

LGTM % a couple last nits!



================
Comment at: libcxx/test/std/containers/sequences/vector.bool/reference/assign_bool.pass.cpp:24
+  assert(ref);
+  ref = false;
+  assert(!ref);
----------------
Here as well, please test with both `true` and `false`.


================
Comment at: libcxx/test/std/containers/sequences/vector.bool/reference/ctor_copy.pass.cpp:22
+  Ref ref2 = ref;
+  assert(ref == ref2);
+
----------------
This test feels insufficient — it's just testing that `ref2` refers to //some// `true` boolean, not that it refers to the //same// `vec[0]` "object" — but I have no great ideas for what would be better here. (We want to test `&ref == &ref2` but of course that doesn't work because `vector<bool>`.)


================
Comment at: libcxx/test/std/containers/sequences/vector.bool/reference/flip.pass.cpp:23
+  ref.flip();
+  assert(!ref);
+  ref.flip();
----------------
Quuxplusone wrote:
> Let's test `!vec[0]` directly, and also test that e.g. `vec[1]` remains unmodified.
IMO you could //remove// `assert(!ref)` and `assert(ref)` at this point (lines 22-24, 26, 30), but this is fine too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117780