[llvm-dev] Codifying our Brace rules-

James Henderson via llvm-dev llvm-dev at lists.llvm.org
Tue Jun 16 01:02:12 PDT 2020


I'm with Matt on this one. I much prefer the approach of ALWAYS use braces
for ifs and for loops, even if they're not needed, for basically the same
reasons as he put. The number of times I've added a statement inside an if
without braces and forget to add them is annoyingly high, especially as
it's not always an obvious error upfront. Similarly, being involved in a
downstream codebase with several private patches, having to sometimes add
the braces makes merges all the harder and sometimes more dangerous.

I doubt we're going to get the policy changed from "don't include
unnecessary braces for trivial statements" but if there's any style that
adds them in more places, I'm up for that.

James

On Mon, 15 Jun 2020 at 20:56, Matt Arsenault via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

>
>
> On Jun 15, 2020, at 15:46, Keane, Erich via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
> Hi all-
>
> A few weeks ago I noticed that our “omit braces with single line blocks”
> rule wasn’t written down!  Additionally, as a group on IRC and in review,
> noticed that the enforcement of this rule has been extremely inconsistent.
> We made a first run at codifying our existing practice here:
> https://reviews.llvm.org/D80947, which was then committed after
> significant time on llvm-commits.
>
> I would like to encourage the list via discussion and further
> reviews/commits to come to a consensus on what we actually MEAN by this
> rule.  For example, a recent comment points out that :
>
> If (cond)
>   Stmt;
> else if (cond)
>   Stmt;
> else {
>   Stmt;
>   Stmt;
> }
>
> Should require braces on all of the conditions!  However, we are
> extraordinarily inconsistent here.  My wish is for us to become more
> consistent, so I would like us to use this thread to organize our
> collective thoughts on figuring out what the rule actually SHOULD be, and
> organizing a handful of commits to the coding standard to make sure it says
> what we mean.
>
> Thanks,
> Erich
>
>
>
> I think braces should be added in all contexts, and the more contexts the
> better. It eliminates any inconsistency or attempt to contextually
> interpret rules. It also reduces merge conflicts, since something
> eventually something will probably be added inside any control flow
> statement. I’ve suffered through many painful merges trying to find where
> the braces went wrong, usually due to switch statements. The
> sometimes-braces-sometimes-not combined with the lack of indentation for
> switch cases leads to way more time figuring out braces than should be
> necessary.
>
> -Matt
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200616/463a54ad/attachment.html>


More information about the llvm-dev mailing list