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

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/Hover.cpp:64
 
+static llvm::StringRef getNameForObjCInterface(const ObjCInterfaceDecl *ID) {
+  return ID ? ID->getName() : "<<error-type>>";
----------------
dgoldman wrote:
> sammccall wrote:
> > this function's name doesn't really describe what it's for
> > 
> > it has 3 callsites and is only needed in 2, just inline it?
> Worth keeping, see below, Open to naming suggestions =)
nameOrError?


================
Comment at: clang-tools-extra/clangd/Hover.cpp:71
+  llvm::raw_string_ostream OS(Name);
+  const ObjCInterfaceDecl *Class = Method->getClassInterface();
+
----------------
dgoldman wrote:
> sammccall wrote:
> > looking at the implementation, this is null iff the method is part of a protocol.
> > <<error-type>> doesn't seem appropriatefor that case
> It can also be NULL for invalid code (in category).
> 
> Since we only call this for Decls in this decl context protocols can't ever appear as protocols can't have method definitions. Do you think it's still worth handling?
ah, I see...

yes I think the protocol case is worth handling in order to give this a simple signature in AST.h (prettyprint an ObjC method, without any surprising caveats)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68590