Mailman 3 Re: [Python-Dev] cpython: fix compiler warning by implementing this more cleverly - Python-Dev

newer
Python 3.4 Release Manager

Re: [Python-Dev] cpython: fix compiler warning by implementing this more cleverly

older
Re: [Python-Dev] cpython: fix...

Antoine Pitrou

23 Nov 2011 23 Nov '11

3:02 a.m.

On Tue, 22 Nov 2011 21:29:43 +0100 benjamin.peterson wrote:

...

http://hg.python.org/cpython/rev/77ab830930ae changeset: 73697:77ab830930ae user: Benjamin Peterson date: Tue Nov 22 15:29:32 2011 -0500 summary: fix compiler warning by implementing this more cleverly

You mean "more obscurely"? Obfuscating the original intent in order to disable a compiler warning doesn't seem very wise to me. Regards Antoine.

...

files: Objects/unicodeobject.c | 7 +------ 1 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c --- a/Objects/unicodeobject.c +++ b/Objects/unicodeobject.c @@ -6164,12 +6164,7 @@ kind = PyUnicode_KIND(unicode); data = PyUnicode_DATA(unicode); len = PyUnicode_GET_LENGTH(unicode); - - switch(kind) { - case PyUnicode_1BYTE_KIND: expandsize = 4; break; - case PyUnicode_2BYTE_KIND: expandsize = 6; break; - case PyUnicode_4BYTE_KIND: expandsize = 10; break; - } + expandsize = kind * 2 + 2;

if (len > PY_SSIZE_T_MAX / expandsize) return PyErr_NoMemory();

-- Repository URL: http://hg.python.org/cpython

Show replies by thread

Benjamin Peterson

23 Nov 23 Nov

3:12 a.m.

New subject: cpython: fix compiler warning by implementing this more cleverly

2011/11/22 Antoine Pitrou :

...

On Tue, 22 Nov 2011 21:29:43 +0100 benjamin.peterson wrote:

...
http://hg.python.org/cpython/rev/77ab830930ae changeset: 73697:77ab830930ae user: Benjamin Peterson date: Tue Nov 22 15:29:32 2011 -0500 summary: fix compiler warning by implementing this more cleverly

You mean "more obscurely"? Obfuscating the original intent in order to disable a compiler warning doesn't seem very wise to me.

Well, I think it makes sense that the kind tells you how many bytes are in it. -- Regards, Benjamin

Antoine Pitrou

3:13 a.m.

New subject: cpython: fix compiler warning by implementing this more cleverly

On Tue, 22 Nov 2011 16:42:35 -0500 Benjamin Peterson wrote:

...

2011/11/22 Antoine Pitrou :

...
On Tue, 22 Nov 2011 21:29:43 +0100 benjamin.peterson wrote:

...
http://hg.python.org/cpython/rev/77ab830930ae changeset: 73697:77ab830930ae user: Benjamin Peterson date: Tue Nov 22 15:29:32 2011 -0500 summary: fix compiler warning by implementing this more cleverly

You mean "more obscurely"? Obfuscating the original intent in order to disable a compiler warning doesn't seem very wise to me.

Well, I think it makes sense that the kind tells you how many bytes are in it.

Yes, but "kind * 2 + 2" looks like a magical formula, while the explicit switch let you check mentally that each estimate was indeed correct. Regards Antoine.

Benjamin Peterson

6:12 a.m.

New subject: cpython: fix compiler warning by implementing this more cleverly

2011/11/22 Antoine Pitrou :

...

On Tue, 22 Nov 2011 16:42:35 -0500 Benjamin Peterson wrote:

...
2011/11/22 Antoine Pitrou :

...
On Tue, 22 Nov 2011 21:29:43 +0100 benjamin.peterson wrote:

...
http://hg.python.org/cpython/rev/77ab830930ae changeset: 73697:77ab830930ae user: Benjamin Peterson date: Tue Nov 22 15:29:32 2011 -0500 summary: fix compiler warning by implementing this more cleverly

You mean "more obscurely"? Obfuscating the original intent in order to disable a compiler warning doesn't seem very wise to me.

Well, I think it makes sense that the kind tells you how many bytes are in it.

Yes, but "kind * 2 + 2" looks like a magical formula, while the explicit switch let you check mentally that each estimate was indeed correct.

I don't see how it's more magic than hardcoding 4, 6, and 10. Don't you have to mentally check that those are correct? -- Regards, Benjamin

Antoine Pitrou

6:16 a.m.

New subject: cpython: fix compiler warning by implementing this more cleverly

On Tue, 22 Nov 2011 19:42:24 -0500 Benjamin Peterson wrote:

...

2011/11/22 Antoine Pitrou :

...
On Tue, 22 Nov 2011 16:42:35 -0500 Benjamin Peterson wrote:

...
2011/11/22 Antoine Pitrou :

...
On Tue, 22 Nov 2011 21:29:43 +0100 benjamin.peterson wrote:

...
http://hg.python.org/cpython/rev/77ab830930ae changeset: 73697:77ab830930ae user: Benjamin Peterson date: Tue Nov 22 15:29:32 2011 -0500 summary: fix compiler warning by implementing this more cleverly

You mean "more obscurely"? Obfuscating the original intent in order to disable a compiler warning doesn't seem very wise to me.

Well, I think it makes sense that the kind tells you how many bytes are in it.

Yes, but "kind * 2 + 2" looks like a magical formula, while the explicit switch let you check mentally that each estimate was indeed correct.

I don't see how it's more magic than hardcoding 4, 6, and 10. Don't you have to mentally check that those are correct?

I don't know. Perhaps I'm saying that because I *have* already done the mental check :) Regards Antoine.

Terry Reedy

12:19 p.m.

New subject: cpython: fix compiler warning by implementing this more cleverly

On 11/22/2011 7:42 PM, Benjamin Peterson wrote:

...

2011/11/22 Antoine Pitrou:

...
On Tue, 22 Nov 2011 16:42:35 -0500 Benjamin Peterson wrote:

...
2011/11/22 Antoine Pitrou:

...
On Tue, 22 Nov 2011 21:29:43 +0100 benjamin.peterson wrote:

...
http://hg.python.org/cpython/rev/77ab830930ae changeset: 73697:77ab830930ae user: Benjamin Peterson date: Tue Nov 22 15:29:32 2011 -0500 summary: fix compiler warning by implementing this more cleverly

You mean "more obscurely"? Obfuscating the original intent in order to disable a compiler warning doesn't seem very wise to me.

Well, I think it makes sense that the kind tells you how many bytes are in it.

Yes, but "kind * 2 + 2" looks like a magical formula, while the explicit switch let you check mentally that each estimate was indeed correct.

I don't see how it's more magic than hardcoding 4, 6, and 10. Don't you have to mentally check that those are correct?

I personally strongly prefer the one-line formula to the hardcoded magic numbers calculated from the formula. I find it much more readable. To me, the only justification for the switch would be if there is a serious worry about the kind being changed to something other than 1, 2, or 4. But the fact that this is checked with an assert that can be optimized away negates that. The one-liner could be followed by assert(kind==1 || kind==2 || kind==4) which would also serve to remind the reader of the possibilities. You could even follow the formula with /* 4, 6, or 10 */ I think you reverted too soon. -- Terry Jan Reedy

Nick Coghlan

12:37 p.m.

New subject: cpython: fix compiler warning by implementing this more cleverly

On Wed, Nov 23, 2011 at 4:49 PM, Terry Reedy wrote:

...

I personally strongly prefer the one-line formula to the hardcoded magic numbers calculated from the formula. I find it much more readable. To me, the only justification for the switch would be if there is a serious worry about the kind being changed to something other than 1, 2, or 4. But the fact that this is checked with an assert that can be optimized away negates that. The one-liner could be followed by assert(kind==1 || kind==2 || kind==4) which would also serve to remind the reader of the possibilities. You could even follow the formula with /* 4, 6, or 10 */ I think you reverted too soon.

+1 to what Terry said here, although I would add a genuinely explanatory comment that gives the calculation meaning: /* For each character, allow for "\U" prefix and 2 hex digits per byte */ expandsize = 2 + 2 * kind; Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia

Victor Stinner

3:10 p.m.

New subject: cpython: fix compiler warning by implementing this more cleverly

Le Mercredi 23 Novembre 2011 01:49:28 Terry Reedy a écrit :

...

The one-liner could be followed by assert(kind==1 || kind==2 || kind==4) which would also serve to remind the reader of the possibilities.

For a ready string, kind must be 1, 2 or 4. We might rename "kind" to "charsize" because its value changed from 1, 2, 3 to 1, 2, 4 (to make it easy to compute the size of a string: length * kind). You are not supposed to see the secret kind==0 case. This value is only used for string created by _PyUnicode_New() and not ready yet: str = _PyUnicode_New() /* use str */ assert(PyUnicode_KIND(str) == 0); if (PyUnicode_READY(str) < 0) /* error */ assert(PyUnicode_KIND(str) != 0); /* kind is 1, 2, 4 */ Thanks to the effort of t0rsten, Martin and me, quite all functions use the new API (PyUnicode_New). For example, PyUnicode_AsRawUnicodeEscapeString() starts by ensuring that the string is ready. For your information, PyUnicode_KIND() fails with an assertion error in debug mode if the string is not ready. -- I don't have an opinion about the one-liner vs the switch :-) But if you want to fix compiler warnings, you should use "enum PyUnicode_Kind" type and PyUnicode_WCHAR_KIND should be removed from the enum. Victor

4532

Age (days ago)

4533

Last active (days ago)

List overview

Download

7 comments

5 participants

participants (5)

Antoine Pitrou
Benjamin Peterson
Nick Coghlan
Terry Reedy
Victor Stinner

Re: [Python-Dev] cpython: fix compiler warning by implementing this more cleverly

tags

participants (5)