[clang-format] Changing Swift's clang-format config to enable always break on templates

TLDR: Currently swift's clang-format config (in swift/.clang-format) has break on templates disabled by leaving BreakTemplateDeclarationsStyle set to its default value BTDS_No. I would like to propose that we enable it by default. This is just my preference but from talking with other people, it seems that other people also prefer this and that new instances AFAIKT only enter via git-clang-format.


In the following I am using info from the clang-format docs: Clang-Format Style Options — Clang 16.0.0git documentation. The specific tweak I am talking about is called BreakTemplateDeclarationsStyle:

AlwaysBreakTemplateDeclarations (BreakTemplateDeclarationsStyle)
The template declaration breaking style to use.
Possible values:
BTDS_No (in configuration: No) Do not force break before declaration. PenaltyBreakTemplateDeclaration is taken into account.
template <typename T> T foo() {
}
template <typename T> T foo(int aaaaaaaaaaaaaaaaaaaaa,
                            int bbbbbbbbbbbbbbbbbbbbb) {
}
BTDS_MultiLine (in configuration: MultiLine) Force break after template declaration only when the following declaration spans multiple lines.
template <typename T> T foo() {
}
template <typename T>
T foo(int aaaaaaaaaaaaaaaaaaaaa,
      int bbbbbbbbbbbbbbbbbbbbb) {
}
BTDS_Yes (in configuration: Yes) Always break after template declaration.
template <typename T>
T foo() {
}
template <typename T>
T foo(int aaaaaaaaaaaaaaaaaaaaa,
      int bbbbbbbbbbbbbbbbbbbbb) {
}

The gist of it is that currently I am pretty sure we are BTDS_No. I am proposing that we change this to BTDS_Yes. If one looks at the code base, most templates already look like BTDS_Yes except for instances where clang-format did it. To be clear, I am not suggesting that we auto-format the code base. I am just suggesting that we change it so that more BTDS_No instances enter the code base.

What do you all think about making this change to the .clang-format file?

12 Likes

I definitely prefer the results from BTDS_Yes, so I am in favour of this style divergence from LLVM.

There is some cost to the style divergences - it makes it more difficult to cross between the LLVM umbrella and the Swift compiler codebase, so we should be judicious in the divergences.

Although I am fine with making this forward facing only, there is really no effective downside to doing it retroactively and formatting the codebase any longer. The argument that it hurts history does not hold true any longer due to git's support for ignoring such revisions (v2.23+) via .git-blame-ignore-revs.

1 Like

+1 to the proposal and reformatting everything. We should be thinking about evolution more than about churn. Churn is a fixed cost (and a pretty small one, unnoticeable to most users, as @compnerd notes), but the cost of not evolving is unbounded, since we're not planning to shut down the project any time soon.

While I agree with @compnerd and @gribozavr that reformatting upfront would be good, I would like to request that we minimize how often such churn happens because it effectively breaks all in-progress branches/work until people have a chance to reformat locally.

If we are going down the path of churn, I would like to deeply and humbly request that style churn events be limited to once a year on a set date. This would allow people to plan around it.

Also, planning on [potential!] annual style tweaks helps avoid needless debates about getting every style option "right". Just let the decisions pile up as needed until "churn day" and then just do it.

5 Likes

+1 to the proposal. I'd also recommend bringing this up on llvm-dev, as the default is wrong for LLVM too.

Not to necro this topic, but I am going to quickly do this for the .clang-format file. It seems like the question is around the amount of change and no one is against an incremental change as a first step. I only have time to ensure we do not enter more of this into the code base. If someone else wants to take on the larger discussion of doing this everywhere, they should feel free to do so.

+1. Thanks @Michael_Gottesman. Let's at least fix what we can when we can. No need to reformat everything en masse.