[libcxx-commits] [PATCH] D129054: [libc++] Consolidate the different [[nodiscard]] configuration options into a single one

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

I wonder whether we want to take this approach. We've had patches in the past to remove this feature and make all `[[nodiscard]]` extensions always enabled. MSVC STL already does this and there have been done some tests on Google and Apple codebases. (I don't recall whether the results have been posted.)

How about going in that direction instead? probably we should then do this shortly after LLVM 15 is branched.



================
Comment at: libcxx/docs/ReleaseNotes.rst:166
+  can now be enabled by defining ``_LIBCPP_ENABLE_NODISCARD_EXT``. Note that this also enables applications of
+  ``[[nodiscard]]`` in places where no version of the standard requires the attribute.
+
----------------
I feel this wording can use a bit of polish. I have a hard time to understand what this means.


================
Comment at: libcxx/include/__config:833
+#  if _LIBCPP_STD_VER > 17 || defined(_LIBCPP_ENABLE_NODISCARD_EXT)
 #    define _LIBCPP_NODISCARD_AFTER_CXX17 _LIBCPP_NODISCARD
 #  else
----------------
Do we need two different macros or can they be combined? It doesn't need to be done in this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129054