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

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.


================
Comment at: clang-tools-extra/clangd/index/Ref.h:36
+  // one below.
+  Implicit = 1 << 3,
+  All = Declaration | Definition | Reference | Implicit,
----------------
kadircet wrote:
> kadircet wrote:
> > instead of doing that, could we rather de-couple two enums completely and have a `symbolRoleToRefKind`(and vice-versa) method instead(we already have some customization in `toRefKind` now) ?
> > 
> > as current change increases the risk of overlapping in future(e.g. someone might change symbolrole::declaration and cause failures/regressions in clangd)
> note that this would also require a bump to version of `on-disk` index in clangd/index/Serialization.cpp, as old information regarding `RefKind` won't be usable anymore.
> instead of doing that, could we rather de-couple two enums completely and have a symbolRoleToRefKind(and vice-versa) method instead(we already have some customization in toRefKind now) ?
> as current change increases the risk of overlapping in future(e.g. someone might change symbolrole::declaration and cause failures/regressions in clangd)

Makes sense, will do!

> note that this would also require a bump to version of on-disk index in clangd/index/Serialization.cpp, as old information regarding RefKind won't be usable anymore.

I checked the Serialization code and the serialization code should be OK as long as `RefKind` stays one byte. Can you please elaborate on this?

Do you mean that the index should be generated again after this change because it would no longer be valid? (this one I understand)
Or do you mean there should be some other change in the code for me to do to land this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72746