Enum formatting changes - polly automatic formatting verifier

Tobias Grosser tobias at grosser.es
Mon Jan 6 00:44:21 PST 2014


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?

Cheers,
Tobias



More information about the llvm-commits mailing list