Re: [PATCH 5/8] clang-format: avoid braces on simple single-statement bodies

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 8, 2024, 16:48 UTC
Message-ID: <xmqqle2bj0dl.fsf@gitster.g>
In-Reply-To: <20240708092317.267915-6-karthik.188@gmail.com>

Karthik Nayak <karthik.188@gmail.com> writes:

> Set the 'RemoveBracesLLVM' to 'true' which ensures that we avoid curly
> braces for single-statement bodies in conditional blocks.

Hmph, two warnings in its documentation [*] sound ominous, especially the second one that says:

    Warning

    Setting this option to true could lead to incorrect code
    formatting due to clang-format’s lack of complete semantic
    information. As such, extra care should be taken to review code
    changes made by this option.

which implies it may not necessarily a good idea to add to automation without telling contributors that they may get hit with a false positive (or incorrect rewrite).

Reading from the examples in that documentation page, it was unclear how it would handle if/else if/.../else cascade where not all branches are multi-statement blocks, e.g.,

	if (A) {
		do_A_thing();
	} else if (B) {
		do_B_thing();
	} else {
		do_C_things();
		do_other_things();
	}

but looking around I am getting a feeling that the tool would do the right thing, i.e., to match our preference that is to use {braces} around all branches, if I am not mistaken.

[Reference]

 * https://releases.llvm.org/16.0.0/tools/clang/docs/ClangFormatStyleOptions.html#:~:text=RemoveBracesLLVM

> +# Remove optional braces of control statements (if, else, for, and while)
> +# according to the LLVM coding style
> +# This avoids braces on simple single-statement bodies of statements.

"... but keeps braces if one side of if/else if/.../else cascade has multi-statement body."

> +RemoveBracesLLVM: true