<div dir="ltr">Thanks. Those latest tweaks about comment hoisting does clarify for me.<div><br></div><div>I'm not sure whether the code diffs are meant to promote one style above the other. They are different styles that make different tradeoffs over different features. Which one somebody prefers depends will depend on how they value those features. It's nice to have some tangible examples to compare though, so thanks for providing them.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jun 25, 2020 at 2:51 PM Mehdi AMINI <<a href="mailto:joker.eph@gmail.com">joker.eph@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div><br></div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jun 25, 2020 at 2:14 PM Mehdi AMINI <<a href="mailto:joker.eph@gmail.com" target="_blank">joker.eph@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jun 24, 2020 at 5:35 PM Mehdi AMINI <<a href="mailto:joker.eph@gmail.com" target="_blank">joker.eph@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jun 24, 2020 at 11:51 AM Adrian McCarthy via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jun 24, 2020 at 2:37 AM Chris Lattner via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Jun 23, 2020, at 11:02 AM, Philip Reames <<a href="mailto:listmail@philipreames.com" target="_blank">listmail@philipreames.com</a>> wrote:<br>
> 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.<br>
<br>
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.</blockquote><div><br></div><div><div>"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?</div><div><br></div><div>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.</div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> 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.<br></blockquote><div><br></div><div>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. <br></div></div></div></blockquote><div><br></div><div>Can you clarify what is unclear with the <a href="https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements" target="_blank">current rule</a>? </div><div>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.</div><div>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. </div><div><br></div><div>Maybe adding more examples there could help?</div></div></div></div></blockquote><div><br></div><div></div></div></div></div></div></div></blockquote><div><div>Gave it a try here: <a href="https://reviews.llvm.org/D82594" target="_blank">https://reviews.llvm.org/D82594</a></div><div><br></div><div>I'm fairly sure we can keep the spirit and write a clang-tidy rule that would cover most of the cases. </div><div><br></div><div>I also ran an experiment adding all braces on one file: <a href="https://reviews.llvm.org/D82599" target="_blank">https://reviews.llvm.org/D82599</a></div><div><br></div><div>And to be more representative, here is a visual diff (low quality for the mailing-list) of what it looks like in an editor for a single function (left is full braces, right is currently committed):</div></div><div><br></div><div><div><img src="cid:ii_kbvbkjzk1" alt="Screen Shot 2020-06-25 at 2.11.29 PM.jpg" width="821" height="460" style="margin-right: 0px;"><br></div></div><div><br></div><div> Best,</div><div><br></div><div>-- </div><div>Mehdi</div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> <br>
> Philip<br>
> <br>
> On 6/22/20 1:44 PM, Chris Lattner via llvm-dev wrote:<br>
>> For those who don’t like it, is the currently documented policy broken enough to be important to changing?<br>
>> <br>
>> I assume you wouldn’t recommend a massive rewrite of the codebase, so we’re going to be with this for quite some time.<br>
>> <br>
>> -Chris<br>
>> <br>
>>> On Jun 22, 2020, at 1:36 PM, Steve Scalpone via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
>>> <br>
>>> Did this conversation reach a conclusion?<br>
>>> <br>
>>> 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.<br>
>>> <br>
>>> 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.<br>
>>> <br>
>>> 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.<br>
>>> <br>
>>> Certainly an "always use braces" rule would be simpler than what's documented now in the LLVM Coding Standards [1].<br>
>>> <br>
>>> Another option would be to make braces a developer's choice, and ask that those omitting braces please follow the rules documented in [1].<br>
>>> <br>
>>> [1] <a href="https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements" rel="noreferrer" target="_blank">https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements</a><br>
>>> <br>
>>> On 6/18/20, 3:56 AM, "llvm-dev on behalf of Nicolai Hähnle via llvm-dev" <<a href="mailto:llvm-dev-bounces@lists.llvm.org" target="_blank">llvm-dev-bounces@lists.llvm.org</a> on behalf of <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
>>> <br>
>>> External email: Use caution opening links or attachments<br>
>>> <br>
>>> <br>
>>> On Tue, Jun 16, 2020 at 10:35 AM Momchil Velikov via llvm-dev<br>
>>> <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
>>>> My 2 pennies is braces add unnecessary clutter and impair readability when<br>
>>>> used on a *single-line* statement. I count comments, that are on their<br>
>>>> own line as statement(s).<br>
>>> +1 for this. I think braces around single-line statements can be<br>
>>> allowed, but they really shouldn't be mandated, and that's been my<br>
>>> personal policy for reviews. In particular,<br>
>>> <br>
>>> if (!is_transform_applicable) {<br>
>>> return {};<br>
>>> }<br>
>>> <br>
>>> is very aggravating clutter.<br>
>>> <br>
>>> Braces should be required around multi-line statements. Note:<br>
>>> <br>
>>> BAD:<br>
>>> for (...)<br>
>>> for (...)<br>
>>> single_line_statement;<br>
>>> <br>
>>> GOOD:<br>
>>> for (...) {<br>
>>> for (...)<br>
>>> single_line_statement;<br>
>>> }<br>
>>> <br>
>>> Cheers,<br>
>>> Nicolai<br>
>>> --<br>
>>> Lerne, wie die Welt wirklich ist,<br>
>>> aber vergiss niemals, wie sie sein sollte.<br>
>>> _______________________________________________<br>
>>> LLVM Developers mailing list<br>
>>> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
>>> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
>>> <br>
>>> _______________________________________________<br>
>>> LLVM Developers mailing list<br>
>>> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
>>> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
>> _______________________________________________<br>
>> LLVM Developers mailing list<br>
>> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
>> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
<br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div></div>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div></div></div>
</blockquote></div></div></div></div></div>
</blockquote></div></div></div>
</blockquote></div>