[PATCH] D68977: [clangd] Report declaration references in findExplicitReferences.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:415
 
 Optional<ReferenceLoc> refInDecl(const Decl *D) {
   struct Visitor : ConstDeclVisitor<Visitor> {
----------------
ilya-biryukov wrote:
> This should return `SmallVector<ReferenceLoc, 2>` now, some declarations can have both decl and non-decl references.
Can you give some examples? It seems that non-decl references are handled by other `refIn*` functions.

```
int Foo = OtherBar; // OtherBar is captured at the momment.
```


================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:444
+
+    void VisitDeclaratorDecl(const DeclaratorDecl *ND) {
+      Ref =
----------------
ilya-biryukov wrote:
> This should be handled in `VisitNamedDecl`, we should use a helper similar to `getQualifier` from `AST.h` to get the qualifier loc instead.
good point, didn't aware of this utility method, exposed it in `AST.h`.


================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:756
+  if (R.IsDecl)
+    OS << ", decl ref";
   return OS;
----------------
ilya-biryukov wrote:
> NIT: maybe use `declaration` instead?
> `ref` is definitely redundant, we know we're printing a reference.
changed to `decl`.


================
Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:528
+  /// AST.
+  AllRefs annotateReferencesInMainAST(llvm::StringRef Code) {
+    TestTU TU;
----------------
ilya-biryukov wrote:
> Why do we need this? Can't we test everything we do by putting into `foo`?
> 
> The original idea was to avoid too much output in the tests by letting the setup code live outside the function that's being annotated.
> Any reason why we have to do the full file here?
> 
> I know it's a weird limitation, but that's one that was already made and I'd keep it this way if it's possible.
I tried it but failed.

We can't test the qualified definitions (see example below) if we put everything into function `foo` (as defining a namespace is not allowed inside a function)

```
namespace $0^ns {
class $1^Foo {
 void $2^method();
};
void $3^func();
}
void $4^ns::$5^Foo::$6^method() {}
void $7^ns::$8^func() {}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68977