[PATCH] D68590: [clangd] Improve hover support for Objective-C

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clangd/XRefs.cpp:461
+
+  OS << (Method->isInstanceMethod() ? '-' : '+')
+     << '[' << Class->getName();
----------------
it looks like `DeclPrinter::VisitObjCMethodDecl` already has a similar handling.
would it provide value if you printed class and category names in the general case? If so it might make sense to update the logic in `DeclPrinter`.
If not, it looks like printing `selector` via `getAsString` is not enough, there's some special handling in `DeclPrinter`. Maybe move it into a public
place and make use of the same logic also in here to be consistent?


================
Comment at: clangd/XRefs.cpp:470
+
+static std::string getNameForObjCContainer(const ObjCContainerDecl *C) {
+  if (const ObjCCategoryDecl *Category = dyn_cast<ObjCCategoryDecl>(C)) {
----------------
again there are printers in for `ObjCCategory{Impl}Decl`s that don't respect `TerseOutput` option in printing policies. It might be sensible to update them instead to print an output like what you propose in here, in the presence of `TerseOutput` option. WDYT?


================
Comment at: clangd/XRefs.cpp:509
+      DC = DC->getParent();
+      if (DC) {  // Skip over parent Objective-C container.
+        DC = DC->getParent();
----------------
instead of saying what you do, could you say why you do it. IIUC, it is because the relevant bits of the container is already printed in `getNameForObjCMethod` right?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68590