[cfe-commits] r165771 - in /cfe/trunk: include/clang/AST/ASTContext.h include/clang/AST/Comment.h lib/AST/ASTContext.cpp lib/AST/Comment.cpp lib/AST/CommentSema.cpp test/Index/overriding-method-comments.mm tools/libclang/CXComment.cpp

Dmitri Gribenko gribozavr at gmail.com
Thu Oct 18 05:03:40 PDT 2012

On Fri, Oct 12, 2012 at 2:52 AM, Fariborz Jahanian <fjahanian at apple.com> wrote:
> +comments::FullComment *ASTContext::cloneFullComment(comments::FullComment *FC,
> +                                                    const Decl *D) const {
> +  comments::DeclInfo *ThisDeclInfo = new (*this) comments::DeclInfo;
> +  ThisDeclInfo->CommentDecl = D;
> +  ThisDeclInfo->IsFilled = false;
> +  ThisDeclInfo->fill();
> +  ThisDeclInfo->CommentDecl = FC->getDecl();
> +  comments::FullComment *CFC =
> +    new (*this) comments::FullComment(FC->getBlocks(),
> +                                      ThisDeclInfo);
> +  return CFC;
> +
> +}

Hi Fariborz,

I think that cloneFullComment() is evil -- it breaks some assumptions
about DeclInfo and does not properly introduce a concept of a comment
being reused for a redeclaration.

(1) DeclInfo is intended to be immutable once filled; here we are just
replacing the original decl that provided all the information with
some other declaration.  I see that it apparently did not break
anything, but it is not the best solution.  Should we better keep two
DeclInfos per FullComment?

(2) ParamCommandComment::getParamName and others used to return the
parameter name *as written* in \param command, without any regard to
resolving the name to an index in function parameter list.  Now they
serve dual purpose of returning that parameter name or rewriting it.
This also breaks the purpose of getParamNameRange() nearby.  I think
that getParamName should be split into two getter functions -- to get
parameter name as written (getParamName() as it was) or with some
magic rewriting (getParamNameInDecl()).


(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/

More information about the cfe-commits mailing list