hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land.
This LGTM. I believe we have not heard back from @arsenm on the response to some of their comments though. ================ Comment at: llvm/docs/CodingStandards.rst:1603 + + // This should also omit braces. The for loop contains only a single statement, + // so it shouldn't have braces. The if also only contains a single statement (the ---------------- erichkeane wrote: > arsenm wrote: > > This loop should use braces. It covers multiple lines. Omitting braces > > invariably just increases diffs/merge conflicts when something else is > > added to the loop body. > > > > This one isn't consistently applied and I've been enforcing the opposite > This is how we've been enforcing the rule however, can you suggest a change > to the wording that you think would match our current enforcement? Much as it may be seen to be "ideal" that conventions are uniform throughout the project, it is the case that different areas of the code have somewhat of a local convention. The example may lean towards not using braces as the general case, but the wording was done so that areas of code where the observed increase in diffs/merge conflicts occur frequently can use and enforce braces because said occurrences would be indicative of harm caused by the omission of braces. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80947/new/ https://reviews.llvm.org/D80947 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
