[llvm-dev] Codifying our Brace rules-

[River Riddle via llvm-dev]

On Mon, Jun 15, 2020 at 4:08 PM David Blaikie via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> On Mon, Jun 15, 2020 at 4:05 PM Mehdi AMINI via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
> >
> > As another data point, the MLIR part of the codebase is pretty
> consistent on this: never use braces for trivial (single statement)
> if/else/for, but always put it on every branch if needed on any side of the
> if/else.
>
> Any opinion/stance policy/practice on the "one line, or one statement
> (& possibly comments, etc)" issue?
>

Generally, any time there is a comment within the body I don't really see
it as "trivial" anymore.

Prefer:
  if (...) {
    // Some comment.
    single statement;
  }
  // Some comment.
  If (...)
    single statement;

Over:
  if (...)
    // Some comment
    single statement;

-- River


>
> > We also have clang-format pretty heavily enforced (including in the
> automated pre-merge testing on phabricator) which does not lead to issues
> where someone would add something into the body of a `for` for example and
> "forget" to add braces. I don't think I have seen any single instance of
> such bugs slipping in our code so far.
> >
> >
> >
> > On Mon, Jun 15, 2020 at 12:46 PM 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
> >>
> >> _______________________________________________
> >> LLVM Developers mailing list
> >> llvm-dev at lists.llvm.org
> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> >
> > _______________________________________________
> > LLVM Developers mailing list
> > llvm-dev at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> _______________________________________________
> 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/20200615/4b332cce/attachment.html>