On Mon, May 19, 2014 at 7:04 PM, Manuel Klimek <kli...@google.com> wrote:
> Oh, I thought NextToken is the comment token... Now I'm thoroughly > confused. > Perhaps: // We only add a newline if the next token is not a comment token and it's on the same line as the closing rbrace. But then perhaps we can write the condition somewhat more simple? > > > On Mon, May 19, 2014 at 6:59 PM, Alexander Kornienko <ale...@google.com>wrote: > >> On Mon, May 19, 2014 at 6:57 PM, Manuel Klimek <kli...@google.com> wrote: >> >>> On Mon, May 19, 2014 at 6:55 PM, Alexander Kornienko >>> <ale...@google.com>wrote: >>> >>>> On Mon, May 19, 2014 at 6:54 PM, Alexander Kornienko <ale...@google.com >>>> > wrote: >>>> >>>>> On Mon, May 19, 2014 at 6:53 PM, Manuel Klimek <kli...@google.com>wrote: >>>>> >>>>>> On Mon, May 19, 2014 at 6:29 PM, Manuel Klimek <kli...@google.com>wrote: >>>>>> >>>>>>> lg >>>>>>> >>>>>>> ================ >>>>>>> Comment at: clang-tidy/llvm/NamespaceCommentCheck.cpp:66 >>>>>>> @@ +65,3 @@ >>>>>>> + Token Tok; >>>>>>> + while (Lexer::getRawToken(Loc, Tok, Sources, >>>>>>> Result.Context->getLangOpts())) { >>>>>>> + Loc = Loc.getLocWithOffset(1); >>>>>>> ---------------- >>>>>>> Can you add a comment explaining why it's ok to retry on failure >>>>>>> with an incremented location? >>>>>>> >>>>>>> ================ >>>>>>> Comment at: clang-tidy/llvm/NamespaceCommentCheck.cpp:73 >>>>>>> @@ +72,3 @@ >>>>>>> + bool NextTokenIsOnSameLine = Sources.getSpellingLineNumber(Loc) >>>>>>> == EndLine; >>>>>>> + bool NeedLineBreak = NextTokenIsOnSameLine && Tok.isNot(tok::eof); >>>>>>> + >>>>>>> ---------------- >>>>>>> Can you explain why we need this? (I'd have expected the fix-it to >>>>>>> not change the number of newlines, thus just inserting the comment >>>>>>> between >>>>>>> the } and the newline). >>>>>>> >>>>>> >>>>>> Am I missing where you added a comment for this? >>>>>> >>>>> >>>>> Should be here: >>>>> http://reviews.llvm.org/D3825?vs=9577&id=9579&whitespace=ignore-all#toc >>>>> >>>>> >>>> >>>> Ah, sorry, there's no comment for this. I've answered your question on >>>> the Phab only. >>>> Should I add a comment? >>>> >>> >>> Yes, please :) Thx! >>> >> >> >> Looking at this once again, I see that a comment would be tautological: >> // We need a line break when the next token is on the same line. >> bool NeedLineBreak = NextTokenIsOnSameLine && Tok.isNot(tok::eof); >> >> Are you sure it will make anything clearer? >> >> >
_______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits