r212591 - clang-format: Revamp function declaration/definition indentation.

[Tobias Grosser]

On 09/07/2014 14:12, Tobias Grosser wrote:
> On 09/07/2014 13:18, Chandler Carruth wrote:
>> On Wed, Jul 9, 2014 at 4:01 AM, Tobias Grosser <tobias at grosser.es> wrote:
>>
>>> Also, in http://llvm.org/PR16157 the very same bug was "solved" already
>>> and Manual especially introduced the option that you just removed to
>>> address the cl::opt changes that now show up.
>>>
>>
>> I'm pretty sure this bug and the subsequent patch dealt almost
>> exclusively
>> with the case of function declarations and definitions. The follow-up
>> examples are all those and the tests for the change are those.
>> Unfortunate,
>> but I don't think it was ever really targeted at global variables.
>
> I have no idea about Manuel's intention, but the original bug report was
> specifically opened due to the cl::opt case.
>
>>> Here is another open bug that talks about inconsistent formatting
>>> between
>>> different lines: http://llvm.org/PR15169
>>>
>>
>> Ok, hadn't seen this bug any more than you had seen mine. Sorry about
>> that.
>> =/ The bugs should probably have been dup'ed.
>>
>>> Formatting is mostly about taking decisions and sticking to them, so I
>>> think having a discussion once might avoid bike-shedding and
>>> clang-format
>>> noise afterwards.
>>>
>>
>> But I don't think we ever made a conscious decision about global
>> variables.
>> They just went along for the ride with the very clear decision about
>> function definitions and declarations previously.
>  > I think the history here is more that global variables got lumped in
> with
>> function declarations because we didn't have a good way to separate them
>> not out of any specific desire for one formatting. Then in this commit
>> Daniel added a way to separate them, and picked the default based on a
>> somewhat arbitrary choice rooted in logic rather than data or widespread
>> opinion of developers. So no decision has really been made or even
>> considered.
>
> Being able to do the separation is obviously beneficial.
>
>> You seem to disagree with the current default selected, so
>> argue for a change (with a fresh thread to llvmdev maybe). The whole
>> reason
>> I don't like checking every single revision is because it creates a false
>> urgency to change clang-format's defaults the moment they disagree with
>> whatever happens to be have come before and happens to have been widely
>> applied to Polly's codebase. If consensus is that we want to format
>> globals
>> in this other way, I doubt the intervening time where clang-format worked
>> differently will cause any harm at all.
>
> I understand that this urgency may have heated up the discussion
> unnecessarily. Unfortunately checking every revision is the only way we
> get timely information on project scale regressions that may pass by
> unnoticed otherwise.
>
> To put this back into scale. Within the last half year, this is the
> first clang-format change where I observed a large policy change that
> worth a discussion.
>
> In this year there have been _3_ other clang-format changes, that have
> all been obviously beneficial and Polly was immediately reformatted
> (everybody is invited to do so)
>
>> A separate reason why I think a revert is a bad way to respond to this is
>> because it conflates a policy decision with adding the mechanism to
>> implement the policy. We should absolutely not revert functionality from
>> clang-format because of a discussion and disagreement about one policy
>> flag
>> in LLVM's style... That makes it incredibly hard to make progress.
>
> This is actually a very good point. Reverting was not really necessary.
>
> However, in cases where we see policy changes that raise concerns, would
> it be a problem to set the policy flag to match the original behavior
> until the discussion is over? (This happens maybe once every six months)

This is not needed any more this time as I trashed the polly history 
already.

Now let's actually move on to detangle the beneficial/non-beneficial 
parts of this change.

Cheers,
Tobias