[PATCH] D42895: [libclang] Add `CXSymbolRole role` to CXIdxEntityRefInfo

[Fangrui Song via Phabricator via cfe-commits]

MaskRay marked 2 inline comments as done.
MaskRay added inline comments.


================
Comment at: include/clang-c/Index.h:6159
+   */
+  CXSymbolRole role;
 } CXIdxEntityRefInfo;
----------------
ilya-biryukov wrote:
> Why do we need to store both `CXIdxEntityRefKind` and `CXSymbolRole`? Can we store just `CXSymbolRole`?
> Is this for compatibility with existing clients?
> 
> If so, maybe we could:
> - remove `Implicit` and `Direct` from the `CXSymbolRole`
> - keep it only in `CXIdxEntityRefKind`
> - not deprecate the `CXIdxEntityRefKind`
I think `CXIdxEntityRefKind` should be deprecated but for compatibility we do not remove the field for this minor version upgrade. But they can be removed after a major version upgrade.

For what I have observed, `Implicit` is only used by Objective-C


================
Comment at: tools/libclang/CXIndexDataConsumer.cpp:154
+  // CXSymbolRole is synchronized with clang::index::SymbolRole.
+  return CXSymbolRole(static_cast<uint32_t>(Role));
+}
----------------
ilya-biryukov wrote:
> `SymbolRoleSet` seems to have more roles not covered by `CXSymbolRole`.
> Should they be accepted by this function? 
> If yes, maybe zero those bits?
> If no, maybe add an assert?
> 
> 
> The extra roles are:
> ```
>   RelationChildOf     = 1 << 9,
>   RelationBaseOf      = 1 << 10,
>   RelationOverrideOf  = 1 << 11,
>   RelationReceivedBy  = 1 << 12,
>   RelationCalledBy    = 1 << 13,
>   RelationExtendedBy  = 1 << 14,
>   RelationAccessorOf  = 1 << 15,
>   RelationContainedBy = 1 << 16,
>   RelationIBTypeOf    = 1 << 17,
>   RelationSpecializationOf = 1 << 18,
> ```
Yes, it has more relations, but most are only used by Objective-C. In all test/Index tests, I have only seen `RelationContainedBy` used for .cpp files. Many have not occurred in .m files. So I do not want to expose them, at least for now.




Repository:
  rC Clang

https://reviews.llvm.org/D42895