Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid raising OverflowError in bool() #74026

Closed
serhiy-storchaka opened this issue Mar 17, 2017 · 8 comments
Closed

Avoid raising OverflowError in bool() #74026

serhiy-storchaka opened this issue Mar 17, 2017 · 8 comments
Labels
3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@serhiy-storchaka
Copy link
Member

BPO 29840
Nosy @rhettinger, @terryjreedy, @mdickinson, @vstinner, @serhiy-storchaka
PRs
  • bpo-29840: bool() no longer raises OverflowError for objects without … #1211
  • Dependencies
  • bpo-29839: Avoid raising OverflowError in len() when len() returns negative large value
  • Files
  • bool-overflow.diff
  • bool-no-overflow-double-call.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2017-04-23.07:14:38.991>
    created_at = <Date 2017-03-17.20:27:26.527>
    labels = ['interpreter-core', 'type-feature', '3.7']
    title = 'Avoid raising OverflowError in bool()'
    updated_at = <Date 2017-04-23.07:14:38.989>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2017-04-23.07:14:38.989>
    actor = 'serhiy.storchaka'
    assignee = 'none'
    closed = True
    closed_date = <Date 2017-04-23.07:14:38.991>
    closer = 'serhiy.storchaka'
    components = ['Interpreter Core']
    creation = <Date 2017-03-17.20:27:26.527>
    creator = 'serhiy.storchaka'
    dependencies = ['29839']
    files = ['46731', '46826']
    hgrepos = []
    issue_num = 29840
    keywords = ['patch']
    message_count = 8.0
    messages = ['289781', '290139', '290606', '290615', '292064', '292121', '292140', '292156']
    nosy_count = 5.0
    nosy_names = ['rhettinger', 'terry.reedy', 'mark.dickinson', 'vstinner', 'serhiy.storchaka']
    pr_nums = ['1211']
    priority = 'normal'
    resolution = 'wont fix'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue29840'
    versions = ['Python 3.7']

    @serhiy-storchaka
    Copy link
    Member Author

    For now bool() raises OverflowError if __bool__ is not defined and __len__ returns large value.

    >>> class A:
    ...     def __len__(self):
    ...         return 1 << 1000
    ... 
    >>> bool(A())
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    OverflowError: cannot fit 'int' into an index-sized integer
    >>> bool(range(1<<1000))
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    OverflowError: Python int too large to convert to C ssize_t

    Proposed patch makes bool() returning True if len() raises OverflowError.

    This is an alternate solution of bpo-28876.

    @serhiy-storchaka serhiy-storchaka added 3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement labels Mar 17, 2017
    @terryjreedy
    Copy link
    Member

    https://docs.python.org/3/library/functions.html#bool refers to
    https://docs.python.org/3/library/stdtypes.html#truth

    The latter says that values are true ("All other values are considered true") unless one of certain conditions holds. For user-defined classes, the condition is that the class defines a __bool__() or __len__() method and that the first of those methods returns the bool False or integer zero.

    I easily interpret this as meaning that bool(x) (should) *always* return True or False. In particular, for user classes, any exception in user-coded __bool__ or __len__ (should be) included in "does not return integer zero or bool value False". This would mean that 'True' would truly be the default return for Bool().

    There is currently an unstated exception for raised Exceptions. This issue proposes an exception to the exception for OverflowErrors (once negative lengths consistently raise ValueErrors and never OverflowErrors). While this sensible in itself, I am completely happy with the added complication. I would like to either reconsider the exception for Exceptions or make it explicit.

    Patch has new text and What's New entry. Added logic in object.c looks correct.

    @vstinner
    Copy link
    Member

    Serhiy: Can you please create a pull request? It would be easier to review.

    @serhiy-storchaka
    Copy link
    Member Author

    This issue depends on bpo-29839. Tests are failed until the patch of bpo-29839 is merged.

    @vstinner
    Copy link
    Member

    Hum, I dislike this change since it's non-obvious what/who is raising the OverflowError. If an object calls a function in __len__() and the function raises OverflowError, should we consider that object is "true"? In temptation to guess, I prefer to not guess but passthrough the exception.

    If you want to support bool(range(1<<1000)), we need to get the result of __len__() as a Python object rather than a C Py_ssize_t.

    Maybe, if __len__() raises an OverflowError: call again the len(), but using the "__len__" method instead of the slot?

    @serhiy-storchaka
    Copy link
    Member Author

    I had similar doubts about this patch and needed opinions of other core developers.

    Maybe, if __len__() raises an OverflowError: call again the len(), but using the "__len__" method instead of the slot?

    Following patch implements this idea. I don't like it because it is too complicated.

    I think that we should either document that raising an OverflowError by __len__() is normal and interpreted as true in Boolean context, or document that __len__() should return a value not larger than sys.maxsize, otherwise len() and bool() can raise an OverflowError (see bpo-15718).

    @vstinner
    Copy link
    Member

    If someone wants to return a value larger than maxsize and support bool():
    it is already possible right now by defining a __bool__ method no? If yes,
    I suggest to only document this CPython implementation detail (consequence
    of slots, for efficiency) and suggest to use __bool__ for such corner case.

    I also dislike retrying to call "__len__" method instead of the slot. It
    can have annoying side effects.

    @serhiy-storchaka
    Copy link
    Member Author

    This was documented in bpo-15718.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants