This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Integer right shift raises OverflowError when second operand is large
Type: behavior Stage:
Components: Interpreter Core Versions: Python 3.0, Python 2.6
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: Nosy List: belopolsky, loewis, mark.dickinson
Priority: normal Keywords: patch

Created on 2008-05-09 18:17 by mark.dickinson, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
issue2804.diff belopolsky, 2008-05-09 20:38
Messages (11)
msg66484 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-05-09 18:17
In Python 2.6a3:

>>> 1 >> (2**31)  # unexpected exception
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OverflowError: long int too large to convert to int
>>> 1 >> (2**31-1)  # works as expected
0L

It might make more sense for the first expression to return 0 instead.
Similarly, perhaps 0 << (2**31) should also return 0.
msg66485 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-05-09 20:38
Attached patch makes a >> b return 0 for b > maxsize.
msg66487 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-05-09 21:01
On the second thought, it is theoretically possible to have a long int a
for which a >> maxsize is not zero because that would require only
maxsize/15 digits.  I am not sure it is worth the trouble to account for
that.
msg66489 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-05-09 21:15
> On the second thought, it is theoretically possible to have a long int 
a
> for which a >> maxsize is not zero

This occurred to me too; somehow this hardly seems worth worrying about-
--there are already other problems in longobject.c when integers get 
very large, and I don't think one more is going to hurt.  Perhaps a 
warning somewhere in the documentation would be appropriate.

(It seems to me that if anyone really cares, the *right* thing to do is
to limit all integers to be at most maxsize bits long.)

Thanks for the patch!  I'll take a look.
msg66500 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-05-09 23:08
I'm -1 on this patch (or any other fixing the perceived problem).
Special cases aren't special enough to break the rules.

If you ever see this error in a real application, you have deeper
problems than the exception.
msg66503 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-05-09 23:37
I'm not sure I understand.  What rule would be broken by this patch?
msg66504 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-05-10 00:13
> I'm not sure I understand.  What rule would be broken by this patch?

The (implicit) rule that there is a range limitation on the shift width.
The limitation would continue to exist, just not for selected left-hand
parameters. This is fairly arbitrary.
msg66505 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-05-10 00:26
On Fri, May 9, 2008 at 7:08 PM, Martin v. Löwis <report@bugs.python.org> wrote:
..
> I'm -1 on this patch (or any other fixing the perceived problem).
> Special cases aren't special enough to break the rules.
>
> If you ever see this error in a real application, you have deeper
> problems than the exception.

That was my first reaction as well, but then I thought that it was
easy to fix because the answer is guaranteed to be 0 for ridiculously
large right shifts.  However, since the patch is not theoretically
correct, I would only be +0 on applying it.

I also note that the current requirement is that shift fits long
rather than Py_ssize_t, which may lead to a >> sys.maxsize being
invalid on platforms where sizeof(long) < sizeof(Py_ssize_t).  This
may be a problem because sys.maxsize is likely to be used as a
placeholder for an arbitrary large number.

This said, I sill don't feel strongly one way or another.
msg66506 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-05-10 00:49
It's the current behaviour that seems arbitrary to me: a >> b is well-
defined, and efficiently computable, for any integer a and nonnegative 
integer b;  it seems odd to have an unnecessary and (from a user's 
viewpoint) arbitrary limit on the size of b.  It's simple to have the 
operation always succeed, and this seems to me like both the least 
surprising, and the most correct, thing to do.

On the other hand, I can't really think of any real-world uses for large 
shifts:  I'm guessing that in applications a shift of more than 63 or so  
must be rare.  So it probably doesn't matter that much one way or 
another.

Changing the limit on the shift to sys.maxsize rather than LONG_MAX 
would seem to make some sense, though.
msg66509 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-05-10 06:51
I still don't think the improvement in observable behavior is worth the
cost of additional code (and as Alexander observes, the patch in
file10234 is incorrect in boundary cases).
msg66514 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-05-10 12:35
Fair enough.  Closing as won't fix.
History
Date User Action Args
2022-04-11 14:56:34adminsetgithub: 47053
2008-05-10 12:35:47mark.dickinsonsetstatus: open -> closed
resolution: wont fix
messages: + msg66514
2008-05-10 06:51:40loewissetmessages: + msg66509
2008-05-10 00:49:43mark.dickinsonsetmessages: + msg66506
2008-05-10 00:26:59belopolskysetmessages: + msg66505
2008-05-10 00:13:11loewissetmessages: + msg66504
2008-05-09 23:37:40mark.dickinsonsetmessages: + msg66503
2008-05-09 23:08:20loewissetnosy: + loewis
messages: + msg66500
2008-05-09 21:15:04mark.dickinsonsetmessages: + msg66489
2008-05-09 21:01:42belopolskysetmessages: + msg66487
2008-05-09 20:38:54belopolskysetfiles: + issue2804.diff
keywords: + patch
messages: + msg66485
nosy: + belopolsky
2008-05-09 18:17:55mark.dickinsoncreate