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

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 10 15:10:11 PST 2022


sammccall created this revision.
sammccall added a reviewer: usaxena95.
Herald added subscribers: kadircet, arphaman.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116978

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


Index: clang-tools-extra/clangd/Selection.cpp
===================================================================
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -313,12 +313,19 @@
   bool mayHit(SourceRange R) const {
     if (SpelledTokens.empty())
       return false;
-    auto B = SM.getDecomposedLoc(R.getBegin());
-    auto E = SM.getDecomposedLoc(R.getEnd());
-    if (B.first == SelFile && E.first == SelFile)
-      if (E.second < SpelledTokens.front().Offset ||
-          B.second > SpelledTokens.back().Offset)
+    // If the node ends before the selection begins, it is not selected.
+    if (R.getEnd().isFileID()) {
+      auto E = SM.getDecomposedLoc(R.getEnd());
+      if (E.first == SelFile && E.second < SpelledTokens.front().Offset)
         return false;
+    }
+    // If the node starts after the selection ends, it is not selected.
+    // All tokens a macro location might claim are >= its expansion site.
+    // So it's safe to use the expansions location for the comparison.
+    // (This is particularly helpful for GTest's TEST macro).
+    auto B = SM.getDecomposedExpansionLoc(R.getBegin());
+    if (B.first == SelFile && B.second > SpelledTokens.back().Offset)
+      return false;
     return true;
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D116978.398755.patch
Type: text/x-patch
Size: 1284 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220110/f35919a7/attachment-0001.bin>


More information about the cfe-commits mailing list