Patch for clang-format to allow references and pointers to bind differently

[Sean Silva]

On Thu, Nov 28, 2013 at 4:21 AM, Daniel Jasper <djasper at google.com> wrote:

> > It's certainly not the most *popular* formatting style, but it's the
> > one I learned. Some other formatting tools, such as astyle, support
> > this style.
> > I'm guessing the reason I learned this style was because my background
> > was C before C++, and most C code uses "int *var" but a lot of C++
> > code uses "int& var" and avoids pointers.
> > Note that the ambiguity doesn't usually happen in practice with
> > references because references must be initialized so only one is
> > declared on each line.
>
> Well, it is also quite rare that people declare multiple uninitialized
> pointers in one line (and it would be a questionable practice).
>
> My key argument against this style would be that it suggests that "*" and
> "&" have a different binding behavior, which I don't think they have.
>
> > I'm trying to go from manually-enforced style to automatically, and I
>
> > don't want the first formatting run to introduce a huge diff. I think
> > that is the *single* *biggest* *thing* people want out of a formatting
> > tool. It's not your job try to force one particular style on anyone
> > (unless you're writing a new language - Go did this well).
>
> Well, it is not quite as easy:
> 1. I am responsible for maintaining clang-format. Ensuring that all
> combinations of options do the right thing gets significantly harder with
> more options.
> 2. I want to make clang-format as useful as possible to as many people as
> possible. Need to churn through pages and pages of flag documentation to
> find the important flags makes it harder to use.
>

But this is precisely what the current documentation does. It is just an
alphabetical listing, with no indication of importance. I would oppose
blocking a patch because adding the option will somehow make it harder to
navigate the docs: the solution is to organize the docs better!


> I am not saying that this should be a counterargument to adding any flag
> at all. However, if it is a flag for a style only used by one or two
> projects, adding the flag might be a net loss.
>

We already have accepted some features like that, like those weird template
spacing options. Also, as you mention below, this problem goes away with a
--detect-style capability. I'm pretty sure that most project maintainers
want this workflow:

$ cd myproject
$ clang-format --detect-style . # writes .clang-format
$ echo "If you clang-format you will never have a formatting issue with
your patch" >> CONTRIBUTING
$ git add . && git commit -m "Add .clang-format"

It would be awesome if we could deliver this to them.


> 3. There are style choices that are bad or chosen for bad reasons. I think
> I would put this one into this category. Now, if we support that style
> choice in clang-format we ingrain it even further. If we don't (but
> clang-format still delivers good value) we might convince a few
> people/projects to move a way from this style.
>

This is assuming that "supporting a style in clang-format" is something
that requires extra maintenance overhead of clang-format. I sketched a
design in another thread that wouldn't have this issue, basically you turn
it into an optimization problem over an abstract formatting ruleset.

-- Sean Silva


>
> Regarding #2: I don't know how many people would be interested in this
> style. I have never seen it before and would think it is rare, but I might
> just be ignorant.
>
> On Thu, Nov 28, 2013 at 6:43 AM, Ben Longbons <brlongbons at gmail.com>wrote:
>
>> On Wed, Nov 27, 2013 at 6:15 PM, Alp Toker <alp at nuanti.com> wrote:
>> > On 28/11/2013 01:50, Ben Longbons wrote:
>> >> I'm trying to go from manually-enforced style to automatically
>> > Would an option to not touch the existing pointer/reference style be
>> > satisfactory?
>> Well, there already is an option DerivePointerBinding, which my patch
>> changes to do separate deduction for references.
>>
>> Changing that to keep the existing style at each individual site would
>> solve the problem of the initial diff when applying it to my repo, but
>> not the main problem that a code formatter is supposed to solve.
>>
>
> I agree.
>
>
>>  > I can see a wider appeal for controlling what gets formatted and what
>> > doesn't, as opposed to just trying to support every personal taste here.
>> I admit, when I first started messing with the GNU indent program as a
>> novice, I asked "why doesn't this program ONLY change indentation?" At
>> the time I didn't even know about expand(1).
>>
>> But what I want *now*, as a maintainer, is to be able to say to my
>> contributors: "if you run 'make format', your code will comply to all
>> style guidelines".
>>
>> I actually think it's a bad idea for a formatter to *ever* preserve
>> anything, except in an initial "clang-format --detect-style >
>> .clang-format" run.
>>
>
> Once we have such an option, my argument #2 looses a lot of its weight.
> Until then, we should keep a close eye on the number of flags clang-format
> has.
>
> The only hard cases I can think of are:
>> 1. if it decides to break a comment (which is irreversible, so should
>> never be done by default ... I'm not sure what exactly clang-format
>> does with PenaltyBreakComment)
>>
>
> Turn PenaltyBreakComment to a high value. clang-format will then avoid
> breaking comments.
>
>
>> 2. when preserving multiple blank lines between logically distinct
>> parts of a source file (but what you really want in that case is
>> probably either a documentation section or a namespace or something,
>> so I could live without that).
>>
>
> You can probably change MaxEmptyLinesToKeep. Or do you mean you need a
> context dependent setting of this?
>
>
>> -o11c
>>
>
> Alp: Yes, I agree that "attach" or "align" (which astyle uses) would be
> better names for this flag. But I am waiting to resolve a few questions
> around this (e.g. there is also a request to support astyle's "middle"
> alignment). We basically have to be backwards compatible, so I'd like to
> change flags as seldomly as possible.
>
> Cheers,
> Daniel
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131203/1bf9610d/attachment.html>