Merge remote-tracking branch 'origin/main' into aballman-tentative-defn-cpp-compat

Author: AaronBallman

clang-tools-extra/docs/ReleaseNotes.rst

@@ -88,6 +88,14 @@ Improvements to clang-doc
 Improvements to clang-query
 ---------------------------
 
+Improvements to include-cleaner
+-------------------------------
+- Deprecated the ``-insert`` and ``-remove`` command line options, and added
+  the ``-disable-remove`` and ``-disable-insert`` command line options as
+  replacements. The previous command line options were confusing because they
+  did not imply the default state of the option (which is inserts and removes
+  being enabled). The new options are easier to understand the semantics of.
+
 Improvements to clang-tidy
 --------------------------
 

clang-tools-extra/include-cleaner/test/tool.cpp

@@ -6,11 +6,11 @@ int x = foo();
 //      CHANGE: - "foobar.h"
 // CHANGE-NEXT: + "foo.h"
 
-//         RUN: clang-include-cleaner -remove=0 -print=changes %s -- -I%S/Inputs/ | FileCheck --check-prefix=INSERT %s
+//         RUN: clang-include-cleaner -disable-remove -print=changes %s -- -I%S/Inputs/ | FileCheck --check-prefix=INSERT %s
 //  INSERT-NOT: - "foobar.h"
 //      INSERT: + "foo.h"
 
-//         RUN: clang-include-cleaner -insert=0 -print=changes %s -- -I%S/Inputs/ | FileCheck --check-prefix=REMOVE %s
+//         RUN: clang-include-cleaner -disable-insert -print=changes %s -- -I%S/Inputs/ | FileCheck --check-prefix=REMOVE %s
 //      REMOVE: - "foobar.h"
 //  REMOVE-NOT: + "foo.h"
 
@@ -58,3 +58,16 @@ int x = foo();
 //        RUN: FileCheck --match-full-lines --check-prefix=EDIT3 %s < %t.cpp
 //      EDIT3: #include "foo.h"
 //  EDIT3-NOT: {{^}}#include "foobar.h"{{$}}
+
+//        RUN: clang-include-cleaner -insert=false -print=changes %s -- -I%S/Inputs/ 2>&1 | \
+//        RUN: FileCheck --check-prefix=DEPRECATED-INSERT %s
+// DEPRECATED-INSERT: warning: '-insert=0' is deprecated in favor of '-disable-insert'. The old flag was confusing since it suggested that inserts were disabled by default, when they were actually enabled.
+
+//       RUN: clang-include-cleaner -remove=false -print=changes %s -- -I%S/Inputs/ 2>&1 | \
+//       RUN: FileCheck --check-prefix=DEPRECATED-REMOVE %s
+// DEPRECATED-REMOVE: warning: '-remove=0' is deprecated in favor of '-disable-remove'. The old flag was confusing since it suggested that removes were disabled by default, when they were actually enabled.
+
+//       RUN: clang-include-cleaner -insert=false -remove=false -print=changes %s -- -I%S/Inputs/ 2>&1 | \
+//       RUN: FileCheck --check-prefix=DEPRECATED-BOTH %s
+// DEPRECATED-BOTH: warning: '-insert=0' is deprecated in favor of '-disable-insert'. The old flag was confusing since it suggested that inserts were disabled by default, when they were actually enabled.
+// DEPRECATED-BOTH: warning: '-remove=0' is deprecated in favor of '-disable-remove'. The old flag was confusing since it suggested that removes were disabled by default, when they were actually enabled.

clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp

@@ -90,19 +90,31 @@ cl::opt<bool> Edit{
     cl::desc("Apply edits to analyzed source files"),
     cl::cat(IncludeCleaner),
 };
-
 cl::opt<bool> Insert{
     "insert",
-    cl::desc("Allow header insertions"),
+    cl::desc(
+        "Allow header insertions (deprecated. Use -disable-insert instead)"),
     cl::init(true),
     cl::cat(IncludeCleaner),
 };
 cl::opt<bool> Remove{
     "remove",
-    cl::desc("Allow header removals"),
+    cl::desc("Allow header removals (deprecated. Use -disable-remove instead)"),
     cl::init(true),
     cl::cat(IncludeCleaner),
 };
+cl::opt<bool> DisableInsert{
+    "disable-insert",
+    cl::desc("Disable header insertions"),
+    cl::init(false),
+    cl::cat(IncludeCleaner),
+};
+cl::opt<bool> DisableRemove{
+    "disable-remove",
+    cl::desc("Disable header removals"),
+    cl::init(false),
+    cl::cat(IncludeCleaner),
+};
 
 std::atomic<unsigned> Errors = ATOMIC_VAR_INIT(0);
 
@@ -183,9 +195,26 @@ class Action : public clang::ASTFrontendAction {
     auto Results =
         analyze(AST.Roots, PP.MacroReferences, PP.Includes, &PI,
                 getCompilerInstance().getPreprocessor(), HeaderFilter);
-    if (!Insert)
+
+    if (!Insert) {
+      llvm::errs()
+          << "warning: '-insert=0' is deprecated in favor of "
+             "'-disable-insert'. "
+             "The old flag was confusing since it suggested that inserts "
+             "were disabled by default, when they were actually enabled.\n";
+    }
+
+    if (!Remove) {
+      llvm::errs()
+          << "warning: '-remove=0' is deprecated in favor of "
+             "'-disable-remove'. "
+             "The old flag was confusing since it suggested that removes "
+             "were disabled by default, when they were actually enabled.\n";
+    }
+
+    if (!Insert || DisableInsert)
       Results.Missing.clear();
-    if (!Remove)
+    if (!Remove || DisableRemove)
       Results.Unused.clear();
     std::string Final = fixIncludes(Results, AbsPath, Code, getStyle(AbsPath));
 

clang/docs/ReleaseNotes.rst

@@ -141,12 +141,16 @@ C Language Changes
   function type in Microsoft compatibility mode. #GH124869
 - Clang now allows ``restrict`` qualifier for array types with pointer elements (#GH92847).
 - Clang now diagnoses ``const``-qualified object definitions without an
-  initializer. If the object is zero-initialized, it will be diagnosed under
-  the new warning ``-Wdefault-const-init`` (which is grouped under
-  ``-Wc++-compat`` because this construct is not compatible with C++). If the
-  object is left uninitialized, it will be diagnosed unsed the new warning
-  ``-Wdefault-const-init-unsafe`` (which is grouped under
-  ``-Wdefault-const-init``). #GH19297
+  initializer. If the object is a variable or field which is zero-initialized,
+  it will be diagnosed under the new warning ``-Wdefault-const-init-var`` or
+  ``-Wdefault-const-init-field``, respectively. Similarly, if the variable or
+  field is not zero-initialized, it will be diagnosed under the new diagnostic
+  ``-Wdefault-const-init-var-unsafe`` or ``-Wdefault-const-init-field-unsafe``,
+  respectively. The unsafe diagnostic variants are grouped under a new
+  diagnostic ``-Wdefault-const-init-unsafe``, which itself is grouped under the
+  new diagnostic ``-Wdefault-const-init``. Finally, ``-Wdefault-const-init`` is
+  grouped under ``-Wc++-compat`` because these constructs are not compatible
+  with C++. #GH19297
 - Added ``-Wimplicit-void-ptr-cast``, grouped under ``-Wc++-compat``, which
   diagnoses implicit conversion from ``void *`` to another pointer type as
   being incompatible with C++. (#GH17792)

clang/include/clang/Basic/DiagnosticGroups.td

@@ -163,8 +163,16 @@ def InitStringTooLongMissingNonString :
 def InitStringTooLongForCpp :
   DiagGroup<"c++-unterminated-string-initialization">;
 def HiddenCppDecl : DiagGroup<"c++-hidden-decl">;
-def DefaultConstInitUnsafe : DiagGroup<"default-const-init-unsafe">;
-def DefaultConstInit : DiagGroup<"default-const-init", [DefaultConstInitUnsafe]>;
+def DefaultConstInitFieldUnsafe : DiagGroup<"default-const-init-field-unsafe">;
+def DefaultConstInitVarUnsafe : DiagGroup<"default-const-init-var-unsafe">;
+def DefaultConstInitUnsafe : DiagGroup<"default-const-init-unsafe",
+                                       [DefaultConstInitFieldUnsafe,
+                                        DefaultConstInitVarUnsafe]>;
+def DefaultConstInitField : DiagGroup<"default-const-init-field">;
+def DefaultConstInitVar : DiagGroup<"default-const-init-var">;
+def DefaultConstInit : DiagGroup<"default-const-init",
+                                 [DefaultConstInitField, DefaultConstInitVar,
+                                  DefaultConstInitUnsafe]>;
 def ImplicitVoidPtrCast : DiagGroup<"implicit-void-ptr-cast">;
 def ImplicitIntToEnumCast : DiagGroup<"implicit-int-enum-cast",
                                       [ImplicitEnumEnumCast]>;

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -8221,14 +8221,20 @@ def err_address_space_qualified_delete : Error<
 
 def note_default_init_const_member : Note<
   "member %0 declared 'const' here">;
+def warn_default_init_const_field : Warning<
+  "default initialization of an object of type %0 with const member is "
+  "incompatible with C++">, InGroup<DefaultConstInitField>, DefaultIgnore;
 def warn_default_init_const : Warning<
-  "default initialization of an object of type %0%select{| with const member}1 "
-  "is incompatible with C++">,
-  InGroup<DefaultConstInit>, DefaultIgnore;
+  "default initialization of an object of type %0 is incompatible with C++">,
+  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++">,
+  InGroup<DefaultConstInitFieldUnsafe>;
 def warn_default_init_const_unsafe : Warning<
-  "default initialization of an object of type %0%select{| with const member}1 "
-  "leaves the object uninitialized and is incompatible with C++">,
-  InGroup<DefaultConstInitUnsafe>;
+  "default initialization of an object of type %0 leaves the object "
+  "uninitialized and is incompatible with C++">,
+  InGroup<DefaultConstInitVarUnsafe>;
 def err_default_init_const : Error<
   "default initialization of an object of const type %0"
   "%select{| without a user-provided default constructor}1">;

clang/lib/Sema/Sema.cpp

@@ -1457,7 +1457,7 @@ void Sema::ActOnEndOfTranslationUnit() {
       if (VD->getStorageDuration() == SD_Static ||
           VD->getStorageDuration() == SD_Thread)
         DiagID = diag::warn_default_init_const;
-      Diag(VD->getLocation(), DiagID) << Type << /*not a field*/ 0;
+      Diag(VD->getLocation(), DiagID) << Type;
     }
 
     // Notify the consumer that we've completed a tentative definition.

clang/lib/Sema/SemaDecl.cpp

@@ -14351,7 +14351,7 @@ 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 << /*not a field*/ 0;
+      Diag(Var->getLocation(), DiagID) << Type;
     }
 
     // Check for jumps past the implicit initializer.  C++0x

clang/lib/Sema/SemaInit.cpp

@@ -6607,12 +6607,12 @@ void InitializationSequence::InitializeFrom(Sema &S,
       // initializer present.
       if (!Initializer) {
         if (const FieldDecl *FD = getConstField(Rec)) {
-          unsigned DiagID = diag::warn_default_init_const_unsafe;
+          unsigned DiagID = diag::warn_default_init_const_field_unsafe;
           if (Var->getStorageDuration() == SD_Static ||
               Var->getStorageDuration() == SD_Thread)
-            DiagID = diag::warn_default_init_const;
+            DiagID = diag::warn_default_init_const_field;
 
-          S.Diag(Var->getLocation(), DiagID) << Var->getType() << /*member*/ 1;
+          S.Diag(Var->getLocation(), DiagID) << Var->getType();
           S.Diag(FD->getLocation(), diag::note_default_init_const_member) << FD;
         }
       }

clang/test/Sema/warn-default-const-init.c

@@ -1,28 +1,45 @@
-// RUN: %clang_cc1 -fsyntax-only -verify=c,unsafe -Wdefault-const-init %s
-// RUN: %clang_cc1 -fsyntax-only -verify=c,unsafe -Wc++-compat -Wno-tentative-definition-compat %s
-// RUN: %clang_cc1 -fsyntax-only -verify=unsafe %s
-// RUN: %clang_cc1 -fsyntax-only -verify=c -Wdefault-const-init -Wno-default-const-init-unsafe %s
+// 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,unsafe-field,zero-init-var,zero-init-field -Wdefault-const-init %s
+
+// This should enable nothing.
 // RUN: %clang_cc1 -fsyntax-only -verify=good -Wno-default-const-init-unsafe %s
+
+// Only unsafe field and variable diagnostics
+// RUN: %clang_cc1 -fsyntax-only -verify=unsafe-var,unsafe-field %s
+// RUN: %clang_cc1 -fsyntax-only -verify=unsafe-var,unsafe-field -Wdefault-const-init-unsafe %s
+
+// Only zero init field and variable diagnostics
+// RUN: %clang_cc1 -fsyntax-only -verify=zero-init-var,zero-init-field -Wdefault-const-init -Wno-default-const-init-unsafe %s
+
+// Only zero init and unsafe field diagnostics
+// RUN: %clang_cc1 -fsyntax-only -verify=zero-init-field,unsafe-field -Wno-default-const-init-var-unsafe -Wdefault-const-init-field %s
+
+// Only zero init and unsafe variable diagnostics
+// RUN: %clang_cc1 -fsyntax-only -verify=zero-init-var,unsafe-var -Wno-default-const-init-field-unsafe -Wdefault-const-init-var %s
+
+// C++ tests
 // RUN: %clang_cc1 -fsyntax-only -verify=cxx -x c++ %s
+
 // good-no-diagnostics
 
 struct A { int i; };
-struct S{ const int i; };              // unsafe-note 2 {{member 'i' declared 'const' here}} \
+struct S{ const int i; };              // unsafe-field-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-note {{member 'a' declared 'const' here}} \
+struct V { int i; const struct A a; }; // unsafe-field-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-note {{member 'j' declared 'const' here}} \
+struct W { struct A a; const int j; }; // unsafe-field-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-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 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-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 and is incompatible with C++}} \
                   cxx-error {{call to implicitly-deleted default constructor of 'struct T'}}
   struct T t2 = { { 0 } };
 }
@@ -31,13 +48,13 @@ void h() {
   struct U u2 = { { 0 }, 0 };
 }
 void x() {
-  struct V v1; // unsafe-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 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-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 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 };
@@ -47,17 +64,17 @@ void y() {
 extern const int i;
 const int i = 12;
 
-static const int j; // c-warning {{default initialization of an object of type 'const int' is incompatible with C++}} \
+static const int j; // 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'}}
-const int k;        // c-warning {{default initialization of an object of type 'const int' is incompatible with C++}} \
+const int k;        // 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'}}
-const struct S s;   // c-warning {{default initialization of an object of type 'const struct S' is incompatible with C++}} \
+const struct S s;   // zero-init-var-warning {{default initialization of an object of type 'const struct S' is incompatible with C++}} \
                        cxx-error {{call to implicitly-deleted default constructor of 'const struct S'}}
 
 void func() {
-  const int a;        // unsafe-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 and is incompatible with C++}} \
                          cxx-error {{default initialization of an object of const type 'const int'}}
-  static const int b; // c-warning {{default initialization of an object of type 'const int' is incompatible with C++}} \
+  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'}}
 }