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

tab-completition on instances repeatedly accesses attribute/descriptors values #69776

Closed
ezio-melotti opened this issue Nov 9, 2015 · 12 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@ezio-melotti
Copy link
Member

BPO 25590
Nosy @ezio-melotti, @vadmium, @serhiy-storchaka
Files
  • getattr-once.patch
  • uncreated-attr.patch: For 3.6
  • 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/vadmium'
    closed_at = <Date 2015-11-14.02:11:41.684>
    created_at = <Date 2015-11-09.22:15:43.539>
    labels = ['type-bug', 'library']
    title = 'tab-completition on instances repeatedly accesses attribute/descriptors values'
    updated_at = <Date 2015-11-14.02:27:23.782>
    user = 'https://github.com/ezio-melotti'

    bugs.python.org fields:

    activity = <Date 2015-11-14.02:27:23.782>
    actor = 'ezio.melotti'
    assignee = 'martin.panter'
    closed = True
    closed_date = <Date 2015-11-14.02:11:41.684>
    closer = 'martin.panter'
    components = ['Library (Lib)']
    creation = <Date 2015-11-09.22:15:43.539>
    creator = 'ezio.melotti'
    dependencies = []
    files = ['41006', '41007']
    hgrepos = []
    issue_num = 25590
    keywords = ['patch']
    message_count = 12.0
    messages = ['254415', '254419', '254422', '254435', '254468', '254469', '254475', '254583', '254637', '254638', '254640', '254641']
    nosy_count = 4.0
    nosy_names = ['ezio.melotti', 'python-dev', 'martin.panter', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue25590'
    versions = ['Python 2.7', 'Python 3.4', 'Python 3.5', 'Python 3.6']

    @ezio-melotti
    Copy link
    Member Author

    Pressing <tab> to invoke autocompletition on instances repeatedly accesses attributes/descriptors values:

    >>> # <tab> is used to indicate when/where I press <tab>
    >>> class Foo:
    ...   @property
    ...   def bar(self): print('Foo.bar called')
    ... 
    >>> f = Foo()
    >>> f.<tab>Foo.bar called 
    Foo.bar called
    Foo.bar called
    Foo.bar called
    <tab>Foo.bar called
    Foo.bar called
    Foo.bar called
    Foo.bar called
    <tab>
    f.__class__(         f.__doc__            f.__getattribute__(  f.__le__(            f.__new__(           f.__setattr__(       f.__weakref__
    f.__delattr__(       f.__eq__(            f.__gt__(            f.__lt__(            f.__reduce__(        f.__sizeof__(        f.bar
    f.__dict__           f.__format__(        f.__hash__(          f.__module__         f.__reduce_ex__(     f.__str__(           
    f.__dir__(           f.__ge__(            f.__init__(          f.__ne__(            f.__repr__(          f.__subclasshook__(  
    >>> f.b<tab>Foo.bar called
    Foo.bar called
    Foo.bar called
    Foo.bar called
    ar<enter>
    Foo.bar called

    Each time I press <tab>, the property is called 4 times. I'm not sure why the value is accessed at all, but even if there was a valid reason to do so, doing it once should be enough.

    @ezio-melotti ezio-melotti added the type-bug An unexpected behavior, bug, or error label Nov 9, 2015
    @vadmium
    Copy link
    Member

    vadmium commented Nov 9, 2015

    Long story short: it is accessed due to the callable suffix check (bpo-449227), and this check is less than optimal, meaning the attribute gets accessed up to four times per Tab press.

    In 3.6, there are only two calls, one for the hasattr() call, and one for the getattr() call:

    Foo.bar called
    File "/home/proj/python/cpython/Lib/rlcompleter.py", line 85, in complete
    self.matches = self.attr_matches(text)
    File "/home/proj/python/cpython/Lib/rlcompleter.py", line 164, in attr_matches
    hasattr(thisobject, word)):
    File "<stdin>", line 3, in bar
    Foo.bar called
    File "/home/proj/python/cpython/Lib/rlcompleter.py", line 85, in complete
    self.matches = self.attr_matches(text)
    File "/home/proj/python/cpython/Lib/rlcompleter.py", line 165, in attr_matches
    val = getattr(thisobject, word)
    File "<stdin>", line 3, in bar

    Before revision 4dbb315fe667 (bpo-25011), “words” was a list rather than a set. It gets two “bar” entries, one from dir(f) and one from dir(f.__class__). Maybe it is worth backporting set(), I dunno.

    The hasattr() call was added in r65168 (bpo-3396) as a look-before-you-leap check to avoid the completer from dying from getattr() exceptions. IMO in this case it is better to “ask for forgiveness” by catching Exception from getattr(). This would be more robust to quirky and buggy attributes anyway.

    Finally (or really, initially), getattr() was added in r64664 (bpo-449227). I think even if getattr() fails, the attribute name could still be output, just not with the callable suffix (which I partly disagree with anyway). It would have been like that previously. But I guess this is debatable, and a separate issue.

    @vadmium vadmium added the stdlib Python modules in the Lib dir label Nov 9, 2015
    @vadmium
    Copy link
    Member

    vadmium commented Nov 9, 2015

    Illustration of why I think removing the attribute on errors is bad:

    >>> class Foo:
    ...   @property
    ...   def bar(self): return self._bar
    ...   @bar.setter
    ...   def bar(self, value): self._bar = value
    ... 
    >>> f = Foo()
    >>> f.bar = ...  # Tab completion fails in this case

    @serhiy-storchaka
    Copy link
    Member

    Agreed with all your suggestions Martin. Do you want to write a patch?

    @vadmium
    Copy link
    Member

    vadmium commented Nov 11, 2015

    getattr-once.patch handles my first two points. It backports the set() change, and avoids the hasattr() check. I guess we apply this to 3.4+; expect merge conflicts with 3.6.

    As for the last point, I will make another patch to include special attributes that don’t actually exist yet, where they are currently omitted. Would we consider this a new feature for 3.6? Going by Serhiy’s logic in <https://bugs.python.org/issue25209#msg251514\> I would say yes.

    @vadmium
    Copy link
    Member

    vadmium commented Nov 11, 2015

    uncreated-attr.patch is against the 3.6 branch, and includes the excessive getattr() test, and new code to include uncreated attribute names. I added a paragraph to What’s New describing the new completion behaviour.

    @serhiy-storchaka
    Copy link
    Member

    Both patches LGTM. Thank you Martin.

    Would we consider this a new feature for 3.6?

    We can consider this a fix of the regression in bpo-449227. But this behavior here is so long. I agree with applying it to 3.6 only.

    For nicer Mercurial history, it would be better to apply both patches separately in 3.6.

    Would not be simpler to use uninitialized slot attribute in test_uncreated_attr?

    @vadmium
    Copy link
    Member

    vadmium commented Nov 13, 2015

    Yeah I plan to merge the first patch (fixing conflicts) into 3.6 without any of the uncreated-property additions, and then apply that extra stuff in a separate commit.

    Using __slots__ would be simpler; I’ll switch it over.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 13, 2015

    New changeset 92f989bfeca2 by Martin Panter in branch '3.4':
    Issue bpo-25590: Make rlcompleter only call getattr() once per attribute
    https://hg.python.org/cpython/rev/92f989bfeca2

    New changeset 808279e14700 by Martin Panter in branch '3.5':
    Issue bpo-25590: Merge rlcompleter change from 3.4 into 3.5
    https://hg.python.org/cpython/rev/808279e14700

    New changeset ff68304356fe by Martin Panter in branch 'default':
    Issue bpo-25590: Merge rlcompleter getattr change from 3.5
    https://hg.python.org/cpython/rev/ff68304356fe

    New changeset bbf63749a129 by Martin Panter in branch 'default':
    Issue bpo-25590: Complete attribute names even if they are not yet created
    https://hg.python.org/cpython/rev/bbf63749a129

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 14, 2015

    New changeset 16d83e11675f by Martin Panter in branch '2.7':
    Issue bpo-25590: Make rlcompleter only call getattr() once per attribute
    https://hg.python.org/cpython/rev/16d83e11675f

    @vadmium
    Copy link
    Member

    vadmium commented Nov 14, 2015

    I also ported the getattr patch to the 2.7 branch.

    @vadmium vadmium closed this as completed Nov 14, 2015
    @ezio-melotti
    Copy link
    Member Author

    Thanks for taking care of this!

    @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
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants