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

Backport next() #46971

Closed
birkenfeld opened this issue Apr 29, 2008 · 13 comments
Closed

Backport next() #46971

birkenfeld opened this issue Apr 29, 2008 · 13 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@birkenfeld
Copy link
Member

BPO 2719
Nosy @gvanrossum, @birkenfeld, @rhettinger, @abalkin
Files
  • nextbackport.diff: Support "bpo-" in Misc/NEWS #1
  • 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/gvanrossum'
    closed_at = <Date 2008-04-30.19:47:35.211>
    created_at = <Date 2008-04-29.21:02:25.876>
    labels = ['interpreter-core', 'type-bug']
    title = 'Backport next()'
    updated_at = <Date 2008-04-30.19:47:35.013>
    user = 'https://github.com/birkenfeld'

    bugs.python.org fields:

    activity = <Date 2008-04-30.19:47:35.013>
    actor = 'georg.brandl'
    assignee = 'gvanrossum'
    closed = True
    closed_date = <Date 2008-04-30.19:47:35.211>
    closer = 'georg.brandl'
    components = ['Interpreter Core']
    creation = <Date 2008-04-29.21:02:25.876>
    creator = 'georg.brandl'
    dependencies = []
    files = ['10141']
    hgrepos = []
    issue_num = 2719
    keywords = ['patch', '26backport']
    message_count = 13.0
    messages = ['65980', '65992', '65993', '65995', '66000', '66004', '66005', '66006', '66007', '66008', '66009', '66010', '66017']
    nosy_count = 4.0
    nosy_names = ['gvanrossum', 'georg.brandl', 'rhettinger', 'belopolsky']
    pr_nums = []
    priority = 'critical'
    resolution = 'accepted'
    stage = None
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue2719'
    versions = ['Python 2.6']

    @birkenfeld
    Copy link
    Member Author

    Backporting 3.0's next() builtin.

    There's no change w.r.t. __next__/next, that is tracked in bpo-2336.

    @birkenfeld birkenfeld added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Apr 29, 2008
    @rhettinger
    Copy link
    Contributor

    ISTM, the only value added by next(g) is that it replaces g.next() with
    a more conventional spelling, g.__next__(). Since 2.6 still has g.next
    (),I don't see how this backport adds value. It does however create a
    second way to do it that will be confusing to some remaining in the 2.x
    world. I think we should avoid double spellings in 2.6 except in cases
    where the 2-to-3 converter would need help.

    @birkenfeld
    Copy link
    Member Author

    IMO having next() in 2.6 helps since if you use it consistently you
    don't have to care about calling .next() or .__next__().

    Also, I don't see how this is different from having e.g. reduce() and
    functools.reduce() in 2.6.

    @rhettinger
    Copy link
    Contributor

    The problem is with the "if you use it consistently" premise. That
    will not hold in an environment with legacy code, multiple programmers,
    lots of code in ASPN recipes and published materials, and third-party
    modules. A patch like this dooms Py2.6 programmers to seeing both of
    these forms intermixed throughout the code base. This is *not* a win.

    @gvanrossum
    Copy link
    Member

    I think it's important to make this available in 2.6; it will let people
    writing 3.0-oriented code in 2.6 (as several developers are planning to
    do) do the right thing for this syntax.

    @abalkin
    Copy link
    Member

    abalkin commented Apr 30, 2008

    I thought new code is supposed to use Py_TYPE macro instead of ->ob_type:

    + "%.200s object is not an iterator", it->ob_type-

    tp_name);
    ..
    + res = (*it->ob_type->tp_iternext)(it);

    Py3k branch has the same issue.

    @abalkin
    Copy link
    Member

    abalkin commented Apr 30, 2008

    One more question: What is the rationale for

    + res = (*it->ob_type->tp_iternext)(it);
    + if (res == NULL) {
    ..
    + PyErr_SetNone(PyExc_StopIteration);
    + return NULL;

    ?

    I would think tp_iternext failing to set an exception should not be
    translated into stop iteration. Instead, builtin_next() should return
    NULL without an exception set and thus trigger a SystemError.

    @gvanrossum
    Copy link
    Member

    I would think tp_iternext failing to set an exception should not be
    translated into stop iteration. Instead, builtin_next() should return
    NULL without an exception set and thus trigger a SystemError.

    Wrong; the iternext slot is designed to return NULL without setting an
    exception. See e.g. listiter_next().

    @gvanrossum
    Copy link
    Member

    +1 on this. I have a few nits about the code:

    Line 1083: "%.200s object is not an iterator", it->ob_type->tp_name);
    Line is too long.

    Line 1088: if (res == NULL) {
    How about

    if (res != NULL)
    return res;

    ?

    Line 1089: if (def) {
    if (def != NULL) {

    Line 1093: PyErr_Clear();
    I would only call this if PyErr_Occurred() returns true.

    @abalkin
    Copy link
    Member

    abalkin commented Apr 30, 2008

    On Wed, Apr 30, 2008 at 12:41 PM, Guido van Rossum
    <report@bugs.python.org> wrote:

    Guido van Rossum <guido@python.org> added the comment:

    > .. builtin_next() should return
    > NULL without an exception set and thus trigger a SystemError.

    Wrong; the iternext slot is designed to return NULL without setting an
    exception. See e.g. listiter_next().

    I did not know that. Thanks for the explanation. In this case,
    wouldn't it be cleaner to call PyIter_Next which is documented to
    return NULL with no exception?

    @abalkin
    Copy link
    Member

    abalkin commented Apr 30, 2008

    On Wed, Apr 30, 2008 at 12:41 PM, Guido van Rossum
    <report@bugs.python.org> wrote:

    the iternext slot is designed to return NULL without setting an
    exception.

    This is not what the documentation says:

    """
    iternextfunc PyTypeObject.tp_iternext
    An optional pointer to a function that returns the next item in an
    iterator, or raises StopIteration when the iterator is exhausted.
    """ <http://docs.python.org/dev/c-api/typeobj.html#tp_iternext\>

    It looks like documentation needs to be updated, but wouldn't it be
    odd to specify that setting StopIteration exception is optional? It's
    probably more logical to intercept StopIteration in slot_tp_iternext
    rather than at every place where tp_iternext is called.

    @gvanrossum
    Copy link
    Member

    Feel free to submit a patch to fix the docs.

    Changing the API is not an option -- it's been like this since the
    tp_iternext slot was added, and it's been designed like this for a
    reason: so that in the common case of iterating over a built-in sequence
    type no exception objects have to be created. In particular the
    for-loop code would just discard the StopIteration instance again.

    The requirement that the exception is *optional* is so that if you're
    calling a Python iterator that *does* create the exception, the
    exception object (with whatever data the creator might have attached to
    it) doesn't get lost (or worse, have to be recreated).

    Calling PyIter_Next() here instead of inlining it would not be
    advantageous; it would just slow things down since we'd have to make a
    redundant call to PyErr_Occurred() to distinguish the StopIteration case
    from other errors.

    @birkenfeld
    Copy link
    Member Author

    Updated and committed as r62599. Also fixed your nits in the original 3k
    version in r62598.

    @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
    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