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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
