[clang-tools-extra] [clang-tidy] Add new check bugprone-tagged-union-member-count (PR #89925)

via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 24 14:51:29 PDT 2024


================
@@ -0,0 +1,125 @@
+//===--- TaggedUnionMemberCountCheck.cpp - clang-tidy ---------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "TaggedUnionMemberCountCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Expr.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+#include "clang/AST/PrettyPrinter.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Casting.h"
+#include <limits>
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+void TaggedUnionMemberCountCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      recordDecl(
+          allOf(isStruct(),
+                has(fieldDecl(hasType(recordDecl(isUnion()).bind("union")))),
+                has(fieldDecl(hasType(enumDecl().bind("tags"))))))
+          .bind("root"),
+      this);
+}
+
+static bool hasMultipleUnionsOrEnums(const RecordDecl *rec) {
+  int tags = 0;
+  int unions = 0;
+  for (const FieldDecl *r : rec->fields()) {
+    TypeSourceInfo *info = r->getTypeSourceInfo();
+    QualType qualtype = info->getType();
+    const Type *type = qualtype.getTypePtr();
+    if (type->isUnionType())
+      unions += 1;
+    else if (type->isEnumeralType())
+      tags += 1;
+    if (tags > 1 || unions > 1)
+      return true;
+  }
+  return false;
+}
+
+static int64_t getNumberOfValidEnumValues(const EnumDecl *ed) {
+  int64_t maxTagValue = std::numeric_limits<int64_t>::min();
+  int64_t minTagValue = std::numeric_limits<int64_t>::max();
+
+  // Heuristic for counter enum constants.
+  //
+  //   enum tag_with_counter {
+  //     tag1,
+  //     tag2,
+  //     tag_count, <-- Searching for these enum constants
+  //   };
+  //
+  // The 'ce' prefix is used to abbreviate counterEnum.
+  // The final tag count is decreased by 1 if and only if:
+  // 1. The number of counting enum constants = 1,
+  int ceCount = 0;
+  // 2. The counting enum constant is the last enum constant that is defined,
+  int ceFirstIndex = 0;
+  // 3. The value of the counting enum constant is the largest out of every enum constant.
+  int64_t ceValue = 0;
+
+  int64_t enumConstantsCount = 0;
+  for (auto En : llvm::enumerate(ed->enumerators())) {
+    enumConstantsCount += 1;
+
+    int64_t enumValue = En.value()->getInitVal().getExtValue();
+    StringRef enumName = En.value()->getName();
+
+    if (enumValue > maxTagValue)
+      maxTagValue = enumValue;
+    if (enumValue < minTagValue)
+      minTagValue = enumValue;
+
+    if (enumName.ends_with_insensitive("count")) {
+      if (ceCount == 0) {
+        ceFirstIndex = En.index();
+      }
+      ceValue = enumValue;
+      ceCount += 1;
+    }
+  }
+
+  int64_t validValuesCount = maxTagValue - minTagValue + 1;
+  if (ceCount == 1 &&
+      ceFirstIndex == enumConstantsCount - 1 &&
+      ceValue == maxTagValue) {
+    validValuesCount -= 1;
+  }
+  return validValuesCount;
----------------
isuckatcs wrote:

I appreciate the effort you put into this, but what this heuristics tries to detect is nearly impossible to detect algorithmically. To detect this, we would need to know what the developer, who implemented the tagged union was thinking, how they named enumerators, etc. 

Also currently this heuristics will only work in a very specific scenario, when the last enumerator is called `count` and even in the default case the result can be misleading.

E.g.:
```c++
enum Tags {
  INT = (1 << 0),
  FLOAT = (1 << 1),
  CLASS_A = (1 << 2),
};
```

`validValuesCount` will be `(1 << 2) - (1 << 0) + 1 = 4 - 1 + 1 = 4`, right? I think you want to decrease and return the number of enumerators here.

https://github.com/llvm/llvm-project/pull/89925


More information about the cfe-commits mailing list