Skip to content
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

[libc++] Don't push and pop extensions diagnostics when using clang modules #85917

Merged

Conversation

philnik777
Copy link
Contributor

Clang modules take a significant compile time hit when pushing and popping diagnostics. Since all the headers are marked as system headers in the modulemap, we can simply disable this pushing and popping when building with clang modules.

@philnik777 philnik777 requested a review from a team as a code owner March 20, 2024 11:05
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Mar 20, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 20, 2024

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

Clang modules take a significant compile time hit when pushing and popping diagnostics. Since all the headers are marked as system headers in the modulemap, we can simply disable this pushing and popping when building with clang modules.


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

1 Files Affected:

  • (modified) libcxx/include/__config (+20-11)
diff --git a/libcxx/include/__config b/libcxx/include/__config
index 11e13e0c24986a..d7f77e4a01ac88 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -838,21 +838,30 @@ typedef __char32_t char32_t;
 #  define _LIBCPP_CLANG_DIAGNOSTIC_IGNORED_CXX23_EXTENSION _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wc++2b-extensions")
 #endif
 
+#  if !__has_feature(modules)
+#    define _LIBCPP_PUSH_EXTENSION_DIAGNOSTICS                                                                         \
+      _LIBCPP_DIAGNOSTIC_PUSH                                                                                          \
+      _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wc++11-extensions")                                                           \
+      _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wc++14-extensions")                                                           \
+      _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wc++17-extensions")                                                           \
+      _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wc++20-extensions")                                                           \
+      _LIBCPP_CLANG_DIAGNOSTIC_IGNORED_CXX23_EXTENSION                                                                 \
+      _LIBCPP_GCC_DIAGNOSTIC_IGNORED("-Wc++14-extensions")                                                             \
+      _LIBCPP_GCC_DIAGNOSTIC_IGNORED("-Wc++17-extensions")                                                             \
+      _LIBCPP_GCC_DIAGNOSTIC_IGNORED("-Wc++20-extensions")                                                             \
+      _LIBCPP_GCC_DIAGNOSTIC_IGNORED("-Wc++23-extensions")
+#    define _LIBCPP_POP_EXTENSION_DIAGNOSTICS _LIBCPP_DIAGNOSTIC_POP
+#  else
+#    define _LIBCPP_PUSH_EXTENSION_DIAGNOSTICS
+#    define _LIBCPP_POP_EXTENSION_DIAGNOSTICS
+#  endif
+
 // Inline namespaces are available in Clang/GCC/MSVC regardless of C++ dialect.
 // clang-format off
-#  define _LIBCPP_BEGIN_NAMESPACE_STD _LIBCPP_DIAGNOSTIC_PUSH                                                          \
-                                      _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wc++11-extensions")                           \
-                                      _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wc++14-extensions")                           \
-                                      _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wc++17-extensions")                           \
-                                      _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wc++20-extensions")                           \
-                                      _LIBCPP_CLANG_DIAGNOSTIC_IGNORED_CXX23_EXTENSION                                 \
-                                      _LIBCPP_GCC_DIAGNOSTIC_IGNORED("-Wc++14-extensions")                             \
-                                      _LIBCPP_GCC_DIAGNOSTIC_IGNORED("-Wc++17-extensions")                             \
-                                      _LIBCPP_GCC_DIAGNOSTIC_IGNORED("-Wc++20-extensions")                             \
-                                      _LIBCPP_GCC_DIAGNOSTIC_IGNORED("-Wc++23-extensions")                             \
+#  define _LIBCPP_BEGIN_NAMESPACE_STD _LIBCPP_PUSH_EXTENSION_DIAGNOSTICS                                               \
                                       namespace _LIBCPP_TYPE_VISIBILITY_DEFAULT std {                                  \
                                inline namespace _LIBCPP_ABI_NAMESPACE {
-#  define _LIBCPP_END_NAMESPACE_STD }} _LIBCPP_DIAGNOSTIC_POP
+#  define _LIBCPP_END_NAMESPACE_STD }} _LIBCPP_POP_EXTENSION_DIAGNOSTICS
 
 #  define _LIBCPP_BEGIN_NAMESPACE_FILESYSTEM _LIBCPP_BEGIN_NAMESPACE_STD                                               \
                                              inline namespace __fs { namespace filesystem {

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

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

LGTM with a comment, thanks.

libcxx/include/__config Show resolved Hide resolved
@philnik777 philnik777 force-pushed the ignore_extension_warnings_with_modules branch from 22ffb51 to 4332cd7 Compare March 23, 2024 12:54
@philnik777 philnik777 merged commit f886dfe into llvm:main Mar 23, 2024
8 of 9 checks passed
@philnik777 philnik777 deleted the ignore_extension_warnings_with_modules branch March 23, 2024 12:54
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