Oh, I thought NextToken is the comment token... Now I'm thoroughly confused.
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
