classification
Title: Guarantee that divmod() and PyNumber_Divmod() return a 2-tuple
Type: enhancement Stage: patch review
Components: Interpreter Core Versions: Python 3.8
process
Status: open Resolution:
Dependencies: 34716 Superseder:
Assigned To: Nosy List: mark.dickinson, p-ganssle, rhettinger, serhiy.storchaka, xtreak, zach.ware
Priority: normal Keywords: patch

Created on 2018-09-14 10:13 by serhiy.storchaka, last changed 2018-09-17 19:55 by serhiy.storchaka.

Pull Requests
URL Status Linked Edit
PR 9301 open serhiy.storchaka, 2018-09-14 10:24
Messages (6)
msg325341 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-09-14 10:13
It is documented that divmod() returns a pair of numbers, but the implementation doesn't enforce this. Actually divmod() just returns the result of __divmod__() or __rdivmod__() without checking it. PyNumber_Divmod() is documented as the C equivalent of divmod(). But since it returns a 2-tuple in all implemented cases, the user code can expect that it always return a 2-tuple. This can lead to hidden bugs similar to issue31577.

I think there are no reasons of returning anything except a 2-tuple from divmod(). Forcing this conditions can save third-party code from crashes. The following PR make divmod() and PyNumber_Divmod() always returning a 2-tuple.
msg325361 - (view) Author: Paul Ganssle (p-ganssle) * Date: 2018-09-14 16:43
I would be somewhat worried that this might break something like numpy (though not numpy specifically) which might be counting on the ability to write a wrapper that overloads this behavior.

Another concern is that people could be playing it fast-and-loose with actual types, and are returning something that fits the bill for a collections.abc.Sequence with length 2 (e.g. return [a, b]) but isn't an actual Tuple.

I would at least think that this should go through a deprecation cycle.
msg325364 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-09-14 16:58
-0 Perhaps this should be left as-is.  There is no known problem that we would be solving and it would add an extra step to a fine-grained function.  In general, I think we have these guards in places where it is known that downstream callers rely on a particular type signature for the return value.
msg325398 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2018-09-14 20:57
I share Paul's concern about somebody using this (mis-?)feature and being unnecessarily broken.  However, such a restriction would have prevented the segfault that was reported and fixed in bpo-31577, and would save any other users of PyNumber_Divmod from having to implement the same kind of checks.

Does anyone have an example of a legitimate use for the current behavior?
msg325423 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-09-15 04:53
An alternate idea: convert the result of __divmod__ and __rdivmod__ to tuple instead raising an error. This will support the case of returning a list. I didn't implement this initially because I think this case is very unlikely occurred in real code.

Similar changes were made in past in PyMapping_Keys(). In Python 2 it just calls the keys() method and returns the result. In Python 3 it initially converted the result to list or tuple. But since the caller code often expects a list and use PyList API with the result, later PyMapping_Keys() was fixed but making it always returning a list.
msg325543 - (view) Author: Paul Ganssle (p-ganssle) * Date: 2018-09-17 13:37
@Serhiy I like the "convert to a tuple" idea before returning *better*, though I think I'd prefer to just check that it's tuple-like, something like:

    if not all(hasattr(return_val, attr) for attr in ['__getitem__', '__len__']) or len(return_val) != 2:
        warnings.warn("divmod must return a Sequence of length 2. " +
                      "This will be an exception in Python 3.9+",
                      Deprecationwarning)

Or some similar equivalent to `if not isinstance(return_val, collections.abc.Sequence) or len(return_val) != 2`.

That said, the PR seems to have already run into an issue, which is that `unittest.mock.MagicMock` returns a `unittest.mock.MagicMock` from `divmod`, which is not a 2-tuple.
History
Date User Action Args
2018-09-17 19:55:24serhiy.storchakasetdependencies: + MagicMock.__divmod__ should return a pair
2018-09-17 13:37:42p-gansslesetmessages: + msg325543
2018-09-15 05:36:05xtreaksetnosy: + xtreak
2018-09-15 04:53:52serhiy.storchakasetmessages: + msg325423
2018-09-14 20:57:28zach.waresetnosy: + zach.ware

messages: + msg325398
title: Guarantie that divmod() and PyNumber_Divmod() return a 2-tuple -> Guarantee that divmod() and PyNumber_Divmod() return a 2-tuple
2018-09-14 16:58:44rhettingersetnosy: + rhettinger
messages: + msg325364
2018-09-14 16:43:35p-gansslesetnosy: + p-ganssle
messages: + msg325361
2018-09-14 10:24:01serhiy.storchakasetkeywords: + patch
stage: patch review
pull_requests: + pull_request8730
2018-09-14 10:13:50serhiy.storchakacreate