[C] Update the -Wdefault-const-init-unsafe wording

[AaronBallman]

This drops the "and is incompatible with C++" phrasing from the diagnostic unless -Wc++-compat is explicitly passed. This makes the diagnostic less confusing when it is on by default rather than enabled because of C++ compatibility concerns

[llvmbot]

@llvm/pr-subscribers-clang

Author: Aaron Ballman (AaronBallman)

Changes

This drops the "and is incompatible with C++" phrasing from the diagnostic unless -Wc++-compat is explicitly passed. This makes the diagnostic less confusing when it is on by default rather than enabled because of C++ compatibility concerns

---

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

7 Files Affected:

• (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+12-2)
• (modified) clang/lib/Sema/Sema.cpp (+6-1)
• (modified) clang/lib/Sema/SemaDecl.cpp (+6-1)
• (modified) clang/lib/Sema/SemaInit.cpp (+5-1)
• (modified) clang/test/Sema/sizeless-1.c (+2-2)
• (modified) clang/test/Sema/typedef-retain.c (+1-1)
• (modified) clang/test/Sema/warn-default-const-init.c (+14-7)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 604bb56d28252..9a15d69d5d78e 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6541,6 +6541,16 @@ def warn_missing_braces : Warning<
   "suggest braces around initialization of subobject">,
   InGroup<MissingBraces>, DefaultIgnore;
 
+// This diagnostic exists only to determine whether -Wc++-compat was explicitly
+// enabled. This allows us to tell the difference between when a diagnostic was
+// enabled by default, was enabled because its subgroup was enabled, or enabled
+// because the -Wc++-compat superset was enabled, generally for purposes of
+// deciding whether to emit "and is incompatible with C++" on diagnostics which
+// are useful in C alone as well as for compatibility checks.
+def warn_cxx_compat_hack_fake_diagnostic_do_not_emit : Warning<
+  "if you see this diagnostic, a Clang developer has made a mistake">,
+  InGroup<CXXCompat>, DefaultIgnore;
+
 def err_redefinition_of_label : Error<"redefinition of label %0">;
 def err_undeclared_label_use : Error<"use of undeclared label %0">;
 def err_goto_ms_asm_label : Error<
@@ -8234,11 +8244,11 @@ def warn_default_init_const : Warning<
   InGroup<DefaultConstInitVar>, DefaultIgnore;
 def warn_default_init_const_field_unsafe : Warning<
   "default initialization of an object of type %0 with const member leaves the "
-  "object uninitialized and is incompatible with C++">,
+  "object uninitialized%select{| and is incompatible with C++}1">,
   InGroup<DefaultConstInitFieldUnsafe>;
 def warn_default_init_const_unsafe : Warning<
   "default initialization of an object of type %0 leaves the object "
-  "uninitialized and is incompatible with C++">,
+  "uninitialized%select{| and is incompatible with C++}1">,
   InGroup<DefaultConstInitVarUnsafe>;
 def err_default_init_const : Error<
   "default initialization of an object of const type %0"
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index b9d33d67ca7e4..1901d19b14dfc 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -1457,7 +1457,12 @@ void Sema::ActOnEndOfTranslationUnit() {
       if (VD->getStorageDuration() == SD_Static ||
           VD->getStorageDuration() == SD_Thread)
         DiagID = diag::warn_default_init_const;
-      Diag(VD->getLocation(), DiagID) << Type;
+
+      bool EmitCppCompat = !Diags.isIgnored(
+          diag::warn_cxx_compat_hack_fake_diagnostic_do_not_emit,
+          VD->getLocation());
+
+      Diag(VD->getLocation(), DiagID) << Type << EmitCppCompat;
     }
 
     // Notify the consumer that we've completed a tentative definition.
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index d7421934032cf..6dbdb4e86278e 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -14367,7 +14367,12 @@ void Sema::ActOnUninitializedDecl(Decl *RealDecl) {
       if (Var->getStorageDuration() == SD_Static ||
           Var->getStorageDuration() == SD_Thread)
         DiagID = diag::warn_default_init_const;
-      Diag(Var->getLocation(), DiagID) << Type;
+
+      bool EmitCppCompat = !Diags.isIgnored(
+          diag::warn_cxx_compat_hack_fake_diagnostic_do_not_emit,
+          Var->getLocation());
+
+      Diag(Var->getLocation(), DiagID) << Type << EmitCppCompat;
     }
 
     // Check for jumps past the implicit initializer.  C++0x
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 3e29a5bf96a0c..160fbddbdff15 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -6627,7 +6627,11 @@ void InitializationSequence::InitializeFrom(Sema &S,
               Var->getStorageDuration() == SD_Thread)
             DiagID = diag::warn_default_init_const_field;
 
-          S.Diag(Var->getLocation(), DiagID) << Var->getType();
+          bool EmitCppCompat = !S.Diags.isIgnored(
+              diag::warn_cxx_compat_hack_fake_diagnostic_do_not_emit,
+              Var->getLocation());
+
+          S.Diag(Var->getLocation(), DiagID) << Var->getType() << EmitCppCompat;
           S.Diag(FD->getLocation(), diag::note_default_init_const_member) << FD;
         }
       }
diff --git a/clang/test/Sema/sizeless-1.c b/clang/test/Sema/sizeless-1.c
index af96023f4bff5..b6b92e7a68c19 100644
--- a/clang/test/Sema/sizeless-1.c
+++ b/clang/test/Sema/sizeless-1.c
@@ -94,12 +94,12 @@ void func(int sel) {
   svint8_t bad_brace_init_int8_6 = {{local_int8, 0}};  // expected-warning {{too many braces around initializer}}
 
   const svint8_t const_int8 = local_int8; // expected-note {{declared const here}}
-  const svint8_t uninit_const_int8;       // expected-warning {{default initialization of an object of type 'const svint8_t' (aka 'const __SVInt8_t') leaves the object uninitialized and is incompatible with C++}};
+  const svint8_t uninit_const_int8;       // expected-warning {{default initialization of an object of type 'const svint8_t' (aka 'const __SVInt8_t') leaves the object uninitialized}};
 
   volatile svint8_t volatile_int8;
 
   const volatile svint8_t const_volatile_int8 = local_int8; // expected-note {{declared const here}}
-  const volatile svint8_t uninit_const_volatile_int8;       // expected-warning {{default initialization of an object of type 'const volatile svint8_t' (aka 'const volatile __SVInt8_t') leaves the object uninitialized and is incompatible with C++}}
+  const volatile svint8_t uninit_const_volatile_int8;       // expected-warning {{default initialization of an object of type 'const volatile svint8_t' (aka 'const volatile __SVInt8_t') leaves the object uninitialized}}
 
   _Atomic svint8_t atomic_int8;      // expected-error {{_Atomic cannot be applied to sizeless type 'svint8_t'}}
   __restrict svint8_t restrict_int8; // expected-error {{requires a pointer or reference}}
diff --git a/clang/test/Sema/typedef-retain.c b/clang/test/Sema/typedef-retain.c
index 76715ca360cbe..71375ad7a8127 100644
--- a/clang/test/Sema/typedef-retain.c
+++ b/clang/test/Sema/typedef-retain.c
@@ -17,7 +17,7 @@ typedef int a[5];
 void test3(void) {
   typedef const a b;
   b r;       // expected-note {{variable 'r' declared const here}} \
-                expected-warning {{default initialization of an object of type 'b' (aka 'const int[5]') leaves the object uninitialized and is incompatible with C++}}
+                expected-warning {{default initialization of an object of type 'b' (aka 'const int[5]') leaves the object uninitialized}}
   r[0] = 10; // expected-error {{cannot assign to variable 'r' with const-qualified type 'b' (aka 'const int[5]')}}
 }
 
diff --git a/clang/test/Sema/warn-default-const-init.c b/clang/test/Sema/warn-default-const-init.c
index ddd4aba2b5de3..e788d72899685 100644
--- a/clang/test/Sema/warn-default-const-init.c
+++ b/clang/test/Sema/warn-default-const-init.c
@@ -1,5 +1,5 @@
 // Both of these should enable everything.
-// RUN: %clang_cc1 -fsyntax-only -verify=unsafe-var,unsafe-field,zero-init-var,zero-init-field -Wc++-compat -Wno-tentative-definition-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify=unsafe-var-compat,unsafe-field-compat,zero-init-var,zero-init-field -Wc++-compat -Wno-tentative-definition-compat %s
 // RUN: %clang_cc1 -fsyntax-only -verify=unsafe-var,unsafe-field,zero-init-var,zero-init-field -Wdefault-const-init %s
 
 // This should enable nothing.
@@ -25,21 +25,26 @@
 
 struct A { int i; };
 struct S{ const int i; };              // unsafe-field-note 2 {{member 'i' declared 'const' here}} \
+                                          unsafe-field-compat-note 2 {{member 'i' declared 'const' here}} \
                                           cxx-note 3 {{default constructor of 'S' is implicitly deleted because field 'i' of const-qualified type 'const int' would not be initialized}}
 struct T { struct S s; };              // cxx-note {{default constructor of 'T' is implicitly deleted because field 's' has a deleted default constructor}}
 struct U { struct S s; const int j; };
 struct V { int i; const struct A a; }; // unsafe-field-note {{member 'a' declared 'const' here}} \
+                                          unsafe-field-compat-note {{member 'a' declared 'const' here}} \
                                           cxx-note {{default constructor of 'V' is implicitly deleted because field 'a' of const-qualified type 'const struct A' would not be initialized}}
 struct W { struct A a; const int j; }; // unsafe-field-note {{member 'j' declared 'const' here}} \
+                                          unsafe-field-compat-note {{member 'j' declared 'const' here}} \
                                           cxx-note {{default constructor of 'W' is implicitly deleted because field 'j' of const-qualified type 'const int' would not be initialized}}
 
 void f() {
-  struct S s1; // unsafe-field-warning {{default initialization of an object of type 'struct S' with const member leaves the object uninitialized and is incompatible with C++}} \
+  struct S s1; // unsafe-field-warning {{default initialization of an object of type 'struct S' with const member leaves the object uninitialized}} \
+                  unsafe-field-compat-warning {{default initialization of an object of type 'struct S' with const member leaves the object uninitialized and is incompatible with C++}} \
                   cxx-error {{call to implicitly-deleted default constructor of 'struct S'}}
   struct S s2 = { 0 };
 }
 void g() {
-  struct T t1; // unsafe-field-warning {{default initialization of an object of type 'struct T' with const member leaves the object uninitialized and is incompatible with C++}} \
+  struct T t1; // unsafe-field-warning {{default initialization of an object of type 'struct T' with const member leaves the object uninitialized}} \
+                  unsafe-field-compat-warning {{default initialization of an object of type 'struct T' with const member leaves the object uninitialized and is incompatible with C++}} \
                   cxx-error {{call to implicitly-deleted default constructor of 'struct T'}}
   struct T t2 = { { 0 } };
 }
@@ -48,13 +53,15 @@ void h() {
   struct U u2 = { { 0 }, 0 };
 }
 void x() {
-  struct V v1; // unsafe-field-warning {{default initialization of an object of type 'struct V' with const member leaves the object uninitialized and is incompatible with C++}} \
+  struct V v1; // unsafe-field-warning {{default initialization of an object of type 'struct V' with const member leaves the object uninitialized}} \
+                  unsafe-field-compat-warning {{default initialization of an object of type 'struct V' with const member leaves the object uninitialized and is incompatible with C++}} \
                   cxx-error {{call to implicitly-deleted default constructor of 'struct V'}}
   struct V v2 = { 0 };
   struct V v3 = { 0, { 0 } };
 }
 void y() {
-  struct W w1; // unsafe-field-warning {{default initialization of an object of type 'struct W' with const member leaves the object uninitialized and is incompatible with C++}} \
+  struct W w1; // unsafe-field-warning {{default initialization of an object of type 'struct W' with const member leaves the object uninitialized}} \
+                  unsafe-field-compat-warning {{default initialization of an object of type 'struct W' with const member leaves the object uninitialized and is incompatible with C++}} \
                   cxx-error {{call to implicitly-deleted default constructor of 'struct W'}}
   struct W w2 = { 0 };
   struct W w3 = { { 0 }, 0 };
@@ -72,9 +79,9 @@ const struct S s;   // zero-init-var-warning {{default initialization of an obje
                        cxx-error {{call to implicitly-deleted default constructor of 'const struct S'}}
 
 void func() {
-  const int a;        // unsafe-var-warning {{default initialization of an object of type 'const int' leaves the object uninitialized and is incompatible with C++}} \
+  const int a;        // unsafe-var-warning {{default initialization of an object of type 'const int' leaves the object uninitialized}} \
+                         unsafe-var-compat-warning {{default initialization of an object of type 'const int' leaves the object uninitialized and is incompatible with C++}} \
                          cxx-error {{default initialization of an object of const type 'const int'}}
   static const int b; // zero-init-var-warning {{default initialization of an object of type 'const int' is incompatible with C++}} \
                          cxx-error {{default initialization of an object of const type 'const int'}}
 }
-

[erichkeane]

I'm not sure I buy the justification, and the flag-diag there is perhaps 'cute' (and frankly, if we find ourself doing that again, I would vastly prefer we do something like this in tablegen/Diagnostics directly to auto generate/check these things in a way that cannot be called.

[AaronBallman]

I'm not sure I buy the justification

FWIW, this came from a post-commit review comment: #137166 (comment)

, and the flag-diag there is perhaps 'cute' (and frankly, if we find ourself doing that again, I would vastly prefer we do something like this in tablegen/Diagnostics directly to auto generate/check these things in a way that cannot be called.

Yeah, at some point we may want to update the diagnostics engine so it can track groups directly instead of by warning, or at the very least, hide this kind of hack behind an interface on DiagnosticsEngine.

[erichkeane]

I'm not sure I buy the justification

FWIW, this came from a post-commit review comment: #137166 (comment)

, and the flag-diag there is perhaps 'cute' (and frankly, if we find ourself doing that again, I would vastly prefer we do something like this in tablegen/Diagnostics directly to auto generate/check these things in a way that cannot be called.

Yeah, at some point we may want to update the diagnostics engine so it can track groups directly instead of by warning, or at the very least, hide this kind of hack behind an interface on DiagnosticsEngine.

I'd seen that, I still am not sure I buy the justification anyway, but if others want it and you're sympathetic, I am not against it. The Diagnostics hack is wacky/hurts me, but this is the one 'pass' :D