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

[David Goldman via Phabricator via cfe-commits]

dgoldman marked 3 inline comments as done.
dgoldman added a comment.

Will revisit this once more critical fixes are in (crash fixes), I'm still not sure where this sort of stuff should belong



================
Comment at: clangd/XRefs.cpp:461
+
+  OS << (Method->isInstanceMethod() ? '-' : '+')
+     << '[' << Class->getName();
----------------
kadircet wrote:
> 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?
`DeclPrinter::VisitObjCMethodDecl` prints the declaration itself, e.g.:

`- (void)someMethod:(int)param;`

it wouldn't make sense to instead of that do

`-[MyClass someMethod:]` for the declaration itself


================
Comment at: clangd/XRefs.cpp:470
+
+static std::string getNameForObjCContainer(const ObjCContainerDecl *C) {
+  if (const ObjCCategoryDecl *Category = dyn_cast<ObjCCategoryDecl>(C)) {
----------------
kadircet wrote:
> 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?
What method names are you looking at? I wasn't able to find them


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68590