[patch] Ignore KeepEmptyLinesAtTheStartOfBlocks for extern "C" blocks

Nico Weber thakis at chromium.org
Thu Nov 13 08:25:52 PST 2014


r221897, thanks!

On Wed, Nov 12, 2014 at 11:37 PM, Daniel Jasper <djasper at google.com> wrote:

> 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/f847fa7f/attachment.html>


More information about the cfe-commits mailing list