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.

Sign up for GitHub

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

Jump to bottom

Weak reference support for C function objects #66314

Closed
pitrou opened this issue Jul 31, 2014 · 10 comments
Closed

Weak reference support for C function objects #66314

pitrou opened this issue Jul 31, 2014 · 10 comments
Labels
easy interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@pitrou
Copy link
Member

pitrou commented Jul 31, 2014

BPO 22116
Nosy @pitrou, @scoder, @ezio-melotti, @kilowu
Files
  • 22116.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 2014-08-06.23:36:29.296>
created_at = <Date 2014-07-31.17:51:31.881>
labels = ['interpreter-core', 'easy', 'type-feature']
title = 'Weak reference support for C function objects'
updated_at = <Date 2014-08-06.23:36:29.295>
user = 'https://github.com/pitrou'

    bugs.python.org fields:

    activity = <Date 2014-08-06.23:36:29.295>
actor = 'pitrou'
assignee = 'none'
closed = True
closed_date = <Date 2014-08-06.23:36:29.296>
closer = 'pitrou'
components = ['Interpreter Core']
creation = <Date 2014-07-31.17:51:31.881>
creator = 'pitrou'
dependencies = []
files = ['36208']
hgrepos = []
issue_num = 22116
keywords = ['patch', 'easy']
message_count = 10.0
messages = ['224432', '224541', '224558', '224561', '224641', '224675', '224692', '224738', '224974', '224975']
nosy_count = 6.0
nosy_names = ['pitrou', 'scoder', 'ezio.melotti', 'python-dev', 'Anthony.Kong', 'kilowu']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue22116'
versions = ['Python 3.5']
    @pitrou
    Copy link
    Member Author

    pitrou commented Jul 31, 2014

    Currently, it is not possible to take a weakref to a PyCFunction object. However, those objects already have full GC support, so it wouldn't be silly to add weakref support to them.

    (this came in the context of numba, which generates such C functions on-the-fly)

    @pitrou pitrou added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement easy labels Jul 31, 2014
    @scoder
    Copy link
    Contributor

    scoder commented Aug 2, 2014

    FWIW, functions in Cython (which C-level-inherit from PyCFunction) support weak references just fine. Adding that as a general feature to PyCFunction sounds like the right thing to do.

    @kilowu
    Copy link
    Mannequin

    kilowu mannequin commented Aug 2, 2014

    I have made a patch related to this issue, please take a look at it. Thanks :)

    @scoder
    Copy link
    Contributor

    scoder commented Aug 2, 2014

    Patch looks ok. Not sure about the test dependency from test_weakref.py to _testcapi, though. Is that module allowed to be used everywhere? Wouldn't it be enough to test that one of the builtin functions is now weak referencible? "len" seems to be used in other places, for example.

    @pitrou
    Copy link
    Member Author

    pitrou commented Aug 3, 2014

    Wouldn't it be enough to test that one of the builtin functions is now weak referencible?

    It's better to check that the weakref gets cleared when the object dies, and for that you need an object you can dispose of.

    @pitrou
    Copy link
    Member Author

    pitrou commented Aug 4, 2014

    @kilowu, your patch looks good to me. As a necessary step to include your contribution, could you please sign the contributor's agreement? See https://www.python.org/psf/contrib/contrib-form/

    Thank you very much!

    @kilowu
    Copy link
    Mannequin

    kilowu mannequin commented Aug 4, 2014

    @pitrou, thank you for the review. I have signed the contributor agreement form after submitting this patch. Please let me know if there is a further step to help you to verify the signed contributor agreement.

    I'm really glad to have the chance to contribute back to the community.

    @pitrou
    Copy link
    Member Author

    pitrou commented Aug 4, 2014

    Ah, yes, indeed, the agreement seems to have been validated in the meantime (you can see it by the asterisk near your name in the comments here). Thank you!

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 6, 2014

    New changeset 87f940e85cb0 by Antoine Pitrou in branch 'default':
    Issue bpo-22116: C functions and methods (of the 'builtin_function_or_method' type) can now be weakref'ed. Patch by Wei Wu.
    http://hg.python.org/cpython/rev/87f940e85cb0

    @pitrou
    Copy link
    Member Author

    pitrou commented Aug 6, 2014

    I have committed your patch. Thank you very much for contributing!

    @pitrou pitrou closed this as completed Aug 6, 2014
    @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
    Assignees
    No one assigned
    Labels
    easy interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @scoder @pitrou