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; }
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; }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits