Enum formatting changes - polly automatic formatting verifier

[Tobias Grosser]

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.

> Btw. this is not only Google Style. GNU style has this explicitly written
> down, in WebKit it seems to be common.

I mostly care about consistency within the LLVM code base.

Cheers,
Tobias