Maybe s/isExternCBlock/startsExternCBlock/.

Otherwise looks good. Thanks!

On Wed, Nov 12, 2014 at 11:46 PM, Nico Weber <[email protected]> wrote:

> Now uses getNextNonComment().
>
> On Wed, Nov 12, 2014 at 2:26 PM, Nico Weber <[email protected]> wrote:
>
>> On Wed, Nov 12, 2014 at 1:46 PM, Daniel Jasper <[email protected]>
>> 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.
>>
>>
>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to