On Oct 18, 2012, at 9:56 AM, Dmitri Gribenko <[email protected]> wrote:
> On Thu, Oct 18, 2012 at 7:18 PM, Douglas Gregor <[email protected]> wrote: >> On Oct 18, 2012, at 5:03 AM, Dmitri Gribenko <[email protected]> wrote: >>> 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? >> >> It's weird to be overwriting CommentDecl in cloneFullComment(), but I don't >> think we need to have two DeclInfos per FullComment. The intent here is that >> CommentDecl is the declaration to which the comment was actually attached >> (in the source), and CurrentDecl would be the declaration with which the >> FullComment is associated. The information in the DeclInfo corresponds to >> CurrentDecl. > > I see now, thank you for the explanation. Maybe improving comments or > renaming CommentDecl/CurrentDecl could help to make this clear. > >> I'm fine with splitting it into two functions. In general in the Clang ASTs, >> we have "getFoo()" be the semantic property and we add a "getFooAsWritten()" >> for the syntax as written. I suggest we do that. > > I agree. > >> I think we should also deal with rewriting of \p. Yes, it means some lookup, >> but it'll give a more consistent experience. > > My main concern is not the additional lookup (it is even useful -- we > could catch more "bugs" in comments), but the fact that there are > unannotated references to parameters, and sometimes both annotated and > unannotated references in the same comment (some will get rewritten, > some will not). It's a very reasonable concern. Personally, I'm more interested in people being able to do the right thing (mark their parameters with \p), so they'll get proper documentation across all redeclarations/overrides even if the names have changed. - Doug _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
