Enum formatting changes - polly automatic formatting verifier

[Tobias Grosser]

On 01/06/2014 09:44 AM, Tobias Grosser wrote:
> Hi Daniel,
>
> thanks to the feedback. As written to Chandler, this is my last mail. I
> made my point. It's on you to take the decision (which you apparently did).
>
> On 01/06/2014 05:05 AM, Daniel Jasper wrote:
>> On Sun, Jan 5, 2014 at 11:31 PM, Tobias Grosser <tobias at grosser.es>
>> wrote:
>>
>>> On 01/05/2014 09:44 PM, Daniel Jasper wrote:
>>>
>>>> On Sun, Jan 5, 2014 at 3:58 PM, Tobias Grosser <tobias at grosser.es>
>>>> wrote:
>>>>
>>>>   On 01/05/2014 02:13 PM, Daniel Jasper wrote:
>>>>>
>>>>>   Tobias, could you submit the two small formatting changes that are
>>>>>> required? I am not entirely certain whether it would be ok for me to
>>>>>> just
>>>>>> submit patches to polly. Also I don't think it would be a good
>>>>>> idea for
>>>>>> clang-format developers to be required to ensure other projects'
>>>>>> adherence
>>>>>> to what clang-format deems to be correct style.
>>>>>>
>>>>>>
>>>>> Hi Daniel,
>>>>>
>>>>> thanks for letting me know. You are not required to fix Polly, but if
>>>>> there is an obvious and easy fix, I would be more than happy if you
>>>>> do so
>>>>> (it also keeps the buildbot noise down). For non-obvious changes I
>>>>> would
>>>>> prefer a short pre-commit mail (or just an headsup as today).
>>>>> However, if
>>>>> the style change is non-obvious it would be great if you could make it
>>>>> off
>>>>> by default for LLVM and give a short headsup on the mailing list such
>>>>> that
>>>>> I can prepare a patch for Polly.
>>>>>
>>>>> Regarding the change today, I wonder how you figured out that the new
>>>>> formatting is 'the predominant choice' in LLVM/clang? I just
>>>>> checked what
>>>>> formatting is more common in LLVM/clang by counting the one line
>>>>> enums in
>>>>> the current LLVM and clang code and after running the latest
>>>>> clang-format
>>>>> over all .cpp and .h files.
>>>>>
>>>>> I use the following grep call to count:
>>>>>
>>>>> grep -R 'enum.*{.*}.*;' tools/clang/lib/ lib/ include/ \
>>>>>                           tools/clang/include | wc
>>>>>
>>>>> I get 259 cases for the current formatting and 531 for the reformatted
>>>>> code, which means 272 cases use currently a multi line formatting.
>>>>>
>>>>>
>>>> I did a slightly different analysis. I searched for "enum.*}" and there
>>>> seem to be 432 files with in LLVM/Clang containing these. This
>>>> compares to
>>>> a search (with multiline regexs) for "\n\ *enum[^\}]{0,73}\n\}" of
>>>> which I
>>>> find 80 files containing them. The latter should be all enums that
>>>> can be
>>>> written on a single line but aren't.
>>>>
>>>> Without paths containing "test" these numbers change to 219 vs. 47
>>>> files.
>>>> The 5:1 ratio seems quite consistent. The regular expressions might
>>>> have
>>>> errors, I only hacked at them until I was satisfied and the individual
>>>> findings were what I was looking for.
>>>>
>>>
>>> It seems there slipped in a bigger problem in your second regexp. You do
>>> not count any indented multiline enums as you match for '\n\}' at the
>>> end.
>>> (Also fixing your regexp is not that easy as multi-line enums with more
>>> than 73 characters can easily fit into a line as most of the whitespace
>>> used for indentation can be removed).
>>>
>>> Do you see any problems in using clang-format directly as proposed
>>> above?
>>> Or can we agree that there is no single line enums are not more common?
>>
>>
>> Chandler has already addressed most of the concerns better than I could.
>> clang-format (being half-way useful) is still less than a year old and we
>> need to be able to make some changes or else development will become
>> unnecessarily hard.
>  >
>>
>> As for this specific change, I tried to explain the numbers I used.
>> Probably your numbers are more accurate and a better way to analyze the
>> current state. However, even based on your numbers, it is an even split.
>> You find 259 cases in favor of single line formatting and 272 cases in
>> favor of one enumerator per line. That is as close to a break-even as it
>> gets. Thus, before and after the change, clang-format would change about
>> the same amount of code of the LLVM codebase making the change a neutral
>> one. Adding the preference of users, e.g. expressed through
>> llvm.org/PR16517,
>> this still makes a good change.
>
> If this is the official way people want to have code formatted, maybe

> this users should push a change to the LLVM style guide?

Specifically, you would probably want to change the formatting in the
example there too to avoid incosistencies between clang-format and the
style in the style guide:

http://www.llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

Cheers,
Tobias