[PATCH] D69511: [clangd] Do not report anonymous entities in findExplicitReferences

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.


================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:453
+      // and the clients are not prepared to handle that.
+      if (ND->getDeclName().isIdentifier() &&
+          !ND->getDeclName().getAsIdentifierInfo())
----------------
kadircet wrote:
> i believe this will break D68937, which relies on findExplicitReferences to even rename unnamed parameters.
That's true, we'll have to handle them separately.

It's a bit more code, but I think it's worth it: having a name location pointing to commas or closing parentheses is super unexpected, having some custom logic to name unnamed things is probably a bit more code, but it should be less surprising.
Sorry about that.


================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:454
+      if (ND->getDeclName().isIdentifier() &&
+          !ND->getDeclName().getAsIdentifierInfo())
+        return;
----------------
hokein wrote:
> nit: I think `if (!ND->getDeclName().getAsIdentifierInfo())` is enough, getAsIdentifierInfo returns null if the name is not an identifier.
Good point, I've also tried that, but ended up with the current version.

The proposed solution also removes references to constructors (and I presume various special names like `operator+`, etc.)
I'm not sure what to do with those, they pose a different problem: unlike anonymous names, they're actually spelled out in the source code.

However, having a single `SourceLocation` is definitely not enough to figure out the interesting source ranges, so we'll have to send more information about the name location to the clients...

I'm currently considering saving a `DeclarationNameInfo` in the `ReferenceLoc`, but I'll need to check whether that's actually possible in all cases, so decided to move this into a separate change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69511