Enum formatting changes - polly automatic formatting verifier

[Tobias Grosser]

On 01/05/2014 11:31 PM, Tobias Grosser 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?
>
>> It seems the one line formatting is actually slightly less common in the
>>> LLVM/clang files. I also don't believe that the new style buys us
>>> anything
>>> in readability.
>>>
>>
>> It still makes sense to build clang-format close to the more frequent
>> style.
>
> Agreed. Unfortunately it seems this is not the case here.
>
>> Plus, the people I talked to before-hand (Manuel, Alex and a few
>> other users prefer single lines).
>
> I did not see this discussion. Though I propose if you meet internally
> and agree on a new or more precise style for LLVM, the next step should
> probably be to submit a patch to the official style guide for review. If
> it is accepted, changing the LLVM style in clang-format is a no-brainer.
>
>> I would prefer clang-format to remain consistent between versions,
>>> as long as there is no good reason to break this consistency.
>>
>>
>> I think the above (preferences of users + more similarity with existing
>> code) constitutes a good reason.
>
> See above.
>
>>> Consequently, I wonder if it would not make sense to use the old
>>> style in
>>> LLVM mode?
>>>
>>
>> a) Not from the numbers I had. We can do a more thorough analysis,
>> though.
>
> They seem incorrect. The analysis I reported seems to give reliable
> numbers.
>
>> b) If there is an even split inside LLVM (as your numbers would suggest)
>> then I'd prefer to have fewer options.
>
> What is the problem with additional options? I did not touch them once yet.
>
> However, as people start to use clang-format regularly we will start to
> have problems with different versions of clang-format formatting
> differently. (This does not affect companies who always provide the
> latest version to their developers. But this may not be the common case).
>
> In the last months clang-format has been shown to be stable and
> consistent with changes only affecting rare special cases. This change
> impacts over 500 cases. Being here inconsistent without a very good
> reason would be a pity.

I meanwhile reformatted Polly to silence the buildbots. I still believe 
this change should be reevaluated for the LLVM style.

Cheers,
Tobias