classification
Title: Have -b warn when directly comparing ints and bytes
Type: enhancement Stage: resolved
Components: Interpreter Core Versions: Python 3.5, Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: Claudiu.Popa, berker.peksag, brett.cannon, larry, martin.panter, paul.moore, python-dev, serhiy.storchaka, vstinner
Priority: release blocker Keywords: patch

Created on 2015-03-16 17:05 by brett.cannon, last changed 2015-03-20 14:59 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
bytes_to_int_compare.patch serhiy.storchaka, 2015-03-16 19:03 review
bytes_to_int_compare_2.patch serhiy.storchaka, 2015-03-20 10:09 review
Messages (10)
msg238228 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2015-03-16 17:05
To help writing Python 2/3 code the -b flag should switch on a warning when comparing an int to a bytes object in Python 2. This will help when someone writes something like `b'abcd'[2] == b'c'` and it always returns False thanks to the indexing returning 99 in Python 3.
msg238236 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-16 18:04
6 tests failed:
    test_buffer test_poplib test_quopri test_smtpd test_sunau
    test_tokenize

And all of them look as bugs.
msg238237 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-16 19:03
Here is a patch that adds required feature and fixes existing bugs in the stdlib and tests.

No one of failed tests was false positive.
msg238261 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2015-03-17 05:01
> [...] fixes existing bugs in the stdlib and tests.

These changes should probably be backported to 3.4.
msg238501 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2015-03-19 10:54
LGTM, just one minor comment in review.
msg238626 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-03-20 09:14
I didn't understand the issue.

$ python2
>>> b'A'[0] == 65
False

$ python3
Python 3.4.1 (default, Nov  3 2014, 14:38:10) 
>>> b'A'[0] == 65
True

Oh ok, now I get it: Python 2 and Python 3 behaves differently when comparing a string of a single byte and an integer.

Ok to raise a warning when -b command line option is used.
msg238636 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-20 10:09
@Brett: Yes, the patch handles `42 == b'*'` as well.

@Victor: The problem in such code:

x = b'Array'
x[0] == b'A'


Added explicit tests for bytes on the right side of the comparison and replaced b'.'[0] to ord(b'.').
msg238637 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-03-20 10:16
> This will help when someone writes something like `b'abcd'[2] == b'c'`

What if someone writes line[-1] == 0 and line is a Unicode string? Should we emit a warning?

I patched locally PyUnicode_RichCompare() to emit a warning. Hum, there are *many* warnings in argparse, http.client, and many other modules. I don't think that it's a good idea. It looks common to use a string for a sentinel (ex: state = "UNKNOWN") and then store an int (ex: state = 0). So comparison need to check the type (ex: isinstance(state, str) and state == "UNKNOWN") which is verbose and annoying.

So no, we should not emit a warning :-)
msg238665 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2015-03-20 13:16
I did one final pass on the patch and only had wording comments, so tweak those and it LGTM. Assigned to Serhiy since it's obviously his patch. =)

Only other thing is to either open up a separate bug or at least apply the fixes to the stdlib in Python 3.4 as well as 3.5.
msg238687 - (view) Author: Roundup Robot (python-dev) Date: 2015-03-20 14:56
New changeset 104c55bc2276 by Serhiy Storchaka in branch '3.4':
Issue #23681: Fixed Python 2 to 3 poring bugs.
https://hg.python.org/cpython/rev/104c55bc2276

New changeset 817f1f47824c by Serhiy Storchaka in branch 'default':
Issue #23681: Fixed Python 2 to 3 poring bugs.
https://hg.python.org/cpython/rev/817f1f47824c

New changeset 68e9cf0ae93d by Serhiy Storchaka in branch 'default':
Issue #23681: The -b option now affects comparisons of bytes with int.
https://hg.python.org/cpython/rev/68e9cf0ae93d
History
Date User Action Args
2015-03-20 14:59:45serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: commit review -> resolved
2015-03-20 14:56:28python-devsetnosy: + python-dev
messages: + msg238687
2015-03-20 13:16:50brett.cannonsetversions: + Python 3.4
nosy: + larry

messages: + msg238665

assignee: brett.cannon -> serhiy.storchaka
stage: patch review -> commit review
2015-03-20 10:16:24vstinnersetmessages: + msg238637
2015-03-20 10:09:08serhiy.storchakasetfiles: + bytes_to_int_compare_2.patch

messages: + msg238636
2015-03-20 09:14:25vstinnersetnosy: + vstinner
messages: + msg238626
2015-03-20 02:58:54martin.pantersetnosy: + martin.panter
2015-03-19 10:54:41paul.mooresetnosy: + paul.moore
messages: + msg238501
2015-03-17 05:01:19berker.peksagsetnosy: + berker.peksag
messages: + msg238261
2015-03-16 19:29:17Claudiu.Popasetnosy: + Claudiu.Popa
2015-03-16 19:03:34serhiy.storchakasetfiles: + bytes_to_int_compare.patch
keywords: + patch
messages: + msg238237

stage: test needed -> patch review
2015-03-16 18:04:00serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg238236
2015-03-16 17:06:10brett.cannonsetpriority: deferred blocker -> release blocker
2015-03-16 17:06:00brett.cannonsetpriority: normal -> deferred blocker
2015-03-16 17:05:54brett.cannonsetversions: + Python 3.5, - Python 2.7
2015-03-16 17:05:30brett.cannoncreate