date:20160119

Re: [Python-Dev] PEP 510: Specialize functions with guards

2016-01-19 Thread Victor Stinner

Oh, I think that the PEP 510 lacks two functions to:

* remove a specific specialized code
* remove all specialized code

Victor
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

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

2016-01-19 Thread M.-A. Lemburg

On 19.01.2016 00:20, Brett Cannon wrote:
> On Sun, 17 Jan 2016 at 11:10 Brett Cannon  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.

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)
>>> Python Projects, Coaching and Consulting ...  http://www.egenix.com/
>>> Python Database Interfaces ...   http://products.egenix.com/
>>> Plone/Zope Database Interfaces ...   http://zope.egenix.com/

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

___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

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

2016-01-19 Thread Maciej Szulik

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

> On 19.01.2016 00:20, Brett Cannon wrote:
> > On Sun, 17 Jan 2016 at 11:10 Brett Cannon  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.
>
> 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)
> >>> Python Projects, Coaching and Consulting ...  http://www.egenix.com/
> >>> Python Database Interfaces ...   http://products.egenix.com/
> >>> Plone/Zope Database Interfaces ...   http://zope.egenix.com/
> 
>
> ::: 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/
>
> ___
> Python-Dev mailing list
> [email protected]
> https://mail.python.org/mailman/listinfo/python-dev
> Unsubscribe:
> https://mail.python.org/mailman/options/python-dev/soltysh%40gmail.com
>
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

[Python-Dev] Reference cycle on the module dict (globals())

2016-01-19 Thread Victor Stinner

Hi,

While working on my FAT Python optimizer project, I found an annoying
bug in my code. When at least one guard is created with a reference to
the global namespace (globals(), the module dictionary), objects of
the module are no more removed at exit.

Example:
---
import sys

class MessageAtExit:
def __del__(self):
print('__del__ called')

# display a message at exit, when message_at_exit is removed
message_at_exit = MessageAtExit()

# create a reference cycle:
# module -> module dict -> Guard -> module dict
guard = sys.Guard(globals())
---
(the code is adapted from a test of test_gc)

Apply attached patch to Python 3.6 to get the sys.Guard object. It's a
minimalist object to keep a strong reference to an object.

I expected the garbage collector to break such (simple?) reference cycle.

The Guard object implements a traverse module, but it is never called.

Did I miss something obvious, or is it a known issue of the garbage
collector on modules?

Victor
diff -r 1f003062d830 Python/sysmodule.c
--- a/Python/sysmodule.c	Tue Jan 19 08:50:56 2016 +0100
+++ b/Python/sysmodule.c	Tue Jan 19 10:40:31 2016 +0100
@@ -1714,6 +1714,83 @@ static struct PyModuleDef sysmodule = {
 NULL
 };
 
+typedef struct {
+PyObject ob_base;
+PyObject *ref;
+} PyFuncGuardObject;
+
+static PyObject *
+guard_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
+{
+PyObject *ref;
+PyObject *op;
+PyFuncGuardObject *self;
+
+if (!PyArg_ParseTuple(args, "O", &ref))
+return NULL;
+
+assert(type != NULL && type->tp_alloc != NULL);
+op = type->tp_alloc(type, 0);
+if (!op)
+return NULL;
+
+self = (PyFuncGuardObject *)op;
+Py_INCREF(ref);
+self->ref = ref;
+
+return op;
+}
+
+static int
+guard_traverse(PyFuncGuardObject *guard, visitproc visit, void *arg)
+{
+printf("guard_traverse(%p, %p, %p)\n", guard, visit, arg);
+Py_VISIT(guard->ref);
+return 0;
+}
+
+PyTypeObject PyFuncGuard_Type = {
+PyVarObject_HEAD_INIT(&PyType_Type, 0)
+"Guard",
+sizeof(PyFuncGuardObject),
+0,
+0,  /* tp_dealloc */
+0,  /* tp_print */
+0,  /* tp_getattr */
+0,  /* tp_setattr */
+0,  /* tp_reserved */
+0,  /* tp_repr */
+0,  /* tp_as_number */
+0,  /* tp_as_sequence */
+0,  /* tp_as_mapping */
+0,  /* tp_hash */
+0, /* tp_call */
+0,  /* tp_str */
+0,  /* tp_getattro */
+0,  /* tp_setattro */
+0,  /* tp_as_buffer */
+Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,   /* tp_flags */
+0,  /* tp_doc */
+(traverseproc)guard_traverse,  /* tp_traverse */
+0,  /* tp_clear */
+0,  /* tp_richcompare */
+0,  /* tp_weaklistoffset */
+0,  /* tp_iter */
+0,  /* tp_iternext */
+0,  /* tp_methods */
+0,  /* tp_members */
+0,  /* tp_getset */
+0,  /* tp_base */
+0,  /* tp_dict */
+0,  /* tp_descr_get */
+0,  /* tp_descr_set */
+0,  /* tp_dictoffset */
+0,  /* tp_init */
+0,  /* tp_alloc */
+guard_new,  /* tp_new */
+0,  /* tp_free */
+};
+
 PyObject *
 _PySys_Init(void)
 {
@@ -1901,6 +1978,12 @@ PyObject *
 SET_SYS_FROM_STRING("thread_info", PyThread_GetInfo());
 #endif
 
+if (PyType_Ready(&PyFuncGuard_Type) < 0)
+return NULL;
+
+Py_INCREF((PyObject *)&PyFuncGuard_Type);
+SET_SYS_FROM_STRING("Guard", (PyObject *)&PyFuncGuard_Type);
+
 #undef SET_SYS_FROM_STRING
 #undef SET_SYS_FROM_STRING_BORROW
 if (PyErr_Occurred())
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/

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

2016-01-19 Thread Stefan Krah

M.-A. Lemburg  egenix.com> writes: 
> > 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

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

___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Reference cycle on the module dict (globals())

2016-01-19 Thread Petr Viktorin

On 01/19/2016 10:42 AM, Victor Stinner wrote:
> Hi,
> 
> While working on my FAT Python optimizer project, I found an annoying
> bug in my code. When at least one guard is created with a reference to
> the global namespace (globals(), the module dictionary), objects of
> the module are no more removed at exit.
> 
> Example:
> ---
> import sys
> 
> class MessageAtExit:
> def __del__(self):
> print('__del__ called')
> 
> # display a message at exit, when message_at_exit is removed
> message_at_exit = MessageAtExit()
> 
> # create a reference cycle:
> # module -> module dict -> Guard -> module dict
> guard = sys.Guard(globals())
> ---
> (the code is adapted from a test of test_gc)
> 
> Apply attached patch to Python 3.6 to get the sys.Guard object. It's a
> minimalist object to keep a strong reference to an object.
> 
> I expected the garbage collector to break such (simple?) reference cycle.
> 
> The Guard object implements a traverse module, but it is never called.
> 
> Did I miss something obvious, or is it a known issue of the garbage
> collector on modules?

The default type flags are for objects that don't store references.
Since you're creating a mutable container, you need to set
Py_TPFLAGS_HAVE_GC. See https://docs.python.org/3/c-api/gcsupport.html
for all the details.
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] _PyThreadState_Current

2016-01-19 Thread Victor Stinner

Since it's a regression introduced in Python 3.5.1, I propose to
introduce a new private function _PyThreadState_FastGet() to
reintroduce the feature:
https://bugs.python.org/issue26154

Using afunction instead of using directly the variable hides how
atomic variables are implemented and so avoid compiler issues.

Victor

2016-01-18 21:18 GMT+01:00 Maciej Fijalkowski :
> Hi
>
> change in between 3.5.0 and 3.5.1 (hiding _PyThreadState_Current and
> pyatomic.h) broke vmprof. The problem is that as a profile, vmprof can
> really encounter _PyThreadState_Current being null, while crashing an
> interpreter is a bit not ideal in this case.
>
> Any chance, a) _PyThreadState_Current can be restored in visibility?
> b) can I get a better API to get it in case it can be NULL, but also
> in 3.5 (since it works in 3.5.0 and breaks in 3.5.1)
>
> Cheers,
> fijal
> ___
> Python-Dev mailing list
> [email protected]
> https://mail.python.org/mailman/listinfo/python-dev
> Unsubscribe: 
> https://mail.python.org/mailman/options/python-dev/victor.stinner%40gmail.com
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Reference cycle on the module dict (globals())

2016-01-19 Thread Victor Stinner

Hi,

2016-01-19 11:39 GMT+01:00 Petr Viktorin :
>> Did I miss something obvious, or is it a known issue of the garbage
>> collector on modules?
>
> The default type flags are for objects that don't store references.
> Since you're creating a mutable container, you need to set
> Py_TPFLAGS_HAVE_GC. See https://docs.python.org/3/c-api/gcsupport.html
> for all the details.

Ok, so I missed this important flag :-) Thanks!

I had to fight against the C API to fix all my bugs, but now it works
well: a guard keeps a strong reference to the global namespace, but
objects are still destroyed when the module is unloaded!

FYI I updated my PEP 510 patch to track guards with the garbage
collector, and my fat project to fix bugs related to GC:

- https://bugs.python.org/issue26098
- https://github.com/haypo/fat

Victor
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

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

2016-01-19 Thread David Malcolm

On Mon, 2016-01-18 at 19:18 -0500, Terry Reedy wrote:
> On 1/18/2016 6:20 PM, Brett Cannon wrote:
> >
> >
> > On Sun, 17 Jan 2016 at 11:10 Brett Cannon  > > 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:
> >
> > +1
> >Brett
> >Ethan
> >Robert
> >Georg
> >Nick
> >Maciej Szulik
> > +0
> >Guido
> > -0
> >Serhiy
> >MAL
> > -1
> >Victor (maybe; didn't specifically vote)
> >Larry
> >Stefan
> 
> 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.

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

___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

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

2016-01-19 Thread Yury Selivanov




On 2016-01-18 6:20 PM, Brett Cannon wrote:



On Sun, 17 Jan 2016 at 11:10 Brett Cannon > 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:

+1
  Brett
  Ethan
  Robert
  Georg
  Nick
  Maciej Szulik



I guess you forgot to count Barry and me as +1s.

Yury

___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

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

2016-01-19 Thread Jim J. Jewett

> On Jan 17, 2016, at 11:10, Brett Cannon  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;
}

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

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
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

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

2016-01-19 Thread Brett Cannon

On Tue, 19 Jan 2016 at 00:48 M.-A. Lemburg  wrote:

> On 19.01.2016 00:20, Brett Cannon wrote:
> > On Sun, 17 Jan 2016 at 11:10 Brett Cannon  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
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

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

2016-01-19 Thread Andrew Barnert via Python-Dev

> On Jan 19, 2016, at 08:56, Jim J. Jewett  wrote:
> 
> On Mon Jan 18 03:39:42 EST 2016, Andrew Barnert pointed out:
>> 
>> 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"
> 
> 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.

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.
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

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

2016-01-19 Thread Ethan Furman


On 01/19/2016 08:56 AM, Jim J. Jewett wrote:


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.


Either of those two, with preference for the second on multiline ifs, 
seems quite readable to me.


--
~Ethan~

___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

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

2016-01-19 Thread Guido van Rossum

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

> On 01/19/2016 08:56 AM, Jim J. Jewett wrote:
>
> 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.
>>
>
> Either of those two, with preference for the second on multiline ifs,
> seems quite readable to me.
>
> --
> ~Ethan~
>
>
> ___
> Python-Dev mailing list
> [email protected]
> https://mail.python.org/mailman/listinfo/python-dev
> Unsubscribe:
> https://mail.python.org/mailman/options/python-dev/guido%40python.org
>



-- 
--Guido van Rossum (python.org/~guido)
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

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

2016-01-19 Thread Brett Cannon

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

> 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  wrote:
>
>> On 01/19/2016 08:56 AM, Jim J. Jewett wrote:
>>
>> 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.
>>>
>>
>> Either of those two, with preference for the second on multiline ifs,
>> seems quite readable to me.
>>
>> --
>> ~Ethan~
>
>
>>
>> ___
>> Python-Dev mailing list
>> [email protected]
>> https://mail.python.org/mailman/listinfo/python-dev
>>
> Unsubscribe:
>> https://mail.python.org/mailman/options/python-dev/guido%40python.org
>>
>
>
>
> --
> --Guido van Rossum (python.org/~guido)
> ___
> Python-Dev mailing list
> [email protected]
> https://mail.python.org/mailman/listinfo/python-dev
> Unsubscribe:
> https://mail.python.org/mailman/options/python-dev/brett%40python.org
>
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

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

2016-01-19 Thread francismb

Hi Brett,

On 01/19/2016 12:20 AM, Brett Cannon wrote:
> On Sun, 17 Jan 2016 at 11:10 Brett Cannon  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?
>>
> 

What about about a code formatter bot ? (new workflow). If one just
could agree, then those reviews should just disappear (?).

Regards,
francis
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

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

2016-01-19 Thread Guido van Rossum

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

> Hi Brett,
>
> On 01/19/2016 12:20 AM, Brett Cannon wrote:
> > On Sun, 17 Jan 2016 at 11:10 Brett Cannon  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?
> >>
> >
>
> What about about a code formatter bot ? (new workflow). If one just
> could agree, then those reviews should just disappear (?).
>
> Regards,
> francis
> ___
> Python-Dev mailing list
> [email protected]
> https://mail.python.org/mailman/listinfo/python-dev
> Unsubscribe:
> https://mail.python.org/mailman/options/python-dev/guido%40python.org
>



-- 
--Guido van Rossum (python.org/~guido)
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

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

2016-01-19 Thread Gregory P. Smith

On Sun, Jan 17, 2016 at 11:12 AM Brett Cannon  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?
>

+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
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

[Python-Dev] Code formatter bot

2016-01-19 Thread francismb

Dear Core-Devs,
what's your opinion about a code-formatter bot for cpython.
Pros, Cons, where could be applicable (new commits, new workflow, it
doesn't make sense), ...


- At least it should follow PEP 7 ;-)
- ...


Thanks in advance,
francis

___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Code formatter bot

2016-01-19 Thread Sven R. Kunze


Not a core dev, but I would definitely recommend using them.

Best,
Sven

On 19.01.2016 21:59, francismb wrote:

Dear Core-Devs,
what's your opinion about a code-formatter bot for cpython.
Pros, Cons, where could be applicable (new commits, new workflow, it
doesn't make sense), ...


- At least it should follow PEP 7 ;-)
- ...


Thanks in advance,
francis

___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/srkunze%40mail.de


___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Code formatter bot

2016-01-19 Thread Senthil Kumaran

On Tue, Jan 19, 2016 at 12:59 PM, francismb  wrote:

> Pros, Cons, where could be applicable (new commits, new workflow, it
> doesn't make sense), ...
>

-1. formatting should be done by humans (with the help of tools) before
committing.
It should not be left to a robot to make automatic changes.

We already some pre-commit hooks which do basic checks. If anything more
automated is desirable then enhancement to the pre-commit hooks could be
the place to look for.
As far as I know, none of the core devs have expressed any complaints the
pre-commit hooks.

-- 
Senthil
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

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

2016-01-19 Thread Martin Panter

On 19 January 2016 at 20:12, Brett Cannon  wrote:
> 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) {
>...

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.
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Code formatter bot

2016-01-19 Thread Gregory P. Smith

Indeed, automated code formatting is a good thing. But a bot is the wrong
approach. You want a code formatting checker as a potential pre-submit hook
(like we have had for white space issues in the past), but until you have
super high confidence in it you need to make sure it is not a blocker for a
commit or push, just a default check that can be explicitly skipped for the
infrequent cases where it is wrong. It's a workflow thing. Devs should have
their editor setup to auto-run the correct formatter or easily run it if
not automatic.

-gps

On Tue, Jan 19, 2016 at 4:29 PM Senthil Kumaran  wrote:

>
> On Tue, Jan 19, 2016 at 12:59 PM, francismb  wrote:
>
>> Pros, Cons, where could be applicable (new commits, new workflow, it
>> doesn't make sense), ...
>>
>
> -1. formatting should be done by humans (with the help of tools) before
> committing.
> It should not be left to a robot to make automatic changes.
>
> We already some pre-commit hooks which do basic checks. If anything more
> automated is desirable then enhancement to the pre-commit hooks could be
> the place to look for.
> As far as I know, none of the core devs have expressed any complaints the
> pre-commit hooks.
>
>
> --
> Senthil
>
>
> ___
> Python-Dev mailing list
> [email protected]
> https://mail.python.org/mailman/listinfo/python-dev
> Unsubscribe:
> https://mail.python.org/mailman/options/python-dev/greg%40krypto.org
>
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] PEP 510: Specialize functions with guards

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

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

[Python-Dev] Reference cycle on the module dict (globals())

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

Re: [Python-Dev] Reference cycle on the module dict (globals())

Re: [Python-Dev] _PyThreadState_Current

Re: [Python-Dev] Reference cycle on the module dict (globals())

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

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

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

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

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

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

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

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

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

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

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

[Python-Dev] Code formatter bot

Re: [Python-Dev] Code formatter bot

Re: [Python-Dev] Code formatter bot

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

Re: [Python-Dev] Code formatter bot

24 matches

Site Navigation

Mail list logo

Footer information