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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

Thanks for the update, only a few small issues remain.



================
Comment at: libcxx/include/__config:118
+// According to the standard `bitset::operator[] const` returns bool
+#  define _LIBCPP_ABI_BITSET_SUBSCRIPT_RETURN_BOOL
 #elif _LIBCPP_ABI_VERSION == 1
----------------
Let's make clear it's about the const overload.


================
Comment at: libcxx/test/std/utilities/template.bitset/bitset.members/index_const.pass.cpp:31
+#if defined(_LIBCPP_VERSION)
+#if _LIBCPP_ABI_VERSION >= 2
+    ASSERT_SAME_TYPE(decltype(cases[0][0]), bool);
----------------
Typically we just use the ABI name in our tests, for example in `test/std/re/re.const/re.synopt/syntax_option_type.pass.cpp`

```
#ifdef _LIBCPP_ABI_REGEX_CONSTANTS_NONZERO  // https://llvm.org/PR35967
    assert(std::regex_constants::ECMAScript != 0);
#else
    assert(std::regex_constants::ECMAScript == 0);
#endif
```
(In the WIP script to test against internal macro's I've whitelisted `_LIBCPP_ABI_*`.)

I actually think the test should be slightly different
```
#if !defined(_LIBCPP_VERSION) || defined(_LIBCPP_ABI_REGEX_CONSTANTS_NONZERO)  // https://llvm.org/PR35967
    assert(std::regex_constants::ECMAScript != 0);
#else
    assert(std::regex_constants::ECMAScript == 0);
#endif
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122092