improvements for initialization lists

Author: balazske

clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.cpp

@@ -8,6 +8,7 @@
 
 #include "InvalidEnumDefaultInitializationCheck.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/TypeVisitor.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include <algorithm>
 
@@ -17,51 +18,118 @@ namespace clang::tidy::bugprone {
 
 namespace {
 
-AST_MATCHER(EnumDecl, isCompleteAndHasNoZeroValue) {
-  const EnumDecl *Definition = Node.getDefinition();
-  return Definition && Node.isComplete() && !Node.enumerators().empty() &&
+bool isCompleteAndHasNoZeroValue(const EnumDecl *D) {
+  const EnumDecl *Definition = D->getDefinition();
+  return Definition && Definition->isComplete() &&
+         !Definition->enumerators().empty() &&
          std::none_of(Definition->enumerator_begin(),
                       Definition->enumerator_end(),
                       [](const EnumConstantDecl *Value) {
                         return Value->getInitVal().isZero();
                       });
 }
 
+AST_MATCHER(EnumDecl, isCompleteAndHasNoZeroValue) {
+  return isCompleteAndHasNoZeroValue(&Node);
+}
+
+// Find an initialization which initializes the value (if it has enum type) to a
+// default zero value.
 AST_MATCHER(Expr, isEmptyInit) {
   if (isa<CXXScalarValueInitExpr>(&Node))
     return true;
   if (isa<ImplicitValueInitExpr>(&Node))
     return true;
-  if (const auto *Init = dyn_cast<InitListExpr>(&Node))
-    return Init->getNumInits() == 0;
+  if (const auto *Init = dyn_cast<InitListExpr>(&Node)) {
+    if (Init->getNumInits() == 0)
+      return true;
+  }
   return false;
 }
 
+AST_MATCHER(InitListExpr, hasArrayFiller) { return Node.hasArrayFiller(); }
+
+// Check if any type has a "child" type that is an enum without zero value.
+// The "child" type can be an array element type or member type of a record
+// type (or a recursive combination of these). In this case, if the "root" type
+// is statically initialized, the enum component is initialized to zero.
+class FindEnumMember : public TypeVisitor<FindEnumMember, bool> {
+public:
+  const EnumType *FoundEnum = nullptr;
+
+  bool VisitType(const Type *T) {
+    const Type *DesT = T->getUnqualifiedDesugaredType();
+    if (DesT != T)
+      return Visit(DesT);
+    return false;
+  }
+  bool VisitArrayType(const ArrayType *T) {
+    return Visit(T->getElementType()->getUnqualifiedDesugaredType());
+  }
+  bool VisitConstantArrayType(const ConstantArrayType *T) {
+    return Visit(T->getElementType()->getUnqualifiedDesugaredType());
+  }
+  bool VisitEnumType(const EnumType *T) {
+    if (isCompleteAndHasNoZeroValue(T->getDecl())) {
+      FoundEnum = T;
+      return true;
+    }
+    return false;
+  }
+  bool VisitRecordType(const RecordType *T) {
+    const RecordDecl *RD = T->getDecl();
+    if (RD->isUnion())
+      return false;
+    auto VisitField = [this](const FieldDecl *F) {
+      return Visit(F->getType()->getUnqualifiedDesugaredType());
+    };
+    return llvm::any_of(RD->fields(), VisitField);
+  }
+};
+
 } // namespace
 
 InvalidEnumDefaultInitializationCheck::InvalidEnumDefaultInitializationCheck(
     StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context) {}
 
-bool InvalidEnumDefaultInitializationCheck::isLanguageVersionSupported(
-    const LangOptions &LangOpts) const {
-  return LangOpts.CPlusPlus;
-}
-
 void InvalidEnumDefaultInitializationCheck::registerMatchers(
     MatchFinder *Finder) {
+  auto EnumWithoutZeroValue = enumType(
+      hasDeclaration(enumDecl(isCompleteAndHasNoZeroValue()).bind("enum")));
+  auto EnumOrArrayOfEnum = qualType(hasUnqualifiedDesugaredType(
+      anyOf(EnumWithoutZeroValue,
+            arrayType(hasElementType(qualType(
+                hasUnqualifiedDesugaredType(EnumWithoutZeroValue)))))));
   Finder->addMatcher(
-      expr(isEmptyInit(),
-           hasType(hasUnqualifiedDesugaredType(enumType(hasDeclaration(
-               enumDecl(isCompleteAndHasNoZeroValue()).bind("enum"))))))
-          .bind("expr"),
-      this);
+      expr(isEmptyInit(), hasType(EnumOrArrayOfEnum)).bind("expr"), this);
+
+  // Array initialization can contain an "array filler" for the (syntactically)
+  // unspecified elements. This expression is not found by AST matchers and can
+  // have any type (the array's element type). This is an implicitly generated
+  // initialization, so if the type contains somewhere an enum without zero
+  // enumerator, the zero initialization applies here. We search this array
+  // element type for the specific enum type manually when this matcher matches.
+  Finder->addMatcher(initListExpr(hasArrayFiller()).bind("array_filler_expr"),
+                     this);
 }
 
 void InvalidEnumDefaultInitializationCheck::check(
     const MatchFinder::MatchResult &Result) {
   const auto *InitExpr = Result.Nodes.getNodeAs<Expr>("expr");
   const auto *Enum = Result.Nodes.getNodeAs<EnumDecl>("enum");
+  if (!InitExpr) {
+    const auto *InitList =
+        Result.Nodes.getNodeAs<InitListExpr>("array_filler_expr");
+    // Initialization of omitted array elements with array filler was found.
+    // Check the type for enum without zero value.
+    FindEnumMember Finder;
+    if (!Finder.Visit(InitList->getArrayFiller()->getType().getTypePtr()))
+      return;
+    InitExpr = InitList;
+    Enum = Finder.FoundEnum->getDecl();
+  }
+
   if (!InitExpr || !Enum)
     return;
 
@@ -99,7 +167,8 @@ void InvalidEnumDefaultInitializationCheck::check(
       }
     }
     // If still not found a source location, omit the warning.
-    // FIXME: All such cases should be fixed to make the checker more precise.
+    // Ideally all such cases (if they exist) should be handled to make the
+    // check more precise.
     if (Loc.isInvalid())
       return;
   }

clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.h

@@ -24,7 +24,6 @@ class InvalidEnumDefaultInitializationCheck : public ClangTidyCheck {
                                         ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
-  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override;
 };
 
 } // namespace clang::tidy::bugprone

clang-tools-extra/docs/clang-tidy/checks/bugprone/invalid-enum-default-initialization.rst

@@ -16,7 +16,8 @@ enumerator values).
 
 The checker emits a warning only if an enum variable is default-initialized
 (contrary to not initialized) and the enum type does not have an enumerator with
-value of 0. The enum type can be scoped or non-scoped enum.
+value of 0. The enum type can be scoped or non-scoped enum. Unions are not
+handled by the check (if it contains a member of enum type).
 
 .. code-block:: c++
 
@@ -48,3 +49,24 @@ value of 0. The enum type can be scoped or non-scoped enum.
   };
 
   S2 VarS2{}; // warn: member 'B' is initialized to 0
+
+The check applies to initialization of arrays or structures with initialization
+lists in C code too. In these cases elements not specified in the list (and have
+enum type) are set to 0.
+
+.. code-block:: c
+
+  enum Enum1 {
+    Enum1_A = 1,
+    Enum1_B
+  };
+  struct Struct1 {
+    int a;
+    enum Enum1 b;
+  };
+
+  enum Enum1 Array1[2] = {Enum1_A}; // warn: omitted elements are initialized to 0
+  enum Enum1 Array2[2][2] = {{Enum1_A}, {Enum1_A}}; // warn: last element of both nested arrays is initialized to 0
+  enum Enum1 Array3[2][2] = {{Enum1_A, Enum1_A}}; // warn: elements of second array are initialized to 0
+
+  struct Struct1 S1 = {1}; // warn: element 'b' is initialized to 0

clang-tools-extra/test/clang-tidy/checkers/bugprone/invalid-enum-default-initialization.c

@@ -0,0 +1,54 @@
+// RUN: %check_clang_tidy %s bugprone-invalid-enum-default-initialization %t
+
+enum Enum1 {
+  Enum1_A = 1,
+  Enum1_B
+};
+
+struct Struct1 {
+  int a;
+  enum Enum1 b;
+};
+
+struct Struct2 {
+  struct Struct1 a;
+  char b;
+};
+
+enum Enum1 E1 = {};
+// CHECK-NOTES: :[[@LINE-1]]:17: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
+// CHECK-NOTES: :3:6: note: enum is defined here
+enum Enum1 E2[10] = {};
+// CHECK-NOTES: :[[@LINE-1]]:21: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
+// CHECK-NOTES: :3:6: note: enum is defined here
+enum Enum1 E3[10] = {Enum1_A};
+// CHECK-NOTES: :[[@LINE-1]]:21: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
+// CHECK-NOTES: :3:6: note: enum is defined here
+enum Enum1 E4[2][2] = {{Enum1_A}, {Enum1_A}};
+// CHECK-NOTES: :[[@LINE-1]]:24: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
+// CHECK-NOTES: :3:6: note: enum is defined here
+// CHECK-NOTES: :[[@LINE-3]]:35: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
+// CHECK-NOTES: :3:6: note: enum is defined here
+enum Enum1 E5[2][2] = {{Enum1_A, Enum1_A}};
+// CHECK-NOTES: :[[@LINE-1]]:23: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
+// CHECK-NOTES: :3:6: note: enum is defined here
+
+
+struct Struct1 S1[2][2] = {{{1, Enum1_A}, {2, Enum1_A}}};
+// CHECK-NOTES: :[[@LINE-1]]:27: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
+// CHECK-NOTES: :3:6: note: enum is defined here
+
+struct Struct2 S2[3] = {{1}};
+// CHECK-NOTES: :[[@LINE-1]]:24: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
+// CHECK-NOTES: :3:6: note: enum is defined here
+// CHECK-NOTES: :[[@LINE-3]]:26: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
+// CHECK-NOTES: :3:6: note: enum is defined here
+
+union Union1 {
+  enum Enum1 a;
+  int b;
+};
+
+// no warnings for union
+union Union1 U1 = {};
+union Union1 U2[3] = {};

clang-tools-extra/test/clang-tidy/checkers/bugprone/invalid-enum-default-initialization.cpp

@@ -57,9 +57,13 @@ void f2() {
   // CHECK-NOTES: :[[@LINE-1]]:13: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
   // CHECK-NOTES: :8:12: note: enum is defined here
   Enum1 C[5] = {{}};
-  // CHECK-NOTES: :[[@LINE-1]]:17: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
+  // CHECK-NOTES: :[[@LINE-1]]:16: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
+  // CHECK-NOTES: :8:12: note: enum is defined here
+  // CHECK-NOTES: :[[@LINE-3]]:17: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
   // CHECK-NOTES: :8:12: note: enum is defined here
   Enum1 D[5] = {}; // FIMXE: warn for this?
+  // CHECK-NOTES: :[[@LINE-1]]:16: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator
+  // CHECK-NOTES: :8:12: note: enum is defined here
 }
 
 struct S1 {
@@ -99,6 +103,11 @@ struct S4 : public S3 {
   int Z;
 };
 
+struct S5 {
+  S2 X[3];
+  int Y;
+};
+
 S2 VarS2{};
 // CHECK-NOTES: :[[@LINE-1]]:9: warning: enum value of type 'Enum1' initialized with invalid value of 0
 // CHECK-NOTES: :8:12: note: enum is defined here
@@ -114,6 +123,9 @@ S4 VarS4{};
 // CHECK-NOTES: :8:12: note: enum is defined here
 // CHECK-NOTES: :[[@LINE-3]]:10: warning: enum value of type 'Enum2' initialized with invalid value of 0
 // CHECK-NOTES: :13:6: note: enum is defined here
+S5 VarS5{};
+// CHECK-NOTES: :[[@LINE-1]]:10: warning: enum value of type 'Enum1' initialized with invalid value of 0
+// CHECK-NOTES: :8:12: note: enum is defined here
 
 enum class EnumFwd;