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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
