At the very least, I accept clarifying the wording to make it clear where braces should/shouldn't be used. I personally would still prefer a general "always add braces in new code" rule, given that I literally just ran into another case where a code change I made locally caused test failures because I'd forgotten to add the braces on a previously single-line if, although in this case, it should have had braces already, since it had a comment between the if and statement within the if. clang-formatting would have pointed out the error, had I run it, but I hadn't done so. Total cost to me was 2-3 minutes trying to figure out what was wrong. A rule of "always braces" would have meant those 2-3 minutes could have been spent on other things. On Tue, 30 Jun 2020 at 20:59, Chris Lattner via llvm-dev < llvm-dev at lists.llvm.org> wrote:> > > > On Jun 29, 2020, at 8:05 AM, David Greene <david.greene at hpe.com> wrote: > > > > Chris Lattner via llvm-dev <llvm-dev at lists.llvm.org> writes: > > > >> For those who don’t like it, is the currently documented policy broken > >> enough to be important to changing? > > > > I believe it is, simply for safety reasons. Adding a statement to a > > single-statement block can introduce bugs. Mandating braces everywhere > > prevents that. > > Sure, I get that. For background, the Swift language (which I was highly > involved in) mandates braces - I’m just saying that I'm aware of the > benefits and tradeoffs here. In addition to the “safety” advantages, there > are balancing cognitive disadvantages to requiring braces (less code fits > on one screen, etc). These are all well understood effects. > > That said, this is not a new project, and many of the benefits fall below > the line of being “worth it” given a large and established codebase. I > don’t see a strong reason to change the policy here, though I think we > should give more guidance and clarify the wording! > > -Chris > > _______________________________________________ > 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/20200701/e8016ad4/attachment.html>
Hi,> On Jul 1, 2020, at 6:23 AM, James Henderson via llvm-dev <llvm-dev at lists.llvm.org> wrote: > > At the very least, I accept clarifying the wording to make it clear where braces should/shouldn't be used. I personally would still prefer a general "always add braces in new code" rule,FWIW, +1 on always using braces. I like having more compact code without the braces but at the same time, LLVM is the only code base that I work on that doesn’t always have braces. Thus, the benefits are not worth the mental gymnastic for me. I am not saying we should update all the code but I think going forward we could adopt this rule and eventually all the code will migrate. Cheers, -Quentin> given that I literally just ran into another case where a code change I made locally caused test failures because I'd forgotten to add the braces on a previously single-line if, although in this case, it should have had braces already, since it had a comment between the if and statement within the if. clang-formatting would have pointed out the error, had I run it, but I hadn't done so. Total cost to me was 2-3 minutes trying to figure out what was wrong. A rule of "always braces" would have meant those 2-3 minutes could have been spent on other things. > > On Tue, 30 Jun 2020 at 20:59, Chris Lattner via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote: > > > > On Jun 29, 2020, at 8:05 AM, David Greene <david.greene at hpe.com <mailto:david.greene at hpe.com>> wrote: > > > > Chris Lattner via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> writes: > > > >> For those who don’t like it, is the currently documented policy broken > >> enough to be important to changing? > > > > I believe it is, simply for safety reasons. Adding a statement to a > > single-statement block can introduce bugs. Mandating braces everywhere > > prevents that. > > Sure, I get that. For background, the Swift language (which I was highly involved in) mandates braces - I’m just saying that I'm aware of the benefits and tradeoffs here. In addition to the “safety” advantages, there are balancing cognitive disadvantages to requiring braces (less code fits on one screen, etc). These are all well understood effects. > > That said, this is not a new project, and many of the benefits fall below the line of being “worth it” given a large and established codebase. I don’t see a strong reason to change the policy here, though I think we should give more guidance and clarify the wording! > > -Chris > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev <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/20200701/8b22c6f9/attachment.html>
On 7/1/20 9:55 AM, Quentin Colombet via llvm-dev wrote:> Hi, > >> On Jul 1, 2020, at 6:23 AM, James Henderson via llvm-dev >> <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote: >> >> At the very least, I accept clarifying the wording to make it clear >> where braces should/shouldn't be used. I personally would still >> prefer a general "always add braces in new code" rule, > > FWIW, +1 on always using braces. > I like having more compact code without the braces but at the same > time, LLVM is the only code base that I work on that doesn’t always > have braces. Thus, the benefits are not worth the mental gymnastic for me. > > I am not saying we should update all the code but I think going > forward we could adopt this rule and eventually all the code will migrate.(Meta point) This thread has mixed a large number of proposed changes. I have lost track completely. I'm about to stop reading this thread, and ask that if there is a consensus reached that a separate RFC is sent the list for broader review. This might even be a good trial run for Chris's new controversial proposal process. Philip> > Cheers, > -Quentin > >> given that I literally just ran into another case where a code change >> I made locally caused test failures because I'd forgotten to add the >> braces on a previously single-line if, although in this case, it >> should have had braces already, since it had a comment between the if >> and statement within the if. clang-formatting would have pointed out >> the error, had I run it, but I hadn't done so. Total cost to me was >> 2-3 minutes trying to figure out what was wrong. A rule of "always >> braces" would have meant those 2-3 minutes could have been spent on >> other things. >> >> On Tue, 30 Jun 2020 at 20:59, Chris Lattner via llvm-dev >> <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote: >> >> >> >> > On Jun 29, 2020, at 8:05 AM, David Greene <david.greene at hpe.com >> <mailto:david.greene at hpe.com>> wrote: >> > >> > Chris Lattner via llvm-dev <llvm-dev at lists.llvm.org >> <mailto:llvm-dev at lists.llvm.org>> writes: >> > >> >> For those who don’t like it, is the currently documented >> policy broken >> >> enough to be important to changing? >> > >> > I believe it is, simply for safety reasons. Adding a statement >> to a >> > single-statement block can introduce bugs. Mandating braces >> everywhere >> > prevents that. >> >> Sure, I get that. For background, the Swift language (which I >> was highly involved in) mandates braces - I’m just saying that >> I'm aware of the benefits and tradeoffs here. In addition to the >> “safety” advantages, there are balancing cognitive disadvantages >> to requiring braces (less code fits on one screen, etc). These >> are all well understood effects. >> >> That said, this is not a new project, and many of the benefits >> fall below the line of being “worth it” given a large and >> established codebase. I don’t see a strong reason to change the >> policy here, though I think we should give more guidance and >> clarify the wording! >> >> -Chris >> >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org <mailto: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 <mailto: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/20200701/251a8358/attachment-0001.html>