dgoldman added inline comments.
================ Comment at: clang-tools-extra/clangd/AST.cpp:174 auto L = D.getLocation(); + // Use the start of the first selector fragment instead of the -/+ location. + if (const auto *MD = dyn_cast<ObjCMethodDecl>(&D)) ---------------- kadircet wrote: > can we do this in a separate change? as it has wider implications Sent out https://reviews.llvm.org/D130095, will rebase this on top once that's submitted. ================ Comment at: clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp:29 + + const char *DeclMarker = nullptr; + const char *DefMarker = nullptr; ---------------- kadircet wrote: > since you're always calling these `decl` or `def`, you can instead check > whether the annotation has any range named `def`/`decl` and expect those > accordingly rather than mentioning them one extra time inside the testcases. > i.e: > > case: > > ``` > void $def[[foo]]() {} > int bar() { > fo^o(); > } > ``` > > check: > ``` > Annotations Test(Case); > SymbolDetails Expected; > auto Def = Test.ranges("def"); > auto Decl = Test.ranges("decl"); > if (!Def.empty()) { > Expected.decl = Expected.def = makelocation ... > } else if (!Decl.empty()) { > Expected.decl = makelocation ... > } > ``` Thanks, although that won't work for the `// Multiple symbols returned - using overloaded function name` test case. Should I move that one out to to be handled separately? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130041/new/ https://reviews.llvm.org/D130041 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
