Modules/binascii.c: simplify expressions

[nikai]

BPO	10324
Nosy	@terryjreedy, @orsenthil
Files	python-binascii.diff: patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2010-11-09.10:02:57.051>
created_at = <Date 2010-11-05.12:31:20.723>
labels = ['extension-modules', 'performance']
title = 'Modules/binascii.c: simplify expressions'
updated_at = <Date 2010-11-09.10:02:57.049>
user = 'https://bugs.python.org/nikai'

bugs.python.org fields:

activity = <Date 2010-11-09.10:02:57.049>
actor = 'orsenthil'
assignee = 'none'
closed = True
closed_date = <Date 2010-11-09.10:02:57.051>
closer = 'orsenthil'
components = ['Extension Modules']
creation = <Date 2010-11-05.12:31:20.723>
creator = 'nikai'
dependencies = []
files = ['19504']
hgrepos = []
issue_num = 10324
keywords = ['patch']
message_count = 6.0
messages = ['120490', '120551', '120565', '120573', '120599', '120853']
nosy_count = 3.0
nosy_names = ['terry.reedy', 'orsenthil', 'nikai']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue10324'
versions = ['Python 3.2']

[nikai]

Hi there!

I noticed two expressions that can be simplified like:
(a || (!a && b)) => (a || b)

Best regards,
Nicolas Kaiser
---

--- a/Modules/binascii.c	2010-11-05 13:21:22.075303326 +0100
+++ b/Modules/binascii.c	2010-11-05 13:24:16.986174756 +0100
@@ -1337,8 +1337,7 @@ binascii_b2a_qp (PyObject *self, PyObjec
             ((data[in] == '\t' || data[in] == ' ') && (in + 1 == datalen)) ||
             ((data[in] < 33) &&
              (data[in] != '\r') && (data[in] != '\n') &&
-             (quotetabs ||
-            (!quotetabs && ((data[in] != '\t') && (data[in] != ' '))))))
+             (quotetabs || ((data[in] != '\t') && (data[in] != ' ')))))
         {
             if ((linelen + 3) >= MAXLINESIZE) {
                 linelen = 0;
@@ -1410,8 +1409,7 @@ binascii_b2a_qp (PyObject *self, PyObjec
             ((data[in] == '\t' || data[in] == ' ') && (in + 1 == datalen)) ||
             ((data[in] < 33) &&
              (data[in] != '\r') && (data[in] != '\n') &&
-             (quotetabs ||
-            (!quotetabs && ((data[in] != '\t') && (data[in] != ' '))))))
+             (quotetabs || ((data[in] != '\t') && (data[in] != ' ')))))
         {
             if ((linelen + 3 )>= MAXLINESIZE) {
                 odata[out++] = '=';

[terryjreedy]

As near as I can tell, since && and || are logical rather than bitwise, and since the variable reference 'quotetabs' has no side effect, you are correct. Have you run the unittest on a patched build?

[nikai]

That's ./Lib/test/test_unittest.py?

With patched builds of Python 2.6.5 and 3.1.2:

----------------------------------------------------------------------
Ran 126 tests in 0.015s

OK

----------------------------------------------------------------------
Ran 187 tests in 0.054s

OK

[terryjreedy]

test_binascii.py

[nikai]

Sorry, found it - with patched builds of Python 2.6.5 and 3.1.2:

python2.6 test_binascii.py
test_base64invalid (main.BinASCIITest) ... ok
test_base64valid (main.BinASCIITest) ... ok
test_crc32 (main.BinASCIITest) ... ok
test_empty_string (main.BinASCIITest) ... ok
test_exceptions (main.BinASCIITest) ... ok
test_functions (main.BinASCIITest) ... ok
test_hex (main.BinASCIITest) ... ok
test_qp (main.BinASCIITest) ... ok
test_uu (main.BinASCIITest) ... ok

----------------------------------------------------------------------
Ran 9 tests in 0.002s

OK

python3.1 test_binascii.py
test_base64invalid (main.BinASCIITest) ... ok
test_base64valid (main.BinASCIITest) ... ok
test_crc32 (main.BinASCIITest) ... ok
test_empty_string (main.BinASCIITest) ... ok
test_exceptions (main.BinASCIITest) ... ok
test_functions (main.BinASCIITest) ... ok
test_hex (main.BinASCIITest) ... ok
test_no_binary_strings (main.BinASCIITest) ... ok
test_qp (main.BinASCIITest) ... ok
test_uu (main.BinASCIITest) ... ok

----------------------------------------------------------------------
Ran 10 tests in 0.006s

OK

[orsenthil]

At first, I was worried if this simplification would cause any harm to readability of the algorithm. Fortunately, it didn't.
Committed in r86357.