Navigation Menu

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

bool of large range raises OverflowError #73062

Closed
Tracked by #74019
mdickinson opened this issue Dec 5, 2016 · 18 comments
Closed
Tracked by #74019

bool of large range raises OverflowError #73062

mdickinson opened this issue Dec 5, 2016 · 18 comments
Assignees
Labels
3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@mdickinson
Copy link
Member

BPO 28876
Nosy @rhettinger, @mdickinson, @methane, @4kir4, @serhiy-storchaka
PRs
  • bpo-28876: bool of large range raises OverflowError #699
  • bpo-28876: bool of large range raises OverflowError (#699) #734
  • bpo-28876: bool of large range raises OverflowError (#699) #735
  • Files
  • range_bool.patch
  • range_bool-no_docs.patch: no docs updates
  • range_bool-c99-designated-initializers.patch: use c99 designated initializers
  • range_bool-c99-designated-initializers-indent.patch: use 4-space indent, space around "="
  • 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 = 'https://github.com/rhettinger'
    closed_at = <Date 2017-03-20.07:39:08.069>
    created_at = <Date 2016-12-05.09:37:42.421>
    labels = ['interpreter-core', 'type-bug', '3.7']
    title = 'bool of large range raises OverflowError'
    updated_at = <Date 2017-03-24.20:18:43.324>
    user = 'https://github.com/mdickinson'

    bugs.python.org fields:

    activity = <Date 2017-03-24.20:18:43.324>
    actor = 'serhiy.storchaka'
    assignee = 'rhettinger'
    closed = True
    closed_date = <Date 2017-03-20.07:39:08.069>
    closer = 'serhiy.storchaka'
    components = ['Interpreter Core']
    creation = <Date 2016-12-05.09:37:42.421>
    creator = 'mark.dickinson'
    dependencies = []
    files = ['45765', '45773', '46378', '46391']
    hgrepos = []
    issue_num = 28876
    keywords = ['patch']
    message_count = 18.0
    messages = ['282402', '282405', '282414', '282418', '282506', '283137', '286008', '286048', '286055', '286089', '286092', '289772', '289778', '289786', '289876', '290129', '290130', '290132']
    nosy_count = 5.0
    nosy_names = ['rhettinger', 'mark.dickinson', 'methane', 'akira', 'serhiy.storchaka']
    pr_nums = ['699', '734', '735']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue28876'
    versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']

    @mdickinson
    Copy link
    Member Author

    The bool of a large range raises OverflowError:

        >>> bool(range(2**63))
        Traceback (most recent call last):
          File "<stdin>", line 1, in <module>
        OverflowError: Python int too large to convert to C ssize_t

    This is a side-effect of len raising OverflowError, which is a general problem that's nontrivial to fix (the sq_length slot is constrained to return a ssize_t). In theory, though, it would be possible to implement nb_bool for range objects to do the right thing.

    In practice, this may well not be worth fixing, though I think it's at least worth reporting.

    @mdickinson mdickinson added 3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Dec 5, 2016
    @rhettinger
    Copy link
    Contributor

    +1 for implementing nb_bool for range objects.

    @4kir4
    Copy link
    Mannequin

    4kir4 mannequin commented Dec 5, 2016

    Here's a patch with range_bool() implementation, tests and the docs update.

    I'm not sure how it should be documented. I've specified it as
    versionchanged:: 3.6

    @mdickinson
    Copy link
    Member Author

    I'm not sure how it should be documented.

    I think a change at this level probably isn't worth documenting in the official docs; it's enough that there's a Misc/NEWS entry for it.

    @4kir4
    Copy link
    Mannequin

    4kir4 mannequin commented Dec 6, 2016

    I've removed the documentation changes from the patch.

    @mdickinson
    Copy link
    Member Author

    Patch LGTM. You could safely drop the initialisers beyond nb_bool in the range_as_number struct (per C99 6.7.8p21), but it's fine as it is.

    @4kir4
    Copy link
    Mannequin

    4kir4 mannequin commented Jan 22, 2017

    Following the python-dev discussion [1] I've added a variant of the patch that uses c99 designated initializers [2]

    [1] https://mail.python.org/pipermail/python-dev/2017-January/147175.html
    [2] https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html

    @rhettinger
    Copy link
    Contributor

    This patch looks ready to go. I'll wait a bit to see it there are any other comments. If not, I'll apply it shortly.

    @rhettinger rhettinger self-assigned this Jan 23, 2017
    @methane
    Copy link
    Member

    methane commented Jan 23, 2017

    LGTM, except 2-space indent.

    @4kir4
    Copy link
    Mannequin

    4kir4 mannequin commented Jan 23, 2017

    I've updated the patch to use 4-space indent (pep-7).

    I've added space around "=" (pep-7); unlike the usual
    "dict(designator=value)" -- no space around "=" for keyword argument
    (pep-8).

    @mdickinson
    Copy link
    Member Author

    Latest patch LGTM too.

    @serhiy-storchaka
    Copy link
    Member

    Akira, could you open a pull request on GitHub?

    @4kir4
    Copy link
    Mannequin

    4kir4 mannequin commented Mar 17, 2017

    Akira, could you open a pull request on GitHub?

    Done. PR 699

    @serhiy-storchaka
    Copy link
    Member

    In bpo-29840 proposed an alternate and more general solution. But I think that nb_bool should be implemented for range objects since bpo-29840 is 3.7 only and nb_bool is faster.

    @serhiy-storchaka
    Copy link
    Member

    Thank you Akira for your patch.

    @serhiy-storchaka
    Copy link
    Member

    New changeset 4276068 by Serhiy Storchaka in branch '3.5':
    bpo-28876: bool of large range raises OverflowError (#699) (#735)
    4276068

    @serhiy-storchaka
    Copy link
    Member

    New changeset 6fad409 by Serhiy Storchaka in branch '3.6':
    bpo-28876: bool of large range raises OverflowError (#699) (#734)
    6fad409

    @serhiy-storchaka
    Copy link
    Member

    New changeset e46fb86 by Serhiy Storchaka (4kir4) in branch 'master':
    bpo-28876: bool of large range raises OverflowError (#699)
    e46fb86

    @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-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants