[clang-tools-extra] 2b2dbe6 - [clangd] Selection: Prune gtest TEST()s earlier

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 13 04:58:49 PST 2022


Author: Sam McCall
Date: 2022-01-13T13:58:42+01:00
New Revision: 2b2dbe6126f8c016e0cbef45a6d22e1359ce39fd

URL: https://github.com/llvm/llvm-project/commit/2b2dbe6126f8c016e0cbef45a6d22e1359ce39fd
DIFF: https://github.com/llvm/llvm-project/commit/2b2dbe6126f8c016e0cbef45a6d22e1359ce39fd.diff

LOG: [clangd] Selection: Prune gtest TEST()s earlier

When searching for AST nodes that may overlap the selection, mayHit() was only
attempting to prune nodes whose begin/end are both in the main file.

While failing to prune never gives wrong results, it hurts performance.
In GTest unit-tests, `TEST()` macros at the top level declare classes.
These were never pruned and we traversed *every* such class for any selection.

We fix this by reasoning about what tokens such a node might claim.
They must lie within its ultimate macro expansion range, so if this doesn't
overlap with the selection, we can prune the node.

Differential Revision: https://reviews.llvm.org/D116978

Added: 
    

Modified: 
    clang-tools-extra/clangd/Selection.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/Selection.cpp b/clang-tools-extra/clangd/Selection.cpp
index bf9783d80cae..f2403e03d771 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -8,7 +8,6 @@
 
 #include "Selection.h"
 #include "AST.h"
-#include "SourceCode.h"
 #include "support/Logger.h"
 #include "support/Trace.h"
 #include "clang/AST/ASTTypeTraits.h"
@@ -303,10 +302,21 @@ class SelectionTester {
   bool mayHit(SourceRange R) const {
     if (SpelledTokens.empty())
       return false;
-    auto B = offsetInSelFile(R.getBegin());
-    auto E = offsetInSelFile(R.getEnd());
-    if (B && E)
-      if (*E < SpelledTokens.front().Offset || *B > SpelledTokens.back().Offset)
+    // If the node starts after the selection ends, it is not selected.
+    // Tokens a macro location might claim are >= its expansion start.
+    // So if the expansion start > last selected token, we can prune it.
+    // (This is particularly helpful for GTest's TEST macro).
+    if (auto B = offsetInSelFile(getExpansionStart(R.getBegin())))
+      if (*B > SpelledTokens.back().Offset)
+        return false;
+    // If the node ends before the selection begins, it is not selected.
+    SourceLocation EndLoc = R.getEnd();
+    while (EndLoc.isMacroID())
+      EndLoc = SM.getImmediateExpansionRange(EndLoc).getEnd();
+    // In the rare case that the expansion range is a char range, EndLoc is
+    // ~one token too far to the right. We may fail to prune, that's OK.
+    if (auto E = offsetInSelFile(EndLoc))
+      if (*E < SpelledTokens.front().Offset)
         return false;
     return true;
   }
@@ -401,9 +411,14 @@ class SelectionTester {
     return NoTokens;
   }
 
+  // Decomposes Loc and returns the offset if the file ID is SelFile.
   llvm::Optional<unsigned> offsetInSelFile(SourceLocation Loc) const {
+    // Decoding Loc with SM.getDecomposedLoc is relatively expensive.
+    // But SourceLocations for a file are numerically contiguous, so we
+    // can use cheap integer operations instead.
     if (Loc < SelFileBounds.getBegin() || Loc >= SelFileBounds.getEnd())
       return llvm::None;
+    // FIXME: subtracting getRawEncoding() is dubious, move this logic into SM.
     return Loc.getRawEncoding() - SelFileBounds.getBegin().getRawEncoding();
   }
 


        


More information about the cfe-commits mailing list