[clang-tools-extra] [clang-doc] Improve performance by adding a short circuit (PR #96809)

[Paul Kirth via cfe-commits]

================
@@ -34,13 +43,26 @@ template <typename T> bool MapASTVisitor::mapDecl(const T *D) {
   // If there is an error generating a USR for the decl, skip this decl.
   if (index::generateUSRForDecl(D, USR))
     return true;
+
+  // Prevent Visiting USR twice
+  {
+    std::lock_guard<llvm::sys::Mutex> Guard(USRVisitedGuard);
+    std::string Visited = USR.str().str();
+    if (USRVisited.count(Visited))
+      return true;
+    // We considered a USR to be visited only when its defined
+    if (IsDefinition)
+      USRVisited.insert(Visited);
----------------
ilovepi wrote:

> My logic was that definition of the USR is parsed last, so when the ASTVisitor visits the definition it would have already parsed every other USR that points to the same declaration. So we can safely short circuit, since every other fragments of USR would've been parsed already

Is it really parsed last in all cases? Isn't it possible to have multiple of these definitions, depending on the scope of the AST construct and the target options. For instance, what if some code is compiled w/ a particular `#define` enabled and that provides different documentation than was found previously, but is compiled elsewhere without that define? I can easily imagine that affecting header code, where documentation is likely to be.



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