[clang-tools-extra] [clang-tidy] Switch misc-confusable-identifiers check to a faster algorithm. (PR #130369)

[Richard Smith via cfe-commits]

================
@@ -181,42 +160,105 @@ void ConfusableIdentifierCheck::check(
   if (!ND)
     return;
 
-  IdentifierInfo *NDII = ND->getIdentifier();
+  addDeclToCheck(ND, cast<Decl>(ND->getDeclContext()
+                                    ->getNonTransparentContext()));
+
+  // Associate template parameters with this declaration of this template.
+  if (const auto *TD = dyn_cast<TemplateDecl>(ND)) {
+    for (const NamedDecl *Param : *TD->getTemplateParameters())
+      addDeclToCheck(Param, TD->getTemplatedDecl());
+  }
+
+  // Associate function parameters with this declaration of this function.
+  if (const auto *FD = dyn_cast<FunctionDecl>(ND)) {
+    for (const NamedDecl *Param : FD->parameters())
+      addDeclToCheck(Param, ND);
+  }
+}
+
+void ConfusableIdentifierCheck::addDeclToCheck(const NamedDecl *ND,
+                                               const Decl *Parent) {
+  const IdentifierInfo *NDII = ND->getIdentifier();
   if (!NDII)
     return;
 
   StringRef NDName = NDII->getName();
   if (NDName.empty())
     return;
 
-  const ContextInfo *Info = getContextInfo(ND->getDeclContext());
+  NameToDecls[NDII].push_back({ND, Parent});
+}
+
+void ConfusableIdentifierCheck::onEndOfTranslationUnit() {
+  llvm::StringMap<llvm::SmallVector<const IdentifierInfo *, 1>> SkeletonToNames;
+  // Compute the skeleton for each identifier.
+  for (auto &[Ident, Decls] : NameToDecls) {
+    SkeletonToNames[skeleton(Ident->getName())].push_back(Ident);
+  }
 
-  llvm::SmallVector<Entry> &Mapped = Mapper[skeleton(NDName)];
-  for (const Entry &E : Mapped) {
-    if (!mayShadow(ND, Info, E.Declaration, E.Info))
+  // Visit each skeleton with more than one identifier.
+  for (auto &[Skel, Idents] : SkeletonToNames) {
+    if (Idents.size() < 2) {
       continue;
+    }
 
-    const IdentifierInfo *ONDII = E.Declaration->getIdentifier();
-    StringRef ONDName = ONDII->getName();
-    if (ONDName == NDName)
-      continue;
+    // Find the declaration contexts that transitively contain each identifier.
+    DeclsWithinContextMap DeclsWithinContext;
----------------
zygoloid wrote:

I ran this a couple of times both ways, and the difference seems to be in the noise (in fact, the clang-tidy profiler found that both runs without this change were faster than the ones with it, but they're close enough that I don't think it's a real difference). It might be that the extra allocations are actually faster than keeping the high-water-mark and performing extra work clearing out a larger bucket array each time? Or maybe we just don't reach this loop enough times for it to be interesting.

I also tried using `SmallDenseMap` but that also seemed in the noise (slightly increased runtime).

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