[PATCH] D72746: [clangd] Add a flag for implicit references in the Index

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D72746#1821097 <https://reviews.llvm.org/D72746#1821097>, @kbobyrev wrote:

> The patch could be shorter and slightly less confusing if I preserved 1:1 `RefKind::Implicit` <-> `index::SymbolKind::Implicit` relation, but I thought we might want to keep `RefKind` being 1 byte so that we don't waste unnecessary memory.


I think it is time to create our own `Declaration`, `Definition` in our `RefKind` (rather than reusing the enums in libindex).

> Also, since we might want to start using `findExplicitReferences` (or something else we come up with) for indexing, it shouldn't be critical. Let me know if you think it's unnecessary.

Yeah, we may have a plan in the future, but I don't think switching to `findExplicitReferences` is trivial, there are a few things we need to figure out carefully, e.g. how to support macros, implicit references.



================
Comment at: clang-tools-extra/clangd/index/Ref.h:33
   Reference = static_cast<uint8_t>(index::SymbolRole::Reference),
-  All = Declaration | Definition | Reference,
+  // This corresponds to index::SymbolRole::Implicit. In order to save memory
+  // and keep RefKind occupying 1 byte, the original value is modified to the
----------------
I'm skeptical about definition of `index::SymbolRole::Implicit`.

I think the `implicit` we want here is references that are spelled/written as the same as the named decls, so it is more likely to align with `index::SymbolRole::NameReferences`.


================
Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:309
+  if (Name.isIdentifier() && Name.getAsString() != Spelling)
+    Roles |= static_cast<uint32_t>(index::SymbolRole::Implicit);
+
----------------
kadircet wrote:
> It seems like we are only checking for `syntactic` occurence of a symbol. Can we make use of `Spelled` Rather than `Implicit` (which has some semantic meaning) to make that clear?
+1, `Implicit` is ambiguous, maybe `Named`, `Explicit` (this reflects to the `findExplicitReferences`)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72746