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

Fix minor inconsistency in the order of == operands #81829

Closed
serhiy-storchaka opened this issue Jul 22, 2019 · 3 comments
Closed

Fix minor inconsistency in the order of == operands #81829

serhiy-storchaka opened this issue Jul 22, 2019 · 3 comments
Labels
3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@serhiy-storchaka
Copy link
Member

BPO 37648
Nosy @gvanrossum, @serhiy-storchaka, @MojoVampire, @tirkarthi
PRs
  • bpo-37648: Fixed minor inconsistency in some __contains__. #14904
  • 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 2019-08-04.11:14:56.813>
    created_at = <Date 2019-07-22.07:58:57.299>
    labels = ['interpreter-core', '3.9']
    title = 'Fix minor inconsistency in the order of == operands'
    updated_at = <Date 2019-09-13.09:43:23.212>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2019-09-13.09:43:23.212>
    actor = 'xtreak'
    assignee = 'none'
    closed = True
    closed_date = <Date 2019-08-04.11:14:56.813>
    closer = 'serhiy.storchaka'
    components = ['Interpreter Core']
    creation = <Date 2019-07-22.07:58:57.299>
    creator = 'serhiy.storchaka'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 37648
    keywords = ['patch']
    message_count = 3.0
    messages = ['348287', '348981', '352262']
    nosy_count = 4.0
    nosy_names = ['gvanrossum', 'serhiy.storchaka', 'josh.r', 'xtreak']
    pr_nums = ['14904']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue37648'
    versions = ['Python 3.9']

    @serhiy-storchaka
    Copy link
    Member Author

    There is two ways to write the comparison when we search an item equal to the needle:

    item == needle
    

    and

    needle == item
    

    In most cases they get the same result. But if __eq__ of the item or the needle is not correctly implemented (returns False instead of NotImplemented for unsupported types) the order can matter. We want the behavior be consistent in all cases.

    The majority of the code uses needle on the right side of the == operator. But there are few outliners:

    • tuple.__contains__ and list.__contains__. They code is different from the code used in methods count(), index(), remove() where needle is on the right.

    • The general implementation of the "in" operator for iterators (see PySequence_Contains).

    • dictitems.__contains__.

    • The copy of list_contains in _ssl.c.

    • The C implementation of _asyncio.Future.remove_done_callback. It differs from the Python implementation.

    In all other code (many tens of occurrences) needle is on the right.

    I think it is worth to fix the minor inconsistency in these few places.

    See also the discussion on Python-Dev: https://mail.python.org/archives/list/python-dev@python.org/thread/VSV4K4AOKM4CBQMOELPFV5VMYALPH464/.

    @serhiy-storchaka serhiy-storchaka added 3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Jul 22, 2019
    @serhiy-storchaka
    Copy link
    Member Author

    New changeset 18b711c by Serhiy Storchaka in branch 'master':
    bpo-37648: Fixed minor inconsistency in some __contains__. (GH-14904)
    18b711c

    @tirkarthi
    Copy link
    Member

    Just to add list.__contains__ and others might cause subtle changes as noted. One example is around discussion at #14700 (comment) where my code to fix mock.ANY was depending on this behavior to work on master and didn't work with 3.8 since it's not backported. mock.ANY depends on the order of operands.

    @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.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs)
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants