[llvm-dev] Codifying our Brace rules-

Mehdi AMINI via llvm-dev llvm-dev at lists.llvm.org
Wed Jun 24 17:35:13 PDT 2020


On Wed, Jun 24, 2020 at 11:51 AM Adrian McCarthy via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

>
>
> On Wed, Jun 24, 2020 at 2:37 AM Chris Lattner via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> On Jun 23, 2020, at 11:02 AM, Philip Reames <listmail at philipreames.com>
>> wrote:
>> > I'll note that reading along I haven't found any of the proposed
>> changes particularly worthwhile.  I'm also not strongly opposed to any of
>> them - I just don't care - but I certainly haven't been convinced there's
>> any clear benefit to be had by changing our current policy.
>>
>> I agree.  The discussion is also hard to follow, because there are many
>> different competing suggestions and opinions.  There are a couple of people
>> talking about clarifying the rules to be less prescriptive, which seem like
>> it is worth discussing.
>
>
> "Clarifying the rules to be less prescriptive" sounds self-contradictory.
> Are you in favor of talking about clarifying the existing guidelines or
> changing them to be less prescriptive?  Or maybe you want to change them a
> little so that they are easier to express clearly?
>
> There are already several well-defined de facto standard brace styles.
> One way to make the guidelines clear (and concise) is simply to declare
> LLVM uses $(FOO) Brace Style with a link to the Wikipedia description.
> That suggests to me that it's not super feasible to divorce clarification
> from style choice, at least, not without putting a bound on how clear we
> can be.
>
>
>>   I think we should take the suggestion of “always require braces” off
>> the table, because it doesn’t make sense given the impact to the code base.
>>
>
> Given that the codebase is already riddled with inconsistencies (and
> instances that I cannot determine the correctness against the current
> guidelines), I don't understand why you think it doesn't make sense to
> consider a simpler scheme.  The current inconsistencies exist because the
> rules are unclear and, because of the edge cases, hard to internalize.  A
> simpler rule (or set of rules) would presumably result in fewer
> inconsistencies going forward, so the code would evolve toward a more
> consistent state.
>

Can you clarify what is unclear with the current rule
<https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements>
?
The title of the section is "Don't Use Braces on Simple Single-Statement
Bodies of if/else/loop Statements", which seems already fairly clear.
It then also mentions exceptions to the rule: readability and
maintainability ; and clarifies what is considered not readable and not
maintainable. It even gives two examples.

Maybe adding more examples there could help?

-- 
Mehdi






>
>
>>
>> -Chris
>>
>> >
>> > Philip
>> >
>> > On 6/22/20 1:44 PM, Chris Lattner via llvm-dev wrote:
>> >> For those who don’t like it, is the currently documented policy broken
>> enough to be important to changing?
>> >>
>> >> I assume you wouldn’t recommend a massive rewrite of the codebase, so
>> we’re going to be with this for quite some time.
>> >>
>> >> -Chris
>> >>
>> >>> On Jun 22, 2020, at 1:36 PM, Steve Scalpone via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>> >>>
>> >>> Did this conversation reach a conclusion?
>> >>>
>> >>> My ad hoc tally says that a slight majority of the responders
>> preferred to fully brace statements and no one wanted to totally eliminate
>> braces.
>> >>>
>> >>> The technical arguments for fully braced statements were 1) it's
>> considered a slightly safer coding style and 2) commit diffs with fully
>> braced statements may be slightly more to the point.
>> >>>
>> >>> I didn't register any technical arguments for less-than-fully-braced
>> statement -- the preference seemed to be aesthetic.  I may have missed a
>> technical argument.
>> >>>
>> >>> Certainly an "always use braces" rule would be simpler than what's
>> documented now in the LLVM Coding Standards [1].
>> >>>
>> >>> Another option would be to make braces a developer's choice, and ask
>> that those omitting braces please follow the rules documented in [1].
>> >>>
>> >>> [1]
>> https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
>> >>>
>> >>> On 6/18/20, 3:56 AM, "llvm-dev on behalf of Nicolai Hähnle via
>> llvm-dev" <llvm-dev-bounces at lists.llvm.org on behalf of
>> llvm-dev at lists.llvm.org> wrote:
>> >>>
>> >>>    External email: Use caution opening links or attachments
>> >>>
>> >>>
>> >>>    On Tue, Jun 16, 2020 at 10:35 AM Momchil Velikov via llvm-dev
>> >>>    <llvm-dev at lists.llvm.org> wrote:
>> >>>> My 2 pennies is braces add unnecessary clutter and impair
>> readability when
>> >>>> used on a *single-line* statement. I count comments, that are on
>> their
>> >>>> own line as statement(s).
>> >>>    +1 for this. I think braces around single-line statements can be
>> >>>    allowed, but they really shouldn't be mandated, and that's been my
>> >>>    personal policy for reviews. In particular,
>> >>>
>> >>>      if (!is_transform_applicable) {
>> >>>        return {};
>> >>>      }
>> >>>
>> >>>    is very aggravating clutter.
>> >>>
>> >>>    Braces should be required around multi-line statements. Note:
>> >>>
>> >>>    BAD:
>> >>>      for (...)
>> >>>        for (...)
>> >>>          single_line_statement;
>> >>>
>> >>>    GOOD:
>> >>>      for (...) {
>> >>>        for (...)
>> >>>          single_line_statement;
>> >>>      }
>> >>>
>> >>>    Cheers,
>> >>>    Nicolai
>> >>>    --
>> >>>    Lerne, wie die Welt wirklich ist,
>> >>>    aber vergiss niemals, wie sie sein sollte.
>> >>>    _______________________________________________
>> >>>    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
>>
>> _______________________________________________
>> 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/20200624/c99387fa/attachment.html>


More information about the llvm-dev mailing list