[PATCH] D112712: [clangd] SelectionTree should prefer lexical declcontext

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 28 06:59:40 PDT 2021


sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang-tools-extra/clangd/Selection.cpp:893
 
 const DeclContext &SelectionTree::Node::getDeclContext() const {
   for (const Node *CurrentNode = this; CurrentNode != nullptr;
----------------
Maybe insert the word "lexical" into the doc comment for this function


================
Comment at: clang-tools-extra/clangd/unittests/SelectionTests.cpp:742
 
+TEST(SelectionTest, DeclContext) {
+  llvm::Annotations Test("namespace a { void $1^foo(); } void a::$2^foo();");
----------------
Nit: call the case DeclContextIsLexical to make the point clearer?


================
Comment at: clang-tools-extra/clangd/unittests/SelectionTests.cpp:751
+  {
+    auto ST = SelectionTree::createRight(AST.getASTContext(), AST.getTokens(),
+                                         Test.point("2"), Test.point("2"));
----------------
I'd probably use makeSelectionTree(Test.code(), AST) for brevity and only test the #2 case, but up to you


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112712/new/

https://reviews.llvm.org/D112712



More information about the cfe-commits mailing list