[PATCH] D100727: [clang-format] Correctly apply AllowShortIfStatementsOnASingleLine: Always to else branch.
Björn Schäpers via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 19 13:00:23 PDT 2021
HazardyKnusperkeks added a comment.
In D100727#2698422 <https://reviews.llvm.org/D100727#2698422>, @curdeius wrote:
> More I look at the current state of this option, more I think it's misleading.
> I would expect that the if after else is also controlled by this option. And, the last else as well (adding yet another option for that seems to be a bit an overkill)
>
> @klimek, @mydeveloperday, I'd love to hear your input as well on the best course of action, as I've seen that you've discussed this topic a bit in https://reviews.llvm.org/D59087.
>
> The current status quo looks like:
>
> /// Different styles for handling short if statements.
> enum ShortIfStyle : unsigned char {
> /// Never put short ifs on the same line.
> /// \code
> /// if (a)
> /// return;
> ///
> /// if (b)
> /// return;
> /// else
> /// return;
> ///
> /// if (c)
> /// return;
> /// else {
> /// return;
> /// }
> /// \endcode
> SIS_Never,
> /// Put short ifs on the same line only if there is no else statement.
> /// \code
> /// if (a) return;
> ///
> /// if (b)
> /// return;
> /// else
> /// return;
> ///
> /// if (c)
> /// return;
> /// else {
> /// return;
> /// }
> /// \endcode
> SIS_WithoutElse,
> /// Always put short ifs on the same line.
> /// \code
> /// if (a) return;
> ///
> /// if (b) return;
> /// else
> /// return;
> ///
> /// if (c) return;
> /// else {
> /// return;
> /// }
> /// \endcode
> SIS_Always,
> };
>
> I.e. `WithoutElse` does not depend on the else statement being compound or not. I think I'll push a NFC commit to fix documentation and add tests for this.
>
> Anyway, what I'm inclined to do is to have these options:
>
> - Never (same as now)
> - WithoutElse (same as now, concerns only a simple `if (c) f();`)
> - OnlyFirstIf (renamed from `Always`, and `Always` kept for backward compatibility, it would behave as currently, so only the first `if` in the sequence of if else if else if else is concerned)
> - AllIfsAndElse (would do what I want to achieve in this patch, so format like this:
>
> if (c) f();
> else if (c) f();
> else if (c) f();
> else if();
>
> Naming is hard. All suggestions are welcome :).
Sounds good.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100727/new/
https://reviews.llvm.org/D100727
More information about the cfe-commits
mailing list