[PATCH] Improved llvm-namespace-comment check.

Manuel Klimek klimek at google.com
Mon May 19 09:57:57 PDT 2014


On Mon, May 19, 2014 at 6:55 PM, Alexander Kornienko <alexfh at google.com>wrote:

>
>
>
> On Mon, May 19, 2014 at 6:54 PM, Alexander Kornienko <alexfh at google.com>wrote:
>
>>
>>
>>
>> On Mon, May 19, 2014 at 6:53 PM, Manuel Klimek <klimek at google.com> wrote:
>>
>>> On Mon, May 19, 2014 at 6:29 PM, Manuel Klimek <klimek at 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!


>
>
>>
>>
>>
>>>
>>>>
>>>> http://reviews.llvm.org/D3825
>>>>
>>>>
>>>>
>>
>
>
> --
> Alexander Kornienko | Software Engineer | alexfh at google.com | Google
> Germany, Munich
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140519/0e8e20d1/attachment.html>


More information about the cfe-commits mailing list