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).
Dmitri
--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <[email protected]>*/
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
