[PATCH] clang-format: options for spacing template argument lists

Christopher Olsen chrisaolsen at gmail.com
Mon Oct 28 09:29:23 PDT 2013


Thanks for the feedback.  I've updated the patch:

Added the flag SpacesInAngles - A<int> vs A< int >
Note that LanguageStandard=Cpp03 overrides SpacesInAngles=false in the
case of '>>' as in A<A<int> >

While I tend to agree with you on spacing in template arguments I have
5+ million lines of code developed over 12 years in strict "pad
everything" style to contend with.  Honestly, I'm hoping clang-format
will eventually allow us to re-evaluate certain style choices if it
gets adopted on our project.

Chris

On Sun, Oct 27, 2013 at 4:32 AM, Daniel Jasper <djasper at google.com> wrote:
> The patch itself looks fine. However, I am also slightly skeptical that it
> is a good general direction.
>
> Specifically:
> - SpaceInEmptyAngles: Quite frankly, it doesn't matter enough. Nobody really
> cares (or should care about this). I'd make a decision and get on with life.
> If you need that space for your codebase, make it dependent on
> SpacesInAngles.
> - SpaceAfterTemplateKeyword: Same as above, it doesn't matter enough. We
> actually had a discussion about this very early into the clang-format
> development. People don't agree, but it really does not matter (no version
> is more or less readable, ..). We then actually went ahead (IIRC) to change
> all instances in the C++11 standard to do exactly what clang-format does
> now. I'd rather have clang-format force people into a consistent behavior
> for such non-issues than add additional technical complexity to it.
> - SpacesInAngles: I would personally be appalled by having to work in a
> codebase that uses this. Constructs like "template < class T..." simply look
> too much like comparisons to me. However, I can see that, if you are used to
> it, switching might be hard. The overlapping of the language standard is
> unfortunate, but I don't really see what we can do here.
>
> So, I am more or less fine with introducing SpacesInAngles (goes well with
> SpacesInParentheses), but I'd rather not introduce the other two (I know
> that we have SpaceInEmptyParentheses - I also don't like that one, but "()"
> is way more common than "<>", so it matters a bit more).
>
> I also agree that, at the very least, we need some grouping of the options
> soon. I have not found a way to specify style options more intuitively. And
> to some extend, I think people having to carefully look through the options
> or accept a default is reasonable.
>
> Cheers,
> Daniel
>
>
> On Fri, Oct 25, 2013 at 11:26 PM, Sean Silva <silvas at purdue.edu> wrote:
>>
>>
>>
>>
>> On Fri, Oct 25, 2013 at 4:00 PM, Christopher Olsen <chrisaolsen at gmail.com>
>> wrote:
>>>
>>> I am evaluating clang-format and need control over spacing in template
>>> argument lists to conform to our coding standards.
>>>
>>> I've attached a patch that adds new flags to control this spacing.
>>> Unittests are also included.
>>>
>>> - SpacesInAngles - A<int> vs A< int >
>>> - SpaceInEmptyAngles - template <> vs template < >
>>> - SpaceAfterTemplateKeyword - template<typename T> vs template <typename
>>> T>
>>>
>>> Note that LanguageStandard=Cpp03 overrides SpacesInAngles=false in the
>>> case of '>>' as in A<A<int> >
>>>
>>> Please let me know if there is anything I need to fix for submission.
>>
>>
>> I foresee (and have already run into multiple real use cases) where the
>> spacing before, after, in between, etc. of many different kinds of tokens
>> needs to be tweaked. For example:
>>
>> * multiplicative operators don't have spaces, but additive operators do `a
>> + b*c`
>> * spaces before parenthesized lists in function calls `foo (bar)`
>> * spaces inside the parentheses of an `if`: `if ( cond ) {`
>> * "function-like" return: `return(3)`
>>
>> I think we should try (though it may not be realistic) to integrate this
>> functionality in a way that covers the different use cases and makes them
>> interact in a consistent and understandable way; otherwise we will just end
>> up growing a forest of not-easy-to-discover options that don't have very
>> good coverage of the configuration space for the next project. For example,
>> adapting clang-format to the OP's project coding standards requires adding 3
>> new options; is there a realistic upper bound on the number of such options
>> that we will need in order for clang-format to support, say, 1000 different
>> projects from 50 different companies/open-source communities?
>>
>> One possibility that I can imagine (although I don't know how feasible it
>> is) is to ship another tool (or more likely keep it under an option to
>> clang-format) which does a "one time" analysis to determine a set of
>> parameters that will conform with a given sample source file (or files) and
>> emits a configuration file. This analysis could work with a larger (but very
>> consistent and well-defined) "plumbing" configuration space that essentially
>> parameterizes the "guts" of clang-format (such as `spaceRequiredBefore`,
>> `spaceRequiredBetween`, `splitPenalty`, etc.) in a data-driven way.
>>
>> On the other hand, I think it has been put out there before is that one of
>> the benefits of clang-format is to help "standardize" to some extent on a
>> common subset of style options, and so providing too-fine-grained support
>> for "tweaking" the output might be undesirable from such a perspective. On
>> the other hand, if clang-format wants to "dominate the world", it can't
>> impose arbitrary changes on a project's coding style. A poignant question is
>> "is it a goal for clang-format be able to conform with
>> more-or-less-arbitrary styles without requiring requiring the users to get
>> involved with clang-format development?", but I can't speak as to the
>> answer.
>>
>> Daniel, what do you think?
>>
>> -- Sean Silva
>>
>>>
>>>
>>> Thanks!
>>> Chris
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang-format_spaces_in_angles.patch
Type: application/octet-stream
Size: 4340 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131028/34e439f5/attachment.obj>


More information about the cfe-commits mailing list