http://bugs.python.org/review/25483/diff/15824/Python/ceval.c File Python/ceval.c (right): http://bugs.python.org/review/25483/diff/15824/Python/ceval.c#newcode3374 Python/ceval.c:3374: fmt_spec = (oparg & 0x4) ? POP() : NULL; Maybe declare variables for the bitfields you break out of the oparg? Just a thought, might slightly enhance readability. int which_conversion = oparg & 0x3; int have_fmt_spec = oparg & 0x4; http://bugs.python.org/review/25483/diff/15824/Python/ceval.c#newcode3378 Python/ceval.c:3378: case 0x0: Surely I would enjoy some symbolic constants here, instead of 0x0 0x1 0x2 0x3? http://bugs.python.org/review/25483/diff/15824/Python/compile.c File Python/compile.c (right): http://bugs.python.org/review/25483/diff/15824/Python/compile.c#newcode3246 Python/compile.c:3246: /* Note that this code uses the builtin functions format(), str(), Comment is now out of date. http://bugs.python.org/review/25483/diff/15824/Python/compile.c#newcode3257 Python/compile.c:3257: !s : 001 0x1 Yup, symbolic constants would make this more readable.
Thanks for the review. I'll address your points. http://bugs.python.org/review/25483/diff/15824/Python/ceval.c File Python/ceval.c (right): http://bugs.python.org/review/25483/diff/15824/Python/ceval.c#newcode3374 Python/ceval.c:3374: fmt_spec = (oparg & 0x4) ? POP() : NULL; Good idea. http://bugs.python.org/review/25483/diff/15824/Python/ceval.c#newcode3378 Python/ceval.c:3378: case 0x0: Yes, I just didn't have a good place to put them. I'll stash them in a .h file somewhere. http://bugs.python.org/review/25483/diff/15824/Python/compile.c File Python/compile.c (right): http://bugs.python.org/review/25483/diff/15824/Python/compile.c#newcode3246 Python/compile.c:3246: /* Note that this code uses the builtin functions format(), str(), Thanks.