[libcxx-commits] [PATCH] D137186: [libc++] Add [[nodiscard]] extensions to ranges algorithms

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

FWIW I got positive feedback from some folks on our nodiscard extensions. I resisted them for a while but I now definitely think they're an improvement.



================
Comment at: libcxx/test/libcxx/diagnostics/ranges.nodiscard.compile.pass.cpp:1
+//===----------------------------------------------------------------------===//
+//
----------------
Filename `ranges.nodiscard_extensions.compile.pass.cpp` maybe? Not as cute but it explains better what it's about.


================
Comment at: libcxx/test/libcxx/diagnostics/ranges.nodiscard.compile.pass.cpp:12-14
+// Disable any builtin recognition of std::* in the compiler, that might also
+// trigger -Wunused-value warnings.
+// ADDITIONAL_COMPILE_FLAGS: -fno-builtin
----------------
I don't think we should try to guard this test against potential future changes in Clang like that. Clang should never implement ranges algorithms as builtins, that sounds like a weird thing to do.


================
Comment at: libcxx/test/libcxx/diagnostics/ranges.nodiscard.verify.cpp:9-10
+
+// Check that ranges algorithms aren't marked [[nodiscard]] when
+// _LIBCPP_DISBALE_NODISCARD_EXT is defined
+
----------------
This is an incorrect copy-paste.


================
Comment at: libcxx/test/libcxx/diagnostics/ranges.nodiscard.verify.cpp:12-14
+// Disable any builtin recognition of std::* in the compiler, that might also
+// trigger -Wunused-value warnings.
+// ADDITIONAL_COMPILE_FLAGS: -fno-builtin
----------------
Same comment, let's remove.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137186