Skip to content

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Apr 29, 2024

I strongly suspect nobody ever used that macro since it wasn't very well known. Furthermore, it only affects a handful of diagnostics and I think it makes sense to either provide them unconditionally, or to not provided them at all.

I strongly suspect nobody ever used that macro since it wasn't very
well known. Furthermore, it only affects a handful of diagnostics and
I think it makes sense to either provide them unconditionally, or to
not provided them at all.
@ldionne ldionne requested a review from a team as a code owner April 29, 2024 18:39
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Apr 29, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 29, 2024

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

I strongly suspect nobody ever used that macro since it wasn't very well known. Furthermore, it only affects a handful of diagnostics and I think it makes sense to either provide them unconditionally, or to not provided them at all.


Full diff: https://github.com/llvm/llvm-project/pull/90512.diff

3 Files Affected:

  • (modified) libcxx/docs/ReleaseNotes/19.rst (+3)
  • (modified) libcxx/docs/UsingLibcxx.rst (-9)
  • (modified) libcxx/include/__config (+1-1)
diff --git a/libcxx/docs/ReleaseNotes/19.rst b/libcxx/docs/ReleaseNotes/19.rst
index ac4fd0ecc122bd..5a07b11cbcd509 100644
--- a/libcxx/docs/ReleaseNotes/19.rst
+++ b/libcxx/docs/ReleaseNotes/19.rst
@@ -118,6 +118,9 @@ Deprecations and Removals
   a ``std::basic_*fstream`` from a ``std::basic_string_view``, a input-iterator or a C-string, instead
   you can construct a temporary ``std::basic_string``. This change has been applied to C++17 and later.
 
+- The ``_LIBCPP_DISABLE_ADDITIONAL_DIAGNOSTICS`` macro has been removed and is not honored anymore. Additional
+  warnings provided by libc++ as a matter of QoI will now be provided unconditionally.
+
 
 Upcoming Deprecations and Removals
 ----------------------------------
diff --git a/libcxx/docs/UsingLibcxx.rst b/libcxx/docs/UsingLibcxx.rst
index e7aaf4e1fbcf9c..0fdaeb433ac6a0 100644
--- a/libcxx/docs/UsingLibcxx.rst
+++ b/libcxx/docs/UsingLibcxx.rst
@@ -167,15 +167,6 @@ safety annotations.
   build of libc++ which does not export any symbols, which can be useful when
   building statically for inclusion into another library.
 
-**_LIBCPP_DISABLE_ADDITIONAL_DIAGNOSTICS**:
-  This macro disables the additional diagnostics generated by libc++ using the
-  `diagnose_if` attribute. These additional diagnostics include checks for:
-
-    * Giving `set`, `map`, `multiset`, `multimap` and their `unordered_`
-      counterparts a comparator which is not const callable.
-    * Giving an unordered associative container a hasher that is not const
-      callable.
-
 **_LIBCPP_NO_VCRUNTIME**:
   Microsoft's C and C++ headers are fairly entangled, and some of their C++
   headers are fairly hard to avoid. In particular, `vcruntime_new.h` gets pulled
diff --git a/libcxx/include/__config b/libcxx/include/__config
index 97cdd020c55d1f..e4c5c685a45645 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -1390,7 +1390,7 @@ typedef __char32_t char32_t;
 #    define _LIBCPP_NO_DESTROY
 #  endif
 
-#  if __has_attribute(__diagnose_if__) && !defined(_LIBCPP_DISABLE_ADDITIONAL_DIAGNOSTICS)
+#  if __has_attribute(__diagnose_if__)
 #    define _LIBCPP_DIAGNOSE_WARNING(...) __attribute__((__diagnose_if__(__VA_ARGS__, "warning")))
 #  else
 #    define _LIBCPP_DIAGNOSE_WARNING(...)

Copy link
Contributor

@philnik777 philnik777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ldionne ldionne merged commit a00bbcb into llvm:main May 1, 2024
@ldionne ldionne deleted the review/remove-disable-additional-diagnostics branch May 1, 2024 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants