On 20-08-01 23:07, Taylor R Campbell wrote:
| > Module Name: src
| > Committed By: lukem
| > Date: Sat Aug 1 02:45:36 UTC 2020
| >
| > Modified Files:
| > src/share/misc: style
| >
| > Log Message:
| > style: prefer braces for single statement control statements
| >
| > Prefer to use { braces } around single statements after
| > control statements, instead of discouraging them.
| >
| > Per discussion on tech-userlevel & tech-kern, where the significant
| > majority of developers who responded (including current and former
| > core members) prefer this new style.
|
| Hmm...that's not the conclusion I got from the thread. What you proposed
| (https://mail-index.netbsd.org/tech-userlevel/2020/07/12/msg012536.html),
| and got consensus on, was:
|
| - discourage braces around single statements
| + permit braces around single statements
|
| What you committed was:
|
| - discourage braces around single statements
| + prefer braces around single statements and add braces to all examples
|
| At least two core members (me and kre) preferred the change you
| originally proposed over the change you committed.
|
| Personally I feel that braces around short statements hurt legibility
| by adding unnecessary visual clutter, and make it more cumbersome to
| have consistent patterns like
|
| if (foo() == -1)
| goto fail0;
| if ((x = bar()) == -1)
| goto fail1;
| if (baz() == -1)
| goto fail2;
|
| which makes it more tempting to get clever with shortcuts for error
| branches or with reversing the sense of the branch, and we have too
| many bugs with clever shortcuts in error branches already.
|
| We don't have a `goto fail' problem in NetBSD -- if we did, our
| toolchain would detect it with -Werror=misleading-indentation, as I
| just confirmed experimentally. (Same goes for macros that expand to
| multiple statements, with -Werror=multistatement-macros.)
|
| Can you please restore this to the change you originally suggested,
| along the lines of the attached patch?
| Index: share/misc/style
| ===================================================================
| RCS file: /cvsroot/src/share/misc/style,v
| retrieving revision 1.56
| diff -p -p -u -r1.56 style
| --- share/misc/style 1 Aug 2020 02:45:35 -0000 1.56
| +++ share/misc/style 1 Aug 2020 22:54:53 -0000
| @@ -241,9 +241,8 @@ main(int argc, char *argv[])
| errno = 0;
| num = strtol(optarg, &ep, 10);
| if (num <= 0 || *ep != '\0' || (errno == ERANGE &&
| - (num == LONG_MAX || num == LONG_MIN)) ) {
| + (num == LONG_MAX || num == LONG_MIN)) )
| errx(1, "illegal number -- %s", optarg);
| - }
| break;
| case '?':
| default:
| @@ -256,16 +255,16 @@ main(int argc, char *argv[])
|
| /*
| * Space after keywords (while, for, return, switch).
| - * Braces are preferred for control statements
| - * with only a single statement.
| + *
| + * Braces around single-line bodies are optional; use discretion.
| *
| * Forever loops are done with for's, not while's.
| */
| - for (p = buf; *p != '\0'; ++p) {
| + for (p = buf; *p != '\0'; ++p)
| continue; /* Explicit no-op */
| - }
| for (;;) {
| - stmt;
| + stmt1;
| + stmt2;
| }
|
| /*
| @@ -317,9 +316,8 @@ main(int argc, char *argv[])
| }
|
| /* No spaces after function names. */
| - if ((result = function(a1, a2, a3, a4)) == NULL) {
| + if ((result = function(a1, a2, a3, a4)) == NULL)
| exit(1);
| - }
|
| /*
| * Unary operators don't require spaces, binary operators do.
| @@ -397,12 +395,10 @@ function(int a1, int a2, float fl, int a
| *
| * Use err/warn(3), don't roll your own!
| */
| - if ((four = malloc(sizeof(*four))) == NULL) {
| + if ((four = malloc(sizeof(*four))) == NULL)
| err(1, NULL);
| - }
| - if ((six = (int *)overflow()) == NULL) {
| + if ((six = (int *)overflow()) == NULL)
| errx(1, "Number overflowed.");
| - }
|
| /* No parentheses are needed around the return value. */
| return eight;
| @@ -426,9 +422,8 @@ dirinfo(const char *p, struct stat *sb,
| _DIAGASSERT(p != NULL);
| _DIAGASSERT(filedesc != -1);
|
| - if (stat(p, sb) < 0) {
| + if (stat(p, sb) < 0)
| err(1, "Unable to stat %s", p);
| - }
|
| /*
| * To printf quantities that might be larger than "long", include
I've reverted the change.
Bikeshed away.
