On Oct 10, 2012, at 11:34 AM, Fariborz Jahanian <[email protected]> wrote:
> Author: fjahanian > Date: Wed Oct 10 13:34:52 2012 > New Revision: 165643 > > URL: http://llvm.org/viewvc/llvm-project?rev=165643&view=rev > Log: > [Doc parsing] This patch searches overridden objc/c++ > methods looking for documentation on a particular base > class inherited by any method that overrides the base class. > In case of redeclaration, as when objc method is defined > in the implementation, it also looks up for documentation > in class/class extension being redeclared. > > Added: > cfe/trunk/test/Index/overriding-method-comments.mm > Modified: > cfe/trunk/include/clang/AST/Comment.h > cfe/trunk/include/clang/AST/DeclObjC.h > cfe/trunk/lib/AST/ASTContext.cpp > cfe/trunk/lib/AST/Comment.cpp > cfe/trunk/lib/AST/CommentDumper.cpp > cfe/trunk/lib/AST/CommentSema.cpp > cfe/trunk/lib/AST/DeclObjC.cpp > cfe/trunk/tools/libclang/CXComment.cpp > cfe/trunk/unittests/AST/CommentParser.cpp > > Modified: cfe/trunk/include/clang/AST/Comment.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Comment.h?rev=165643&r1=165642&r2=165643&view=diff > ============================================================================== > --- cfe/trunk/include/clang/AST/Comment.h (original) > +++ cfe/trunk/include/clang/AST/Comment.h Wed Oct 10 13:34:52 2012 > @@ -17,6 +17,7 @@ > #include "clang/Basic/SourceLocation.h" > #include "clang/AST/Type.h" > #include "clang/AST/CommentCommandTraits.h" > +#include "clang/AST/DeclObjC.h" > #include "llvm/ADT/ArrayRef.h" > #include "llvm/ADT/StringRef.h" > > @@ -727,7 +728,17 @@ > return getNumArgs() > 0; > } > > - StringRef getParamName() const { > + StringRef getParamName(const Decl *OverridingDecl) const { How about passing the FullComment down, since that's what the user actually has to consider? Also, it will help with my next comment… > + if (OverridingDecl && isParamIndexValid()) { > + if (const ObjCMethodDecl *OMD = > dyn_cast<ObjCMethodDecl>(OverridingDecl)) { > + const ParmVarDecl *ParamDecl = OMD->param_begin()[getParamIndex()]; > + return ParamDecl->getName(); > + } > + else if (const FunctionDecl *FD = > dyn_cast<FunctionDecl>(OverridingDecl)) { > + const ParmVarDecl *ParamDecl = FD->param_begin()[getParamIndex()]; > + return ParamDecl->getName(); > + } > + } The parameters are actually already stored in the FullComment's DeclInfo. I suggest going there instead, so we don't have to match the various forms of OverridingDecl. This would also let you fix the redeclaration case fairly easily, e.g., /// \brief Adds two integers /// \param lhs The left-hand side /// \param rhs The right-hand side /// \returns the sum of \p lhs and \p rhs int add(int lhs, int rhs); int add(int x, int y); // comment block for this will refer to parameters 'lhs' and 'rhs', but should refer to x and y and even perhaps for typedefs (!) /// \brief Adds two integers /// \param lhs The left-hand side /// \param rhs The right-hand side /// \returns the sum of \p lhs and \p rhs typedef int adder(int lhs, int rhs); typedef int adder(int x, int y); > @@ -936,22 +947,22 @@ > /// Information about the declaration, useful to clients of FullComment. > struct DeclInfo { > /// Declaration the comment is attached to. Should not be NULL. > - const Decl *ThisDecl; > - > - /// Parameters that can be referenced by \\param if \c ThisDecl is > something > + const Decl *CommentDecl; > + These are great, thanks! > Modified: cfe/trunk/lib/AST/ASTContext.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=165643&r1=165642&r2=165643&view=diff > ============================================================================== > --- cfe/trunk/lib/AST/ASTContext.cpp (original) > +++ cfe/trunk/lib/AST/ASTContext.cpp Wed Oct 10 13:34:52 2012 > @@ -24,6 +24,7 @@ > #include "clang/AST/ASTMutationListener.h" > #include "clang/AST/RecordLayout.h" > #include "clang/AST/Mangle.h" > +#include "clang/AST/Comment.h" > #include "clang/Basic/Builtins.h" > #include "clang/Basic/SourceManager.h" > #include "clang/Basic/TargetInfo.h" > @@ -355,21 +356,72 @@ > return RC; > } > > +static void addRedeclaredMethods(const ObjCMethodDecl *ObjCMethod, > + SmallVectorImpl<const NamedDecl *> &Redeclared) { > + const DeclContext *DC = ObjCMethod->getDeclContext(); > + if (const ObjCImplDecl *IMD = dyn_cast<ObjCImplDecl>(DC)) { > + const ObjCInterfaceDecl *ID = IMD->getClassInterface(); > + if (!ID) > + return; > + // Add redeclared method here. > + for (const ObjCCategoryDecl *ClsExtDecl = ID->getFirstClassExtension(); > + ClsExtDecl; ClsExtDecl = ClsExtDecl->getNextClassExtension()) { > + if (ObjCMethodDecl *RedeclaredMethod = > + ClsExtDecl->getMethod(ObjCMethod->getSelector(), > + ObjCMethod->isInstanceMethod())) > + Redeclared.push_back(RedeclaredMethod); > + } > + } > +} > + Isn't there some pre-existing function you can re-use to find the redeclared methods? It seems that Sema must do this already. > comments::FullComment *ASTContext::getCommentForDecl( > const Decl *D, > const Preprocessor *PP) const { > D = adjustDeclToTemplate(D); > + > const Decl *Canonical = D->getCanonicalDecl(); > llvm::DenseMap<const Decl *, comments::FullComment *>::iterator Pos = > ParsedComments.find(Canonical); > - if (Pos != ParsedComments.end()) > + > + if (Pos != ParsedComments.end()) { > + if (Canonical != D && > + (isa<ObjCMethodDecl>(D) || isa<FunctionDecl>(D))) { Why do we need the isa<> checks here? This seems like a fairly general approach to having the CommentDecl and the ThisDecl be different. Heck, we might want it for, e.g., /// \brief A wonderful comment @interface A @end @class A; // should be able to get docs here! @implementation A // and here! @end > + // case of method being redeclaration of the canonical, not > + // overriding it; i.e. method in implementation, canonical in > + // interface. Or, out-of-line cxx-method definition. > + comments::FullComment *FC = Pos->second; > + comments::FullComment *CFC = > + new (*this) comments::FullComment(FC->getBlocks(), > + FC->getThisDeclInfo(), > + const_cast<Decl *>(D)); I think you want to create a new DeclInfo for the cloned FullComment, based on the actual declaration (ThisDecl) rather than the comment declaration (CommentDecl). Obviously, it's CommentDecl will be the same as the original FullComment's CommentDecl. > + return CFC; > + } > return Pos->second; > - > + } > + > const Decl *OriginalDecl; > + > const RawComment *RC = getRawCommentForAnyRedecl(D, &OriginalDecl); > - if (!RC) > + if (!RC) { > + if (isa<ObjCMethodDecl>(D) || isa<FunctionDecl>(D)) { > + SmallVector<const NamedDecl*, 8> overridden; > + if (const ObjCMethodDecl *OMD = dyn_cast<ObjCMethodDecl>(D)) > + addRedeclaredMethods(OMD, overridden); > + const_cast<ASTContext > *>(this)->getOverriddenMethods(dyn_cast<NamedDecl>(D), > + overridden); > + for (unsigned i = 0, e = overridden.size(); i < e; i++) { > + if (comments::FullComment *FC = getCommentForDecl(overridden[i], > PP)) { > + comments::FullComment *CFC = > + new (*this) comments::FullComment(FC->getBlocks(), > + FC->getThisDeclInfo(), > + const_cast<Decl *>(D)); > + return CFC; > + } > + } > + } > return NULL; This comment-cloning code is duplicated; please pull it out into a helper routine. I suspect, based on my @class/@interface/@implementation example above, that we'll want to have a few other cases here for "search for a comment on a related declaration". > > Modified: cfe/trunk/lib/AST/CommentDumper.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/CommentDumper.cpp?rev=165643&r1=165642&r2=165643&view=diff > ============================================================================== > --- cfe/trunk/lib/AST/CommentDumper.cpp (original) > +++ cfe/trunk/lib/AST/CommentDumper.cpp Wed Oct 10 13:34:52 2012 > @@ -183,7 +183,7 @@ > OS << " implicitly"; > > if (C->hasParamName()) > - OS << " Param=\"" << C->getParamName() << "\""; > + OS << " Param=\"" << C->getParamName(0) << "\""; We should be able to pass down the FullComment here, right? > Added: cfe/trunk/test/Index/overriding-method-comments.mm > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/overriding-method-comments.mm?rev=165643&view=auto > ============================================================================== > --- cfe/trunk/test/Index/overriding-method-comments.mm (added) > +++ cfe/trunk/test/Index/overriding-method-comments.mm Wed Oct 10 13:34:52 > 2012 > @@ -0,0 +1,111 @@ > +// RUN: rm -rf %t > +// RUN: mkdir %t > +// RUN: c-index-test -test-load-source all > -comments-xml-schema=%S/../../bindings/xml/comment-xml-schema.rng %s > %t/out > +// RUN: FileCheck %s < %t/out Nice, thanks! > +// rdar://12378793 A comment about what this tests would be more useful than a radar number, since not everyone has access to radar. - Doug _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
