[clang-tools-extra] [include-cleaner] Report refs for enum constants used through namespace aliases (PR #106706)

Ilya Biryukov via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 2 05:36:37 PDT 2024


================
@@ -146,9 +148,24 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> {
     //
     // If it's an enum constant, it must be due to prior decl. Report references
     // to it when qualifier isn't a type.
-    if (llvm::isa<EnumConstantDecl>(FD)) {
-      if (!DRE->getQualifier() || DRE->getQualifier()->getAsNamespace())
-        report(DRE->getLocation(), FD);
+    auto QualifierIsNamepsaceOrNone = [&DRE]() {
+      const auto *Qual = DRE->getQualifier();
+      if (!Qual)
+        return true;
+      switch (Qual->getKind()) {
+      case NestedNameSpecifier::Namespace:
+      case NestedNameSpecifier::NamespaceAlias:
+      case NestedNameSpecifier::Global:
+        return true;
+      case NestedNameSpecifier::TypeSpec:
+      case NestedNameSpecifier::TypeSpecWithTemplate:
+      case NestedNameSpecifier::Super:
+      case NestedNameSpecifier::Identifier:
+        return false;
+      }
----------------
ilya-biryukov wrote:

Yeah, we don't use `-Werror`, though, and don't use the same compiler.
So missing the error during development is not unheard of, and I don't think our precommit CI catches that either.

This is why `llvm_unreachable` still makes sense from my perspective, I always put it as an extra layer of defense.

As for `default: ... `, we have an explicit [rule in LLVM Style Guide](https://llvm.org/docs/CodingStandards.html#don-t-use-default-labels-in-fully-covered-switches-over-enumerations) that asks not to do that for fully-covered switches.

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


More information about the cfe-commits mailing list