Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(45)

#16840: Tkinter doesn't support large integers

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years ago by storchaka+cpython
Modified:
4 years, 9 months ago
Reviewers:
victor.stinner
CC:
loewis, haypo, Benjamin Peterson, ggpolo_gmail.com, devnull_psf.upfronthosting.co.za, storchaka
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Patch Set 3 #

Patch Set 4 #

Total comments: 12

Patch Set 5 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Modules/_tkinter.c View 1 2 3 4 3 chunks +4 lines, -3 lines 0 comments Download
Modules/tkinter.h View 1 2 3 4 1 chunk +15 lines, -7 lines 0 comments Download

Messages

Total messages: 2
victor.stinner_gmail.com
https://bugs.python.org/review/16840/diff/13988/Lib/test/test_tcl.py File Lib/test/test_tcl.py (right): https://bugs.python.org/review/16840/diff/13988/Lib/test/test_tcl.py#newcode139 Lib/test/test_tcl.py:139: if tcl_version >= (8, 5): Hum, please add a ...
4 years, 10 months ago #1
storchaka_gmail.com
4 years, 10 months ago #2
https://bugs.python.org/review/16840/diff/13988/Lib/test/test_tcl.py
File Lib/test/test_tcl.py (right):

https://bugs.python.org/review/16840/diff/13988/Lib/test/test_tcl.py#newcode139
Lib/test/test_tcl.py:139: if tcl_version >= (8, 5):
On 2015/03/27 11:16:37, haypo wrote:
> Hum, please add a comment explaining why larger integers are allowed on Tcl
> 8.5+.

This type was added in Tcl 8.5.

Needed also a check for tcl_version >= (8, 4) if apply the paytch to older
Python versions and support Tcl < 8.4 (wideInt support was added in 8.4).

> What is the behaviour for larger integers on Tcl < 8.5? Can we guarantee a
> reliable exception? (Write an unit test?)

The same as now. The problem is that exact range of wideInt is unknown. It is at
least 64-bit, but it can be wider on some platform and 2**63 can be supported
even in Tcl < 8.5.

https://bugs.python.org/review/16840/diff/13988/Lib/test/test_tcl.py#newcode482
Lib/test/test_tcl.py:482: integers = (0, 1, -1, 2**31-1, -2**31, 2**31,
-2**31-1, 2**63-1, -2**63)
On 2015/03/27 11:16:37, haypo wrote:
> Hum, you should generate this list in a function taking tcl_version as
> parameter, instead of duplicating it multiple times in tests.

May be. This list is used in 4 tests, but only in 4 tests.

https://bugs.python.org/review/16840/diff/13988/Modules/_tkinter.c
File Modules/_tkinter.c (right):

https://bugs.python.org/review/16840/diff/13988/Modules/_tkinter.c#newcode1098
Modules/_tkinter.c:1098: return NULL;
On 2015/03/27 11:16:37, haypo wrote:
> Hum, you should call PyErr_NoMemory() here, no?

Good catch.

https://bugs.python.org/review/16840/diff/13988/Modules/_tkinter.c#newcode1111
Modules/_tkinter.c:1111: PyObject *res2 = PyNumber_Negative(res);
On 2015/03/27 11:16:37, haypo wrote:
> We should maybe expose _PyLong_Negate() in longobject.h to avoid the copy?

Large negative numbers are pretty rare (they never occurred in typical
application). This is not performance critical.

https://bugs.python.org/review/16840/diff/13988/Modules/_tkinter.c#newcode1203
Modules/_tkinter.c:1203: #ifdef HAVE_LIBTOMMAMTH
On 2015/03/27 11:16:37, haypo wrote:
> Why not moving this block closer to the fromWideIntObj() block?

Initially it was close to the fromWideIntObj() block. OK, I'll return this part
back.

https://bugs.python.org/review/16840/diff/13988/Modules/_tkinter.c#newcode1209
Modules/_tkinter.c:1209: if (app->BignumType == NULL &&
On 2015/03/27 11:16:37, haypo wrote:
> Maybe keep this unlikely case at the end.

Done.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+