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

[Tobias Grosser]

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)

Cheers,
Tobias