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

[Daniel Jasper]

In addition to what Chandler said, from a more technical view (possibly
leading up to a separate post to llvm-dev):

Fundamentally, this change enables clang-format to distinguish variable
declarations from function declarations, which inherently is a good thing.
As Chandler said, this puts us into a position to actually format variable
and function declarations differently (as opposed to not being able to
pick) and I tried to pick a default based on what I gathered from existing
practice, existing bugs and other considerations.

Are you actually disagreeing with this default? If so for any reason other
than the difference from the current state of the codebase? To be clear,
this change means, we now format:

  int // break
      SomeVariable;
  int
  SomeFunction();

.. and that decision is not really arbitrary. I can give many arguments for
this being a good idea:

- Clearly visually distinguish variable declarations from function
declarations.
- How would you handle variable declarations declaring multiple variables?
- Indentation after function declarations/definitions IS NOT a good idea as
the function name can get confused with the following body or wrapped
parameters. Indentation after variable declarations IS a good idea so the
variable name does not blur in with the subsequent statements.
- ..



On Wed, Jul 9, 2014 at 1:18 PM, Chandler Carruth <chandlerc at google.com>
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.
>
>
>> 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. 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.
>
> 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.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140709/116958c2/attachment.html>