[patch] Ignore KeepEmptyLinesAtTheStartOfBlocks for extern "C" blocks

Nico Weber thakis at chromium.org
Wed Nov 12 14:26:17 PST 2014


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/20141112/f71358d2/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: format-extern-c.patch
Type: application/octet-stream
Size: 2913 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141112/f71358d2/attachment.obj>


More information about the cfe-commits mailing list