Skip to content

[libc++] Remove _LIBCPP_DISABLE_ADDITIONAL_DIAGNOSTICS #90512

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

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