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

[David Goldman via Phabricator via cfe-commits]

dgoldman 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>>";
----------------
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 =)


================
Comment at: clang-tools-extra/clangd/Hover.cpp:71
+  llvm::raw_string_ostream OS(Name);
+  const ObjCInterfaceDecl *Class = Method->getClassInterface();
+
----------------
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?


================
Comment at: clang-tools-extra/clangd/Hover.cpp:96
+  }
+  if (const ObjCCategoryImplDecl *CI = dyn_cast<ObjCCategoryImplDecl>(C)) {
+    std::string Name;
----------------
sammccall wrote:
> can't you return objcContainerLocalScope(CI->getCategoryDecl()) ?
As noted in getCategoryDecl() the class interface can be NULL when working with invalid code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68590