On Oct 18, 2012, at 4:42 AM, Dmitri Gribenko <[email protected]> wrote:
> On Thu, Oct 18, 2012 at 12:58 AM, Fariborz Jahanian <[email protected]> > wrote: >> Author: fjahanian >> Date: Wed Oct 17 16:58:03 2012 >> New Revision: 166130 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=166130&view=rev >> Log: >> [Doc parsing]: This patch adds <Declaration> tag to >> XML comment for declarations which pretty-prints >> declaration. I had to XFAIL one test annotate-comments.cpp. >> This test is currently unmaintainable as written. >> Dmitri G., can you see what we can do about this test. > > Hi Fariborz, > > While I agree that annotate-comments is hard to maintain, but it is > not just "a test", it is *the* test (it tests almost all aspects of > comments parsing), and thus I disagree with your approach of > committing a new feature and XFAILing this test -- I think we should > have discussed this. I fully agree with Dmitri. Revert, don't XFAIL, if a test is breaking. >> Modified: cfe/trunk/include/clang/AST/Comment.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Comment.h?rev=166130&r1=166129&r2=166130&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/AST/Comment.h (original) >> +++ cfe/trunk/include/clang/AST/Comment.h Wed Oct 17 16:58:03 2012 >> @@ -905,9 +905,9 @@ >> /// Declaration the comment is attached to. Should not be NULL. >> const Decl *CommentDecl; >> >> - /// Location of this declaration. Not necessarily same as location of >> - /// CommentDecl. >> - SourceLocation Loc; >> + /// CurrentDecl is the declaration for which comment is being put into an >> XML comment. >> + /// Not necessarily same as CommentDecl. >> + const Decl *CurrentDecl; > > I think that this "CurrentDecl" variable represents (or should > represent) some bigger concept than what is stated in the comment. It > should not be related with XML output at all. And, I don't see (in > current code) how it can be different from CommentDecl. I hope my other message explained it. However, the comment needs to be improved to describe this. Fariborz? - Doug _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
