Terry J. Reedy <tjre...@udel.edu> added the comment:

Since we have defined an enum, I think we should use it consistently. (Am I 
correct in thinking it was added after the initial patches.) So I did a sanity 
check of -/+ lines for perhaps unintended changes. The only things I found to 
comment on, mostly somewhat picky, would *not* trigger test failures. 

===
-            int kind = PyUnicode_KIND(output);
+             kind = PyUnicode_KIND(output);
extra space added misaligns text (for humans)

===
+    default: assert(0);

I presume this is standard elsewhere for switches and just got omitted here.

-    }
-    assert(0);
-    return -1;
+    default:
+        assert(0);
+        return -1;
+    }
(two places)

Since assert(0) always fails, return can never happen (and was not added above. 
So I would think remove it. And I agree with putting it consistently under 
default: (which is a form of else) even if not required as it improves human 
readability.

===
-    int kind_self;
-    int kind_sub;
+    enum PyUnicode_Kind kind_self;
+    enum PyUnicode_Kind kind_sub;
     void *data_self;
     void *data_sub;

Several places elsewhere, multiple enum vars are defined on one line, so you 
*could* do the same here (and with void *vars?).

====
-    int kind1, kind2, kind;
+    enum PyUnicode_Kind kind1, kind2, kind;
<four places>
being really picky, I would put 'kind' first, not last, unless there is a good 
reason in the omitted context. It is mentally jarring for me as is without 
context. Elsewhere, things like src_/dest_/kind and x/y/z/kind are in the 
'expected' order.

----------
nosy: +terry.reedy

_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue15092>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to