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