[LLVMdev] [bikeshed] Anyone have strong feelings about always putting `template <...>` on its own line?

[David Blaikie]

On Mon, Jul 1, 2013 at 2:21 PM, Sean Silva <silvas at purdue.edu> wrote:
> tl;dr If there are no objections I'd like to change clang-format's LLVM
> style to always put `template <...>` on its own line. I think it's a general
> code-layout consistency win and avoids some cases where trivial code changes
> result in significant formatting differences (see the last example).
>
> Examples of the current behavior:
>
> --------------
> template <class ELFT>
> class ELFState {
>
> clang-format's to:
>
> template <class ELFT> class ELFState {
> --------------
>
> --------------
> template <class T>
> static size_t vectorDataSize(const std::vector<T> &Vec) {
>
> clang-format's to:
>
> template <class T> static size_t vectorDataSize(const std::vector<T> &Vec) {
> --------------
>
> Pathological example:
> --------------
> template <class T, size_t N, int X>
> template <class U>
> Foo<T, N, X>::bar(U Uv) {
>   for (unsigned i = 0, e = Uv.size(); i != e; ++i)
>     use(i);
> }
>
> clang-format's to:
>
> template <class T, size_t N, int X> template <class U> Foo<T, N, X>::bar(U
> Uv) {
>   for (unsigned i = 0, e = Uv.size(); i != e; ++i)
>     use(i);
> }
>
> while the completely minor modification s/int/unsigned/ results (due to line
> length restrictions) in clang-format agreeing with the former layout:
>
> template <class T, size_t N, unsigned X>
> template <class U>
> Foo<T, N, X>::bar(U Uv) {
>   for (unsigned i = 0, e = Uv.size(); i != e; ++i)
>     use(i);
> }
> --------------
>
> I would prefer that two pieces of code with similar logical structure to be
> similarly formatted, as much as reasonable, and I think that always breaking
> on template declarations is reasonable.
>
> clang-format has an option for this (`AlwaysBreakTemplateDeclarations`), but
> we don't have it enabled currently for LLVM style. Daniel Jasper informs me
> that the current setting is a carry-over from before the setting was
> introduced, where it was effectively always "false" (the current setting for
> LLVM style).
>
> I hate to bring up such a microscopic issue, but I find myself manually
> fixing clang-format's behavior with LLVM style when it comes to putting
> `template <...>` on its own line, since TBH I feel like a reviewer would ask
> me to change it.
>
> At a basic level I think what bothers me about it is that it breaks useful
> invariants while reading code:
> - A class or struct declaration always has `class` or `struct` as the first
> non-whitespace character on the line (also helpful when grepping).
> - When reading code, you can seek past the template by just going down to
> the next line starting at the same indentation. If the `template <...>` gets
> put on the same line, then you have to "parse through it" to find what is
> actually being declared.
> - There is a single way to lay out `template <...>`. With the current
> setting, clang-format will still put the `template <...>` on its own line in
> a lot of cases due to line length restrictions, but in others it will be put
> onto the same line. Hence in some cases you can "skip down past it" and in
> others you have to "parse through it", but worst of all you have to detect
> which it is when reading the code.

Have you got any statistics for the current state of LLVM with respect
to this formatting issue? If something is already the overwhelmingly
common style (& it's not a case where it used to be the style, the
style has been updated, and nothing has been migrated yet) then just
make clang-format agree with reality - this doesn't require a
discussion or bikeshed.