[libcxx-commits] [PATCH] D128267: [libc++] Enable [[nodiscard]] extensions by default

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/docs/UsingLibcxx.rst:343
 For this reason libc++ provides an extension that does just that! The
-extension must be enabled by defining ``_LIBCPP_ENABLE_NODISCARD_EXT``. The extended
+extension is enabled by default and can be disabled by defining ``_LIBCPP_DISABLE_NODISCARD_EXT``. The extended
 applications of ``[[nodiscard]]`` takes two forms:
----------------
Let's break the line here.


================
Comment at: libcxx/include/__config:868
 
-#  if _LIBCPP_STD_VER > 17 || defined(_LIBCPP_ENABLE_NODISCARD_EXT)
+#  if _LIBCPP_STD_VER > 17 && !defined(_LIBCPP_DISABLE_NODISCARD_EXT)
 #    define _LIBCPP_NODISCARD_AFTER_CXX17 _LIBCPP_NODISCARD
----------------
I think this is what we mean:

1. In C++20, always mark those things required by the standard as nodiscard
2. In < C++20, always mark as nodiscard unless the user has opted out by defining `_LIBCPP_DISABLE_NODISCARD_EXT`.


================
Comment at: libcxx/include/__config:871
 #  else
 #    define _LIBCPP_NODISCARD_AFTER_CXX17
 #  endif
----------------
(not attached to any particular line)

I think we need to go through all of our tests for `nodiscard`. Many of them will currently be marked as `// UNSUPPORTED: c++03, c++11, c++14`. Instead, they should be changed to: `// UNSUPPORTED: (c++03 || c++11 || c++14) && !stdlib=libc++` so that we gain coverage for this newly added behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128267