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