[patch] Ignore KeepEmptyLinesAtTheStartOfBlocks for extern "C" blocks

Daniel Jasper djasper at google.com
Wed Nov 12 23:37:46 PST 2014


Maybe s/isExternCBlock/startsExternCBlock/.

Otherwise looks good. Thanks!

On Wed, Nov 12, 2014 at 11:46 PM, Nico Weber <thakis at chromium.org> wrote:

> Now uses getNextNonComment().
>
> On Wed, Nov 12, 2014 at 2:26 PM, Nico Weber <thakis at chromium.org> wrote:
>
>> On Wed, Nov 12, 2014 at 1:46 PM, Daniel Jasper <djasper at google.com>
>> wrote:
>>
>>> Grml... Really prefer reviews on phabricator ...
>>>
>>
>> (I try to set up the command-line tool every now and then, it always ends
>> up being a huge mess. And using the web interface takes 10x as long as just
>> emailing the output of `svn diff` – I need to figure out how to convince
>> svn how to include more context, click through a a bunch of web forms, etc.
>> Since it's a small patch, I figured it's ok. If uploading to phab was
>> easier, I'd use it more.)
>>
>>
>>>
>>> >+    } else if (Limit != 0 && Line.First->isNot(tok::kw_namespace) &&
>>> >+               (Line.First->isNot(tok::kw_extern) ||
>>> >+                (Line.First->Next &&
>>> !Line.First->Next->isStringLiteral()))) {
>>>
>>> Isn't this doing something entirely different, namely not contracting
>>> such declaration to a single line. I'd be fine with that, but it should be
>>> called out explicitly.. And fixes part of llvm.org/PR21419.
>>>
>>
>> Ah, true. I only did what my description claimed at first, and then my
>> testcase failed since it got formatted as a single line, so I figured this
>> should probably be handled the same way namespaces are too. I'll call it
>> out in the commit message.
>>
>>
>>> Also, this would prevent:
>>>
>>>   extern "C" int f() { return 42; }
>>>
>>> Right? Probably not what we want.
>>>
>>> >+        PreviousLine->First->isNot(tok::kw_namespace) &&
>>> >+        PreviousLine->First->isNot(tok::kw_extern))
>>>
>>> !PreviousLine->First->isOneOf(tok::kw_namespace, tok::kw_extern)
>>>
>>> Also, this prevents removing the empty line in
>>>
>>>   extern "C" int f() {
>>>
>>>     int i = 42;
>>>     return i;
>>>   }
>>>
>>> Also not what we want, I guess.
>>>
>>
>> Great catch! Updated patch attached.
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141113/ef01a1a6/attachment.html>


More information about the cfe-commits mailing list