Update PEP 7 to require curly braces in C

While doing a review of http://bugs.python.org/review/26129/ I asked to have curly braces put around all `if` statement bodies. Serhiy pointed out that PEP 7 says curly braces are optional: https://www.python.org/dev/peps/pep-0007/#id5. I would like to change that. My argument is to require them to prevent bugs like the one Apple made with OpenSSL about two years ago: https://www.imperialviolet.org/2014/02/22/applebug.html. Skipping the curly braces is purely an aesthetic thing while leaving them out can lead to actual bugs. Anyone object if I update PEP 7 to remove the optionality of curly braces in PEP 7?

On Sun, 17 Jan 2016, 13:59 Ethan Furman <ethan@stoneleaf.us> wrote:
Yeah, bad phrasing on my part. What I meant to say is leaving them off is an aesthetic thing while requiring them is a bug prevention thing. When it comes to writing C code I always vote for practicality over aesthetics.

I'm +0. The editor I use is too smart to let me make this mistake, but I don't object to recommending it. As usual, though, let's not start mindless reformatting of existing code. -- --Guido van Rossum (python.org/~guido)

On 01/17/2016 11:19 PM, Brett Cannon wrote:
+1. Out of curiosity, I made a quick script to see if we had any candidates for bugs related to this. I didn't expect any bugs to be found, since with the amount of static checkers that have been run they should have been found. The only problem I found was in the S390 port of libffi (#ifdef-conditional code which wouldn't even compile). I also found (in ast.c) two instances of semantically correct code with the wrong indent level which I fixed (see rev 1ececa34b748). cheers, Georg

On 18.01.2016 08:00, Victor Stinner wrote:
I like if without braces when the body is only one line, especially when there is no else block.
Same here. Compilers warn about these things today, so I don't think we need to go paranoid ;-)
-- Marc-Andre Lemburg eGenix.com Professional Python Services directly from the Experts (#1, Jan 18 2016)
::: We implement business ideas - efficiently in both time and costs ::: eGenix.com Software, Skills and Services GmbH Pastor-Loeh-Str.48 D-40764 Langenfeld, Germany. CEO Dipl.-Math. Marc-Andre Lemburg Registered at Amtsgericht Duesseldorf: HRB 46611 http://www.egenix.com/company/contact/ http://www.malemburg.com/

On 17.01.16 21:10, Brett Cannon wrote:
I'm -0. The code without braces looks more clear. Especially if the body is one-line return, break, continue or goto statement. Sometimes it is appropriate to add an empty line after it for even larger clearness. On the other hand, there is no a precedence of bugs like the one Apple made in CPython sources. Mandatory braces *may be* will prevent hypothetical bug, but for sure make a lot of correct code harder to read.

On Jan 17, 2016, at 11:10, Brett Cannon <brett@python.org> wrote:
There are two ways you could do that. The first is to just change "braces may be omitted where C permits, but when present, they should be formatted as follows" to something like "braces must not be omitted, and should be formatted as follows", changing one-liner tests into this: if (!obj) { return -1; } Alternatively, it could say something like "braces must not be omitted; when other C styles would use a braceless one-liner, a one-liner with braces should be used instead; otherwise, they should be formatted as follows", changing the same tests into: if (!obj) { return -1; } The first one is obviously a much bigger change in the formatting of actual code, even if it's a simpler change to the PEP. Is that what was intended?

On Jan 17, 2016, at 11:10, Brett Cannon <brett at python.org> wrote:
While doing a review of http://bugs.python.org/review/26129/ ... update PEP 7 to remove the optionality of curly braces
On Mon Jan 18 03:39:42 EST 2016, Andrew Barnert pointed out:
There are two ways you could do that.
[The one most people are talking about, which often makes an if-clause visually too heavy ... though Alexander Walters pointed out that "Any excuse to break code out into more functions... is usually the right idea."] if (!obj) { return -1; }
That "otherwise" gets a bit awkward, but I like the idea. Perhaps "braces must not be omitted, and should normally be formatted as follows. ... Where other C styles would permit a braceless one-liner, the expression and braces may be moved to a single line, as follows: " if (x > 5) { y++ } I think that is clearly better, but it may be *too* lightweight for flow control. if (!obj) { return -1; } does work for me, and I think the \n{} may actually be useful for warning that flow control takes a jump. One reason I posted was to point to a specific example already in PEP 7 itself: if (type->tp_dictoffset != 0 && base->tp_dictoffset == 0 && type->tp_dictoffset == b_size && (size_t)t_size == b_size + sizeof(PyObject *)) return 0; /* "Forgive" adding a __dict__ only */ For me, that return is already visually lost, simply because it shares an indentation with the much larger test expression. Would that be better as either: /* "Forgive" adding a __dict__ only */ if (type->tp_dictoffset != 0 && base->tp_dictoffset == 0 && type->tp_dictoffset == b_size && (size_t)t_size == b_size + sizeof(PyObject *)) { return 0; } or: /* "Forgive" adding a __dict__ only */ if (type->tp_dictoffset != 0 && base->tp_dictoffset == 0 && type->tp_dictoffset == b_size && (size_t)t_size == b_size + sizeof(PyObject *)) { return 0; } -jJ -- If there are still threading problems with my replies, please email me with details, so that I can try to resolve them. -jJ

Your wording is much better than mine. And so is your suggestion. Giving people the option of 1 or 3 lines, but not 2, seems a little silly. And, while I rarely use or see your 2-line version in C, I use it quite a bit in C++ (and related languages like D), so it doesn't look at all weird to me. But I'll leave it up to people who only do C (and Python) and/or who are more familiar with the CPython code base to judge.

Let's not switch to either of those options. Visually I much prefer either of these: if (test) { blah; } or if (test) blah; over the versions with '{ blah; }' (regardless of whether it's on the same line as 'if' or on the next line). It looks like the shorter versions are mostly used inside macros, where aesthetics usually go out the door anyways in favor of robustness. Since this discussion is never going to end until someone says "enough", let me just attempt that (though technically it's Brett's call) -- let's go with the strong recommendation to prefer if (test) { blah; } and stop there. On Tue, Jan 19, 2016 at 10:29 AM, Ethan Furman <ethan@stoneleaf.us> wrote:
-- --Guido van Rossum (python.org/~guido)

Here is a proposed update: diff -r 633f51d10a67 pep-0007.txt --- a/pep-0007.txt Mon Jan 18 10:52:57 2016 -0800 +++ b/pep-0007.txt Tue Jan 19 12:11:44 2016 -0800 @@ -75,9 +75,9 @@ } * Code structure: one space between keywords like ``if``, ``for`` and - the following left paren; no spaces inside the paren; braces may be - omitted where C permits but when present, they should be formatted - as shown:: + the following left paren; no spaces inside the paren; braces are + strongly preferred but may be omitted where C permits, and they + should be formatted as shown:: if (mro != NULL) { ... On Tue, 19 Jan 2016 at 11:37 Guido van Rossum <guido@python.org> wrote:

On 19 January 2016 at 20:12, Brett Cannon <brett@python.org> wrote:
This change seems to be accidentally smuggled in, in the guise of a PEP 512 update :) My view is I prefer always using curly brackets in my own code. It is easier to add printf() debugging without making mistakes. I support “strongly preferring” them in the style guide, which is as much as a style guide can do anyway.

The wording is totally fine! You might still want to revert it and re-commit it so it doesn't look like a mistake when reviewing the log. BTW When can we start using git for the peps repo? On Wed, Jan 20, 2016 at 12:18 PM, Brett Cannon <brett@python.org> wrote:
-- --Guido van Rossum (python.org/~guido)

On Wed, 20 Jan 2016 at 14:31 Guido van Rossum <guido@python.org> wrote:
The wording is totally fine! You might still want to revert it and re-commit it so it doesn't look like a mistake when reviewing the log.
Sure thing!
BTW When can we start using git for the peps repo?
Depends on how fast the parts covered in https://www.python.org/dev/peps/pep-0512/#requirements-for-code-only-reposit... and https://www.python.org/dev/peps/pep-0512/#requirements-for-web-related-repos... takes. My hope is before PyCon US (although if we choose to not enforce the CLA on PEP contributions then it could happen even faster). -Brett

On 21 January 2016 at 10:16, Brett Cannon <brett@python.org> wrote:
We should probably mention that in PEP 1 - I wasn't aware of that requirement, so I've never explicitly checked CLA status for folks contributing packaging related PEPs. (And looking at the just-checked-in PEP 513, I apparently have a CLA to chase up...) Regards, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia

I'm still yet to meet a lawyer who trusts "public domain" statements... The CLA will ensure we have enough rights to republish the PEP on p.o or future sites, and it doesn't prevent authors from also releasing the text elsewhere under other terms. Top-posted from my Windows Phone -----Original Message----- From: "Zachary Ware" <zachary.ware+pydev@gmail.com> Sent: 1/21/2016 9:23 To: "Python-Dev" <python-dev@python.org> Subject: Re: [Python-Dev] Update PEP 7 to require curly braces in C On Thu, Jan 21, 2016 at 11:12 AM, Brett Cannon <brett@python.org> wrote:
It's quite surprising to me, since all (as far as I know) PEPs are explicitly public domain. I am very much not a lawyer, though :) -- Zach _______________________________________________ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/steve.dower%40python.org

Zachary Ware writes:
It's quite surprising to me, since all (as far as I know) PEPs are explicitly public domain. I am very much not a lawyer, though :)
The reason for any explicit CLA is CYA: you can't be sure that the contributor DTRTs, so the CLA shows that the contribution was received in good faith. (Some CLAs also include indemnification clauses, such as the contributor warrants that they own the copyright, etc, but all CLAs are useful to show good faith, which may matter big time if the contributor turns out to have a all-IP-yours-is-ours employment contract or the like.) You don't really need to be a lawyer to understand this stuff, you just need to think like a security person. https://www.schneier.com/blog/archives/2008/03/the_security_mi_1.html

On 18 January 2016 at 05:10, Brett Cannon <brett@python.org> wrote:
+1 from me, as I usually add them to code I'm editing anyway (I find it too hard to read otherwise, especially when there's a long series of them). Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia

On 01/17/2016 11:10 AM, Brett Cannon wrote:
Anyone object if I update PEP 7 to remove the optionality of curly braces in PEP 7?
I'm -1. I don't like being forced to add the curly braces when the code is perfectly clear without them. If this was a frequent problem then I'd put up with it, but I can't recall ever making this particular mistake myself or seeing it in CPython source. It seems to me like a fix for a problem we don't have. //arry/

On Mon, Jan 18, 2016 at 9:59 AM, Larry Hastings <larry@hastings.org> wrote:
I'm +1. We don't have that problem yet and the idea Brett brought up is for future changes that will happen. We'll be soon moving to github, which should simplify the process of submitting PRs from other developers interested in making our beautiful language even more awesome. I'm quite positive that with current review process that kind of bug should not happen, but you never know. Having this as a requirement is rather to minimize the risk of potentially having such bugs. I've switched to this style myself good couple years ago and I find it very readable atm. Maciej */arry*

On Mon, Jan 18, 2016 at 10:42 PM, Maciej Szulik <soltysh@gmail.com> wrote:
Rather than forcing people to use braces, wouldn't it be easier to just add a linter to the toolchain that will detect those kinds of problems and reject the commit? Which might be as simple as telling a compiler to treat this warning as an error, and then always use that compiler at some point before committing. (I don't know how many compilers can check this.) I'm -1 on forcing people to use a particular style when a script could do the same job more reliably. ChrisA

On Mon, Jan 18, 2016 at 11:02 PM, Larry Hastings <larry@hastings.org> wrote:
Only in the exact situation that this is trying to prevent, where indentation implies something that braces don't stipulate. From the original Apple bug link: if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0) goto fail; goto fail; Since there are two indented lines after the if and no braces, this should be flagged as an error. If there's only one indented line, braces are optional. ChrisA

On Mon, 18 Jan 2016 at 00:59 Larry Hastings <larry@hastings.org> wrote:
I'm going to assume you mean it is clearer without them, else I don't hear an argument against beyond "I don't wanna". For me, I don't see how:: int x = do_something(); if (x != 10) return NULL; do_some_more(); is any clearer or more readable than:: int x = do_something(); if (x != 10) { return NULL; } do_some_more();
I personally have come close to screwing up like this, but I caught it before I put up a patch. I honestly also prefer the consistency of just always using braces rather than the sometimes rule we have. It isn't like C is a pretty, safe language to begin with, so I would rather be consistent and avoid potential errors.

Brett Cannon wrote:
Maybe not for that piece of code on its own, but the version with braces takes up one more line. Put a few of those together, and you can't fit as much code on the screen. If it makes the difference between being able to see e.g. the whole of a loop at once vs. having to scroll up and down, it could make the code as a whole harder to read. -- Greg

On 1/18/2016 23:27, Greg Ewing wrote:
When someone trying to make this argument in #python for Python code... the response is newlines are free. Almost this entire thread has me confused - the arguments against are kind of hypocritical; You are developing a language with a built in design ethic, and ignoring those ethics while building the implementation itself. Newlines are free, use them Explicit > Implicit - Explicitly scope everything. I am not a core developer, but I just kind of feel its hypocritical to oppose always using brackets for the development of *python*

On 19 January 2016 at 14:40, Alexander Walters <tritium-list@sdamon.com> wrote:
There are two conflicting code aesthetics at work here, and the relevant one for the folks that prefer to avoid braces in C where they can is:
So if we bring *Python* into the comparison, we can see it splits the difference between the two C variants by omitting the closing brace and replacing the opening brace with ":": x = do_something() if (x != 10): return None do_some_more() The additional "cost" of mandatory braces in C is thus more lines containing only a single "}", while the benefit is simply not having to think about the braceless variant as a possible alternative spelling. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia

Alexander Walters wrote:
When someone trying to make this argument in #python for Python code... the response is newlines are free.
Well, I disagree. I very rarely put blank lines in a function in any language, because it makes it hard to scan the code visually and pick out the beginnings of functions. So while they may help readability locally, they hurt it globally. If I find myself needing to put blank lines in a function in order to make it readable, I take it as a sign that the function ought to be split up into smaller functions. -- Greg

Alexander Walters wrote:
I am not a core developer, but I just kind of feel its hypocritical to oppose always using brackets for the development of *python*
If we were being *really* pythonic, we would write all the C code without any braces at all, and feed it through a filter that adds them based on indentation. End of argument. :-) -- Greg

On 01/18/2016 08:40 PM, Alexander Walters wrote:
I am not a core developer, but I just kind of feel its hypocritical to oppose always using brackets for the development of *python*
CPython isn't written in Python, it's written in C. So we use C idioms, which naturally are different from Python idioms. //arry/

On 18 January 2016 at 23:25, Alexander Walters <tritium-list@sdamon.com> wrote:
And in the spirit of safety, which I think is Brett's concern. Yes, there are tools that could catch the Apple bug <https://www.imperialviolet.org/2014/02/22/applebug.html>, by catching incorrect indentation. But if the rule is to *always* use braces, that just feels safer. Even if it offends some people's ability to read everything at one glance (my suggestion: use a smaller font or a larger screen; or refactor the code so it isn't as big a chunk). There's a fairly exhaustive list of styles here: https://en.wikipedia.org/wiki/Indent_style I'm sure that some of these have not been advocated in this thread.

On Jan 17, 2016, at 07:10 PM, Brett Cannon wrote:
Anyone object if I update PEP 7 to remove the optionality of curly braces in PEP 7?
+1 for requiring them everywhere, -1 for a wholesale reformatting of existing code. New code and significant refactoring/rewriting can adopt braces everywhere. Cheers, -Barry

I'm usually fine with code like this: if ( ... ) return But when there is a multi-line 'else' clause, I just want to use braces for both 'if' and its 'else'. So, for consistency, I'm +1 for recommending to use braces everywhere. And +1 for requiring to use braces if one of the clauses of the 'if' statement is multi-line. Yury On 2016-01-17 2:10 PM, Brett Cannon wrote:

On 1/18/2016 6:20 PM, Brett Cannon wrote:
Though I don't write C anymore, I occasionally read our C sources. I dislike mixed bracketing in a multiple clause if/else statement, and would strongly recommend against that. On the other hand, to my Python-trained eye, brackets for one line clauses are just noise. +-0. If coverity's scan does not flag the sort of misleading bug bait formatting that at least partly prompted this thread if (a): b; c; then I think we should find or write something that does and run it over existing code as well as patches. -- Terry Jan Reedy

On 01/19/2016 01:18 AM, Terry Reedy wrote:
I don't know if static checkers care about whitespace and indentation in C; it might be a very obvious thing to do for Python programmers, but maybe not for C static checker developers :) And probably with good reason, since whitespace isn't a consideration except for nicely readable code, which many people (not talking about CPython here) apparently don't care about, you'd have tons of spurious checker messages. In many cases (especially error handling ones like "goto fail"), I expect the checker to flag the error anyway, but for semantic reasons, not because of whitespace. Georg

On Mon, 2016-01-18 at 19:18 -0500, Terry Reedy wrote:
FWIW, for the forthcoming gcc 6, I've implemented a new -Wmisleading-indentation warning that catches this. It's currently enabled by -Wall: sslKeyExchange.c: In function 'SSLVerifySignedServerKeyExchange': sslKeyExchange.c:631:8: warning: statement is indented as if it were guarded by... [-Wmisleading-indentation] goto fail; ^~~~ sslKeyExchange.c:629:4: note: ...this 'if' clause, but it is not if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0) ^~ (not that I've had time for core Python development lately, but FWIW in gcc-python-plugin I mandate braces for single-statement clauses). Dave

On 19.01.2016 00:20, Brett Cannon wrote:
Make that:
There are plenty other cases where typos can ruin the flow of your code in C; the discussed case is not a very common one. I find the whole discussion a bit strange: In Python we're advocating not having to use curly braces, because whitespace already provides the needed semantics for us, yet you are now advocating that without adding extra curly braces we'd be in danger of writing wrong code. The Apple bug can happen in Python just as well: if a: run_if_true() else: run_if_false() can turn into (say by hitting a wrong key in the editor): if a: run_if_true() run_if_false() (run_if_false is now run when a is true, and nothing is done in case a is false) So what's the correct way to address this ? It's having a test for both branches (a is true, a is false), not starting to write e.g. if a: run_if_true() if not a: run_if false() to feel more "secure". Also note that the extra braces won't necessarily help you. If you remove "else" from: if (a) { run_if_true(); } else { run_if_false(); } you get exactly the same Apply bug as before, only with more curly braces. This all sounds a bit like security theater to me ;-) I'd say: people who feel better using the extra braces can use them, while others who don't need them can go ahead as always ... and both groups should write more tests :-) BTW: There are other things in PEP 7 which should probably be updated instead, e.g. it still says we should use C89, but we're having more and more C99 code (mostly new library functions) in CPython. -- Marc-Andre Lemburg eGenix.com Professional Python Services directly from the Experts (#1, Jan 19 2016)
::: We implement business ideas - efficiently in both time and costs ::: eGenix.com Software, Skills and Services GmbH Pastor-Loeh-Str.48 D-40764 Langenfeld, Germany. CEO Dipl.-Math. Marc-Andre Lemburg Registered at Amtsgericht Duesseldorf: HRB 46611 http://www.egenix.com/company/contact/ http://www.malemburg.com/

To add my additional 0.02$ to the discussion. Quite a while ago my coworkers and I agreed to use strict rules regarding code formatting. That idea turned into a formatter we've had committed in the project itself. This forced everyone on the team to use it and thus producing exactly "the same looking" code. I found it very nice, although we've debated over how those rules should be applied for good couple months, that was at one of the previous companies I've worked for. Currently by day I'm working with Go, which go-fmt is by default required for all our projects and I found it extremely handy see [1], [2], [3]. Maciej [1] https://github.com/openshift/origin/ [2] https://github.com/openshift/source-to-image/ [3] https://github.com/kubernetes/kubernetes On Tue, Jan 19, 2016 at 9:48 AM, M.-A. Lemburg <mal@egenix.com> wrote:

M.-A. Lemburg <mal <at> egenix.com> writes:
I want to clarify my position a bit: Personally, in _decimal/* I've always used braces and I prefer that. But from reading the Python sources in general, I got the impression that the default style at least for one-liner if-statements is to omit braces. So, in memoryview.c, I adapted to that style. I think enforcing braces won't do anything for security. DJB (who had a single exploit found in qmail in 20 years) even uses nested for-loops without braces. IMO secure code can only be achieved by auditing it quietly in a terminal, not being distracted by peripheral things like version control and web interfaces (green merge buttons!) and trying to do formal proofs (if time allows for it). So I would not want to enforce a style if it makes some people unhappy. Stefan Krah

On Tue, 19 Jan 2016 at 00:48 M.-A. Lemburg <mal@egenix.com> wrote:
Yes, because in one language whitespace represents semantics while the other is just formatting; I don't have to question the meaning of when something is indented in Python, but in C I have to question whether the indentation is an accident or the missing braces is the accident.
OK, but what if the block was instead: if (a) run_if_true(); Py_INCREF(a); ? Unit tests are not going to easily turn up a refcount leak, and the number of times I have needed to email python-dev when Antoine's daily refcount email has found a leak for several days shows people do not watch closely for this. It's one thing when the expressions are obviously mutually exclusive, but that's an ideal case. This isn't always about losing an `else` statement as it can come from inserting a new statement and not noticing that the braces are missing.
That's fine. I also want format consistency by always using braces.
Well, I'm dropping out of this discussion because I have enough on my plate with the GitHub migration than to keep fighting this.
That's a whole other discussion (which I support, but I'm not going to lead since I'm burned out at the moment on C-related discussions). -Brett

A formatter bot would be quite complicated to introduce without disrupitions of everybody's workflow (remember that we have about half a million lines of C code in the Python repo). If you want to discuss that please start a new thread on python-dev. On Tue, Jan 19, 2016 at 12:22 PM, francismb <francismb@email.de> wrote:
-- --Guido van Rossum (python.org/~guido)

On Sun, Jan 17, 2016 at 11:12 AM Brett Cannon <brett@python.org> wrote:
+1, always using {}s is just good C style. (and, duh, of course we do *not* go modifying code for this retroactively, pep8 vs our existing python code is evidence of that) If I had _my_ way we'd require clang format for C/C++ files and yapf for all Python files before accepting a commit. Like any good modern open source project should. People who don't like defensive bug reducing coding practices should be glad I don't get my way. :P -gps

On Sun, 17 Jan 2016, 13:59 Ethan Furman <ethan@stoneleaf.us> wrote:
Yeah, bad phrasing on my part. What I meant to say is leaving them off is an aesthetic thing while requiring them is a bug prevention thing. When it comes to writing C code I always vote for practicality over aesthetics.

I'm +0. The editor I use is too smart to let me make this mistake, but I don't object to recommending it. As usual, though, let's not start mindless reformatting of existing code. -- --Guido van Rossum (python.org/~guido)

On 01/17/2016 11:19 PM, Brett Cannon wrote:
+1. Out of curiosity, I made a quick script to see if we had any candidates for bugs related to this. I didn't expect any bugs to be found, since with the amount of static checkers that have been run they should have been found. The only problem I found was in the S390 port of libffi (#ifdef-conditional code which wouldn't even compile). I also found (in ast.c) two instances of semantically correct code with the wrong indent level which I fixed (see rev 1ececa34b748). cheers, Georg

On 18.01.2016 08:00, Victor Stinner wrote:
I like if without braces when the body is only one line, especially when there is no else block.
Same here. Compilers warn about these things today, so I don't think we need to go paranoid ;-)
-- Marc-Andre Lemburg eGenix.com Professional Python Services directly from the Experts (#1, Jan 18 2016)
::: We implement business ideas - efficiently in both time and costs ::: eGenix.com Software, Skills and Services GmbH Pastor-Loeh-Str.48 D-40764 Langenfeld, Germany. CEO Dipl.-Math. Marc-Andre Lemburg Registered at Amtsgericht Duesseldorf: HRB 46611 http://www.egenix.com/company/contact/ http://www.malemburg.com/

On 17.01.16 21:10, Brett Cannon wrote:
I'm -0. The code without braces looks more clear. Especially if the body is one-line return, break, continue or goto statement. Sometimes it is appropriate to add an empty line after it for even larger clearness. On the other hand, there is no a precedence of bugs like the one Apple made in CPython sources. Mandatory braces *may be* will prevent hypothetical bug, but for sure make a lot of correct code harder to read.

On Jan 17, 2016, at 11:10, Brett Cannon <brett@python.org> wrote:
There are two ways you could do that. The first is to just change "braces may be omitted where C permits, but when present, they should be formatted as follows" to something like "braces must not be omitted, and should be formatted as follows", changing one-liner tests into this: if (!obj) { return -1; } Alternatively, it could say something like "braces must not be omitted; when other C styles would use a braceless one-liner, a one-liner with braces should be used instead; otherwise, they should be formatted as follows", changing the same tests into: if (!obj) { return -1; } The first one is obviously a much bigger change in the formatting of actual code, even if it's a simpler change to the PEP. Is that what was intended?

On Jan 17, 2016, at 11:10, Brett Cannon <brett at python.org> wrote:
While doing a review of http://bugs.python.org/review/26129/ ... update PEP 7 to remove the optionality of curly braces
On Mon Jan 18 03:39:42 EST 2016, Andrew Barnert pointed out:
There are two ways you could do that.
[The one most people are talking about, which often makes an if-clause visually too heavy ... though Alexander Walters pointed out that "Any excuse to break code out into more functions... is usually the right idea."] if (!obj) { return -1; }
That "otherwise" gets a bit awkward, but I like the idea. Perhaps "braces must not be omitted, and should normally be formatted as follows. ... Where other C styles would permit a braceless one-liner, the expression and braces may be moved to a single line, as follows: " if (x > 5) { y++ } I think that is clearly better, but it may be *too* lightweight for flow control. if (!obj) { return -1; } does work for me, and I think the \n{} may actually be useful for warning that flow control takes a jump. One reason I posted was to point to a specific example already in PEP 7 itself: if (type->tp_dictoffset != 0 && base->tp_dictoffset == 0 && type->tp_dictoffset == b_size && (size_t)t_size == b_size + sizeof(PyObject *)) return 0; /* "Forgive" adding a __dict__ only */ For me, that return is already visually lost, simply because it shares an indentation with the much larger test expression. Would that be better as either: /* "Forgive" adding a __dict__ only */ if (type->tp_dictoffset != 0 && base->tp_dictoffset == 0 && type->tp_dictoffset == b_size && (size_t)t_size == b_size + sizeof(PyObject *)) { return 0; } or: /* "Forgive" adding a __dict__ only */ if (type->tp_dictoffset != 0 && base->tp_dictoffset == 0 && type->tp_dictoffset == b_size && (size_t)t_size == b_size + sizeof(PyObject *)) { return 0; } -jJ -- If there are still threading problems with my replies, please email me with details, so that I can try to resolve them. -jJ

Your wording is much better than mine. And so is your suggestion. Giving people the option of 1 or 3 lines, but not 2, seems a little silly. And, while I rarely use or see your 2-line version in C, I use it quite a bit in C++ (and related languages like D), so it doesn't look at all weird to me. But I'll leave it up to people who only do C (and Python) and/or who are more familiar with the CPython code base to judge.

Let's not switch to either of those options. Visually I much prefer either of these: if (test) { blah; } or if (test) blah; over the versions with '{ blah; }' (regardless of whether it's on the same line as 'if' or on the next line). It looks like the shorter versions are mostly used inside macros, where aesthetics usually go out the door anyways in favor of robustness. Since this discussion is never going to end until someone says "enough", let me just attempt that (though technically it's Brett's call) -- let's go with the strong recommendation to prefer if (test) { blah; } and stop there. On Tue, Jan 19, 2016 at 10:29 AM, Ethan Furman <ethan@stoneleaf.us> wrote:
-- --Guido van Rossum (python.org/~guido)

Here is a proposed update: diff -r 633f51d10a67 pep-0007.txt --- a/pep-0007.txt Mon Jan 18 10:52:57 2016 -0800 +++ b/pep-0007.txt Tue Jan 19 12:11:44 2016 -0800 @@ -75,9 +75,9 @@ } * Code structure: one space between keywords like ``if``, ``for`` and - the following left paren; no spaces inside the paren; braces may be - omitted where C permits but when present, they should be formatted - as shown:: + the following left paren; no spaces inside the paren; braces are + strongly preferred but may be omitted where C permits, and they + should be formatted as shown:: if (mro != NULL) { ... On Tue, 19 Jan 2016 at 11:37 Guido van Rossum <guido@python.org> wrote:

On 19 January 2016 at 20:12, Brett Cannon <brett@python.org> wrote:
This change seems to be accidentally smuggled in, in the guise of a PEP 512 update :) My view is I prefer always using curly brackets in my own code. It is easier to add printf() debugging without making mistakes. I support “strongly preferring” them in the style guide, which is as much as a style guide can do anyway.

The wording is totally fine! You might still want to revert it and re-commit it so it doesn't look like a mistake when reviewing the log. BTW When can we start using git for the peps repo? On Wed, Jan 20, 2016 at 12:18 PM, Brett Cannon <brett@python.org> wrote:
-- --Guido van Rossum (python.org/~guido)

On Wed, 20 Jan 2016 at 14:31 Guido van Rossum <guido@python.org> wrote:
The wording is totally fine! You might still want to revert it and re-commit it so it doesn't look like a mistake when reviewing the log.
Sure thing!
BTW When can we start using git for the peps repo?
Depends on how fast the parts covered in https://www.python.org/dev/peps/pep-0512/#requirements-for-code-only-reposit... and https://www.python.org/dev/peps/pep-0512/#requirements-for-web-related-repos... takes. My hope is before PyCon US (although if we choose to not enforce the CLA on PEP contributions then it could happen even faster). -Brett

On 21 January 2016 at 10:16, Brett Cannon <brett@python.org> wrote:
We should probably mention that in PEP 1 - I wasn't aware of that requirement, so I've never explicitly checked CLA status for folks contributing packaging related PEPs. (And looking at the just-checked-in PEP 513, I apparently have a CLA to chase up...) Regards, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia

I'm still yet to meet a lawyer who trusts "public domain" statements... The CLA will ensure we have enough rights to republish the PEP on p.o or future sites, and it doesn't prevent authors from also releasing the text elsewhere under other terms. Top-posted from my Windows Phone -----Original Message----- From: "Zachary Ware" <zachary.ware+pydev@gmail.com> Sent: 1/21/2016 9:23 To: "Python-Dev" <python-dev@python.org> Subject: Re: [Python-Dev] Update PEP 7 to require curly braces in C On Thu, Jan 21, 2016 at 11:12 AM, Brett Cannon <brett@python.org> wrote:
It's quite surprising to me, since all (as far as I know) PEPs are explicitly public domain. I am very much not a lawyer, though :) -- Zach _______________________________________________ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/steve.dower%40python.org

Zachary Ware writes:
It's quite surprising to me, since all (as far as I know) PEPs are explicitly public domain. I am very much not a lawyer, though :)
The reason for any explicit CLA is CYA: you can't be sure that the contributor DTRTs, so the CLA shows that the contribution was received in good faith. (Some CLAs also include indemnification clauses, such as the contributor warrants that they own the copyright, etc, but all CLAs are useful to show good faith, which may matter big time if the contributor turns out to have a all-IP-yours-is-ours employment contract or the like.) You don't really need to be a lawyer to understand this stuff, you just need to think like a security person. https://www.schneier.com/blog/archives/2008/03/the_security_mi_1.html

On 18 January 2016 at 05:10, Brett Cannon <brett@python.org> wrote:
+1 from me, as I usually add them to code I'm editing anyway (I find it too hard to read otherwise, especially when there's a long series of them). Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia

On 01/17/2016 11:10 AM, Brett Cannon wrote:
Anyone object if I update PEP 7 to remove the optionality of curly braces in PEP 7?
I'm -1. I don't like being forced to add the curly braces when the code is perfectly clear without them. If this was a frequent problem then I'd put up with it, but I can't recall ever making this particular mistake myself or seeing it in CPython source. It seems to me like a fix for a problem we don't have. //arry/

On Mon, Jan 18, 2016 at 9:59 AM, Larry Hastings <larry@hastings.org> wrote:
I'm +1. We don't have that problem yet and the idea Brett brought up is for future changes that will happen. We'll be soon moving to github, which should simplify the process of submitting PRs from other developers interested in making our beautiful language even more awesome. I'm quite positive that with current review process that kind of bug should not happen, but you never know. Having this as a requirement is rather to minimize the risk of potentially having such bugs. I've switched to this style myself good couple years ago and I find it very readable atm. Maciej */arry*

On Mon, Jan 18, 2016 at 10:42 PM, Maciej Szulik <soltysh@gmail.com> wrote:
Rather than forcing people to use braces, wouldn't it be easier to just add a linter to the toolchain that will detect those kinds of problems and reject the commit? Which might be as simple as telling a compiler to treat this warning as an error, and then always use that compiler at some point before committing. (I don't know how many compilers can check this.) I'm -1 on forcing people to use a particular style when a script could do the same job more reliably. ChrisA

On Mon, Jan 18, 2016 at 11:02 PM, Larry Hastings <larry@hastings.org> wrote:
Only in the exact situation that this is trying to prevent, where indentation implies something that braces don't stipulate. From the original Apple bug link: if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0) goto fail; goto fail; Since there are two indented lines after the if and no braces, this should be flagged as an error. If there's only one indented line, braces are optional. ChrisA

On Mon, 18 Jan 2016 at 00:59 Larry Hastings <larry@hastings.org> wrote:
I'm going to assume you mean it is clearer without them, else I don't hear an argument against beyond "I don't wanna". For me, I don't see how:: int x = do_something(); if (x != 10) return NULL; do_some_more(); is any clearer or more readable than:: int x = do_something(); if (x != 10) { return NULL; } do_some_more();
I personally have come close to screwing up like this, but I caught it before I put up a patch. I honestly also prefer the consistency of just always using braces rather than the sometimes rule we have. It isn't like C is a pretty, safe language to begin with, so I would rather be consistent and avoid potential errors.

Brett Cannon wrote:
Maybe not for that piece of code on its own, but the version with braces takes up one more line. Put a few of those together, and you can't fit as much code on the screen. If it makes the difference between being able to see e.g. the whole of a loop at once vs. having to scroll up and down, it could make the code as a whole harder to read. -- Greg

On 1/18/2016 23:27, Greg Ewing wrote:
When someone trying to make this argument in #python for Python code... the response is newlines are free. Almost this entire thread has me confused - the arguments against are kind of hypocritical; You are developing a language with a built in design ethic, and ignoring those ethics while building the implementation itself. Newlines are free, use them Explicit > Implicit - Explicitly scope everything. I am not a core developer, but I just kind of feel its hypocritical to oppose always using brackets for the development of *python*

On 19 January 2016 at 14:40, Alexander Walters <tritium-list@sdamon.com> wrote:
There are two conflicting code aesthetics at work here, and the relevant one for the folks that prefer to avoid braces in C where they can is:
So if we bring *Python* into the comparison, we can see it splits the difference between the two C variants by omitting the closing brace and replacing the opening brace with ":": x = do_something() if (x != 10): return None do_some_more() The additional "cost" of mandatory braces in C is thus more lines containing only a single "}", while the benefit is simply not having to think about the braceless variant as a possible alternative spelling. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia

Alexander Walters wrote:
When someone trying to make this argument in #python for Python code... the response is newlines are free.
Well, I disagree. I very rarely put blank lines in a function in any language, because it makes it hard to scan the code visually and pick out the beginnings of functions. So while they may help readability locally, they hurt it globally. If I find myself needing to put blank lines in a function in order to make it readable, I take it as a sign that the function ought to be split up into smaller functions. -- Greg

Alexander Walters wrote:
I am not a core developer, but I just kind of feel its hypocritical to oppose always using brackets for the development of *python*
If we were being *really* pythonic, we would write all the C code without any braces at all, and feed it through a filter that adds them based on indentation. End of argument. :-) -- Greg

On 01/18/2016 08:40 PM, Alexander Walters wrote:
I am not a core developer, but I just kind of feel its hypocritical to oppose always using brackets for the development of *python*
CPython isn't written in Python, it's written in C. So we use C idioms, which naturally are different from Python idioms. //arry/

On 18 January 2016 at 23:25, Alexander Walters <tritium-list@sdamon.com> wrote:
And in the spirit of safety, which I think is Brett's concern. Yes, there are tools that could catch the Apple bug <https://www.imperialviolet.org/2014/02/22/applebug.html>, by catching incorrect indentation. But if the rule is to *always* use braces, that just feels safer. Even if it offends some people's ability to read everything at one glance (my suggestion: use a smaller font or a larger screen; or refactor the code so it isn't as big a chunk). There's a fairly exhaustive list of styles here: https://en.wikipedia.org/wiki/Indent_style I'm sure that some of these have not been advocated in this thread.

On Jan 17, 2016, at 07:10 PM, Brett Cannon wrote:
Anyone object if I update PEP 7 to remove the optionality of curly braces in PEP 7?
+1 for requiring them everywhere, -1 for a wholesale reformatting of existing code. New code and significant refactoring/rewriting can adopt braces everywhere. Cheers, -Barry
participants (30)
-
Alexander Walters
-
Andrew Barnert
-
Barry Warsaw
-
Brett Cannon
-
Chris Angelico
-
David Malcolm
-
Ethan Furman
-
francismb
-
Georg Brandl
-
Glenn Linderman
-
Greg Ewing
-
Gregory P. Smith
-
Guido van Rossum
-
Jim J. Jewett
-
Larry Hastings
-
M.-A. Lemburg
-
Maciej Szulik
-
Martin Panter
-
MRAB
-
Nick Coghlan
-
Peter Ludemann
-
Robert Collins
-
Serhiy Storchaka
-
Stefan Krah
-
Stephen J. Turnbull
-
Steve Dower
-
Terry Reedy
-
Victor Stinner
-
Yury Selivanov
-
Zachary Ware