[libcxx-commits] [PATCH] D122092: [libc++] `bitset::operator[] const` should return bool

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/test/std/containers/sequences/vector.bool/const_reference.pass.cpp:16
+
+bool test() {
+  using CRefT = std::vector<bool>::const_reference;
----------------
var-const wrote:
> Optional nit: since this test isn't `constexpr`, it doesn't need to return anything -- unless you expect it to become `constexpr` at some future point?
This test should become `constexpr` once we implement P1004R2.


================
Comment at: libcxx/test/std/utilities/template.bitset/bitset.members/index_const.pass.cpp:46-55
+  std::bitset<1> set_;
+  set_[0] = false;
+  const auto& set = set_;
+  auto b = set[0];
+  set_[0] = true;
+#if !defined(_LIBCPP_VERSION) || defined(_LIBCPP_ABI_BITSET_CONST_SUBSCRIPT_RETURN_BOOL)
+  assert(!b);
----------------
var-const wrote:
> ldionne wrote:
> > Can you add the same test to the non-const `operator[]` (but you'll only have one `assert`)?
> > 
> > Also, I'd recommend against using `#endif // comment` since this block is so short -- in this case it's even wrong.
> This comment doesn't seem to be addressed.
Thanks! I completely forgot to remove the comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122092