[Python-Dev] Update PEP 7 to require curly braces in C

[Brett Cannon]

On Tue, 19 Jan 2016 at 00:48 M.-A. Lemburg <mal at egenix.com> wrote:

> On 19.01.2016 00:20, Brett Cannon wrote:
> > On Sun, 17 Jan 2016 at 11:10 Brett Cannon <brett at python.org> wrote:
> >
> >> 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?
> >>
> >
> > Currently this thread stands at:
>
> Make that:
>
> > +1
> >   Brett
> >   Ethan
> >   Robert
> >   Georg
> >   Nick
> >   Maciej Szulik
> > +0
> >   Guido
> > -0
> >   Serhiy
> > -1
>     MAL
> >   Victor (maybe; didn't specifically vote)
> >   Larry
> >   Stefan
>
> 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.
>

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.


>
> 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".
>

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.


>
> 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 ;-)
>

That's fine. I also want format consistency by always using braces.


>
> 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 :-)
>

Well, I'm dropping out of this discussion because I have enough on my plate
with the GitHub migration than to keep fighting this.


>
> 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.
>

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/python-dev/attachments/20160119/307ba4e8/attachment.html>