[cfe-dev] clang-format turning tests into no-ops

[Sean Silva]

On Fri, Nov 8, 2013 at 1:57 AM, Daniel Jasper <djasper at google.com> wrote:

>
>
>
> On Thu, Nov 7, 2013 at 10:48 PM, Sean Silva <silvas at purdue.edu> wrote:
>
>>
>>
>>
>> On Fri, Nov 8, 2013 at 12:49 AM, Alp Toker <alp at nuanti.com> wrote:
>>
>>>  Hi Daniel,
>>>
>>> Here's a good one for you..
>>>
>>> I've recently been fixing a whole lot of lit tests in the LLVM / clang
>>> suites that don't check anything.
>>>
>>> It was a mystery why people keep checking in tests like this:
>>>
>>> // RUN: clang-check "%s" -- -target x86_64-apple-darwin10 -fasm-blocks
>>> 2>&1 |
>>> // FileCheck %s
>>>
>>> The first line will run successfully, and the second doesn't begin with
>>> RUN so the test passes silently. Not good.
>>>
>>> Then it dawned on me -- contributors must be running clang-format before
>>> committing, causing their RUN lines to get split up at the line boundary.
>>>
>>> I know this isn't strictly a clang-format bug, but it's time consuming
>>> to manually audit and find no-op tests like this after the fact and the
>>> failure mode is insidious because these tests will always silently pass.
>>>
>>
>> Although it doesn't eliminate the hassle of having to manually fix this,
>> it seems like at least one issue worth fixing in its own right is the fact
>> that RUN lines ending with a | are silently accepted by our test
>> infrastructure.
>>
>> Also, FWIW, it seems like clang-format's comment reflowing turns out to
>> do the wrong thing in a ton of (most?) scenarios (for example, it will wrap
>> the last word onto the next line, leaving a bunch of spurious "one word"
>> lines), to the point that I wish there were an option to tell clang-format
>> to not mess with comments at all.
>>
>
> clang-format does not "reflow" yet. It just breaks a comment if it is too
> long. We'll likely want a "reflow" option, but probably bound to a specific
> "editor mode", so that clang-format does not mess up some person's
> ASCII-art comment unless it is done in an editor and the result can easily
> be undone.
>

Ok, that explains it. I guess I was misinterpreting "reflow not
implemented" as "broken".


>
> FWIW, I consider a "spurious on word" comment better than a comment
> violating the column limit, so I am quite strongly against just turning
> comment breaking off.
>

Maybe better, but still unacceptable (it's a tough pickle to get out of).
This is a not-that-extreme example of what I have seen (on some random
lorem ipsum text):

// Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas mattis
quam
// non nibh
// pretium ultrices. Etiam iaculis facilisis tristique.  Suspendisse at
mattis
// magna.
// Fusce facilisis purus dui, eget porttitor velit porta at. Nulla commodo
// mauris
// porttitor mauris malesuada, quis facilisis justo egestas. Cras laoreet
neque
// est, vel
// congue nisi dapibus mollis. Morbi vehicula auctor libero, sit amet
// pellentesque justo.
// Mauris nisi nisi, sodales id ligula in, mattis sollicitudin lorem. Sed
// eleifend dui
// sit amet arcu vehicula dignissim. Suspendisse at porta felis, ut interdum
// nisl. Donec
// pellentesque justo ac dui sagittis, et facilisis augue fermentum. Fusce
// dapibus
// pulvinar erat et blandit. Praesent in quam non nunc consequat venenatis
ac
// sed ipsum.
// Morbi rutrum leo quis elit iaculis tristique. Curabitur et metus justo.

This kind of comment shape would not be acceptable in any codebase I know
of. I think it could be argued that simply not touching comments is the
better choice, but since it seems like proper reflowing is on the agenda,
it's probably not worth discussing (especially since it's so easy to fix
with `gq` in vim).

-- Sean Silva


>
> -- Sean Silva
>>
>>
>>>
>>> I was wondering if you can think of a way to exclude the test suites
>>> from clang-format and clang-format-diff runs. Guessing this could be tricky
>>> given that there are several different ways to run clang-format but
>>> something needs to be done..
>>>
>>> Alp.
>>>
>>> -- http://www.nuanti.com
>>> the browser experts
>>>
>>>
>>> _______________________________________________
>>> cfe-dev mailing list
>>> cfe-dev at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20131108/4af45318/attachment.html>