[Python-bugs-list] [ python-Bugs-660455 ] -hex/oct const generates wrong code

[SourceForge.net]

Bugs item #660455, was opened at 2002-12-31 18:11
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=660455&group_id=5470

Category: Parser/Compiler
Group: Python 2.2
Status: Open
Resolution: None
Priority: 6
Submitted By: Guido van Rossum (gvanrossum)
Assigned to: Nobody/Anonymous (nobody)
Summary: -hex/oct const generates wrong code

Initial Comment:
Since Python 2.2, there's code in com_factor() in
compile.c that expands negative constants inline. For
example, the expression -1 generates a single
LOAD_CONST opcode for -1, rather than (as in Python 2.1
and before) generating a LOAD_CONST for +1 followed by
a UNARY_NEGATIVE opcode.

Unfortunately there's an end case where this causes
surprising behavior: when the constant is an octal or
hexadecimal constant that already has a negative value
(despite having no sign), the generated code yields a
different result than expected.  For example:

>>> print -(0xffffffff)
1
>>> x = 0xffffffff 
>>> print -x
1
>>> print -0xffffffff 
-4294967295
>>> 

Despite the fact that in Python 2.4 (!) all of these
will print the third outcome, in Python 2.2 and 2.3, I
think the third outcome is wrong and unexpected.

No time to fix this for 2.3a1, but should fix it before
2.3a2.

----------------------------------------------------------------------

>Comment By: Michael Hudson (mwh)
Date: 2003-01-17 10:47

Message:
Logged In: YES 
user_id=6656

Oh, so maybe the fact that Lib/compiler has a problem in the
same place *isn't* a problem with the compiler package... 

What ever gets done here should probably coincide with
whatever happens in patch 661536.

----------------------------------------------------------------------

Comment By: Guido van Rossum (gvanrossum)
Date: 2003-01-16 19:53

Message:
Logged In: YES 
user_id=6380

I believe that test relies on the bug. :-(

So the test should be fixed.

The value ought to be equal to -(020000000000L).

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2003-01-16 19:45

Message:
Logged In: YES 
user_id=33168

The test fails because of line test_grammar.py:40:

  if -2147483647-1 != -020000000000:

What should -020000000000 be?
Without the -, it is:  -(2**31)-1, so it can't be
represented in an int (on 32-bit arches).
With the patch, it is: 2147483648L.

Later, I'll take a look at doing what Guido suggested.

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2003-01-16 18:38

Message:
Logged In: YES 
user_id=33168

mwh pointed out that my patch breaks test_grammar.  I'll
take another look later, unless Raymond beats me to it.

----------------------------------------------------------------------

Comment By: Guido van Rossum (gvanrossum)
Date: 2003-01-16 17:32

Message:
Logged In: YES 
user_id=6380

I'd like to keep the optimization too.

I think you have to do something different. The code
currently tries to save some time by prepending a minus sign
to the literal. I think it should just evaluate the literal
and then apply PyNumber_Negative() to the result, all the
while catching exceptions and falling back to unoptimized
code if there are exceptions.

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2003-01-16 15:18

Message:
Logged In: YES 
user_id=33168

Ideally, I'd like to keep the code, rather than rip it out.
 Is it possible to keep the optimization, but if it is -
followed by a 0, then fallback to the original technique
(ie, UNARY_NEGATIVE, followed by the value)?

I have attached a patch which seems to implement this.

----------------------------------------------------------------------

Comment By: Raymond Hettinger (rhettinger)
Date: 2003-01-12 11:35

Message:
Logged In: YES 
user_id=80475

See attached patch.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=660455&group_id=5470