[PATCH] D133425: Silence -Wctad-maybe-unsupported stemming from system headers

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: dblaikie, ldionne, clang-language-wg, libc++.
Herald added a project: All.
aaron.ballman requested review of this revision.
Herald added a project: clang.

Currently, this diagnostic fires on system header code that the user has no control over, such as `std::lock_guard` from `<mutex>` in both libc++ and libstdc++. I believe the diagnostic should be silenced when the class template exists in a system header, for two reasons: 1) the user can't fix that code anyway, so their only recourse is to disable the diagnostic, 2) system headers (especially STL headers) have had time to add the CTAD guides where necessary, and so the warning is more likely to be a false positive in those cases.

This issue was observed by a customer in Intel's downstream and is also related to Issue #44202.

(Adding libc++ reviewers as an FYI for the diagnostic firing in their headers and as a sounding board for whether they agree we should disable this diagnostic in system headers by default.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133425

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaInit.cpp
  clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp


Index: clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
===================================================================
--- clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
+++ clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
@@ -519,6 +519,18 @@
 TestSuppression ta("abc");
 static_assert(__is_same(decltype(ta), TestSuppression<const char *>), "");
 }
+
+// Test that the diagnostic is suppressed if within a system header.
+# 2 "test.h" 1 3
+template <class T>
+struct SysHeaderObj {
+  SysHeaderObj(T) {}
+  SysHeaderObj(T, int) {}
+};
+# 6 "test.h" 2
+
+SysHeaderObj sho(42); // Okay because SysHeaderObj is from a system header.
+
 #pragma clang diagnostic pop
 
 namespace PR41549 {
Index: clang/lib/Sema/SemaInit.cpp
===================================================================
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -10295,16 +10295,16 @@
   //  by overload resolution for class template deduction.
   QualType DeducedType =
       SubstAutoType(TSInfo->getType(), Best->Function->getReturnType());
-  Diag(TSInfo->getTypeLoc().getBeginLoc(),
-       diag::warn_cxx14_compat_class_template_argument_deduction)
+  SourceLocation TyLoc = TSInfo->getTypeLoc().getBeginLoc();
+  Diag(TyLoc, diag::warn_cxx14_compat_class_template_argument_deduction)
       << TSInfo->getTypeLoc().getSourceRange() << 1 << DeducedType;
 
   // Warn if CTAD was used on a type that does not have any user-defined
-  // deduction guides.
-  if (!HasAnyDeductionGuide) {
-    Diag(TSInfo->getTypeLoc().getBeginLoc(),
-         diag::warn_ctad_maybe_unsupported)
-        << TemplateName;
+  // deduction guides, but do not warn if the deduction guide is in a system
+  // header on the assumption that the support is intentional there.
+  if (!HasAnyDeductionGuide &&
+      !SourceMgr.isInSystemHeader(Template->getLocation())) {
+    Diag(TyLoc, diag::warn_ctad_maybe_unsupported) << TemplateName;
     Diag(Template->getLocation(), diag::note_suppress_ctad_maybe_unsupported);
   }
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2470,7 +2470,7 @@
   InGroup<CXXPre17Compat>, DefaultIgnore;
 def warn_ctad_maybe_unsupported : Warning<
   "%0 may not intend to support class template argument deduction">,
-  InGroup<CTADMaybeUnsupported>, DefaultIgnore;
+  InGroup<DiagGroup<"ctad-maybe-unsupported">>, DefaultIgnore;
 def note_suppress_ctad_maybe_unsupported : Note<
   "add a deduction guide to suppress this warning">;
 
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1329,8 +1329,6 @@
 // A group for cross translation unit static analysis related warnings.
 def CrossTU : DiagGroup<"ctu">;
 
-def CTADMaybeUnsupported : DiagGroup<"ctad-maybe-unsupported">;
-
 def FortifySource : DiagGroup<"fortify-source">;
 
 def MaxTokens : DiagGroup<"max-tokens"> {
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -132,6 +132,9 @@
 - no_sanitize("...") on a global variable for known but not relevant sanitizers
   is now just a warning. It now says that this will be ignored instead of
   incorrectly saying no_sanitize only applies to functions and methods.
+- No longer issue ``-Wctad-maybe-unsupported`` warnings when the template which
+  might not support CTAD is defined in a system header. This silences warnings
+  when using ``std::lock_guard`` from ``<mutex>``.
 
 Non-comprehensive list of changes in this release
 -------------------------------------------------


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D133425.458451.patch
Type: text/x-patch
Size: 3927 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220907/5b087aa1/attachment-0001.bin>