[libcxx-commits] [PATCH] D80895: [libcxx] adds consistent comparison for `basic_string`

Christopher Di Bella via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Tue May 11 19:05:21 PDT 2021

cjdb marked 4 inline comments as done.
cjdb added inline comments.

Comment at: libcxx/include/string:4037
+template<class _CharT, class _Traits, class _Allocator>
cjdb wrote:
> Mordante wrote:
> > Quuxplusone wrote:
> > > Either remove `[[nodiscard]]` (the easy path), or replace it with `_LIBCPP_NODISCARD_EXT` and put a libcxx/test/libcxx/ test on it.
> > The `[[nodiscard]]` is an extension, can you the `_LIBCPP_NODISCARD_EXT` macro and add it to the list extensions.
> It's still unclear to me why `_LIBCPP_NODISCARD_EXT` is desirable over an attribute users can already disable.
This doesn't answer my question. Most attributes don't change the meaning of a program, so it doesn't make sense to me why this macro is useful at all. Unless I'm grepping wrong, we don't have a `_LIBCPP_NOEXCEPT_EXT` macro for when the standard doesn't put `noexcept` on things. Why is `[[nodiscard]]` any different? This is at least my fourth time asking this question without an answer; I'd appreciate one please.

Comment at: libcxx/test/std/strings/basic.string/string.nonmembers/string_opcmp/pointer_string.pass.cpp:31
+int main(int, char**)
Mordante wrote:
> Can you turn the test into a templated test and test with all allowed string types? 
> The support header `make_string.h` has some macros that will be useful for these tests.
Not sure I understand what you're asking for, but let's take this up on Discord.

Comment at: libcxx/test/std/strings/basic.string/string.nonmembers/string_opcmp/pointer_string.pass.cpp:14
+// template<class charT, class traits, class Allocator>
+//   bool operator<=>(const charT* lhs, const basic_string<charT,traits,Allocator>& rhs);
Mordante wrote:
> curdeius wrote:
> > `bool`?
> Please address the comment above.
@curdeius was asking why it was `bool`, because it shouldn't be, so I've already addressed it.

Comment at: libcxx/test/std/strings/basic.string/string.nonmembers/string_opcmp/string_string.pass.cpp:12
+// <string>
+// we get this comparison "for free" because the string implicitly converts to the string_view
Mordante wrote:
> Please add the function being tested.
There isn't a function that's being tested. See the comment below.

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list