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

Py_INCREF/DECREF available as macros only #40170

Closed
etrepum mannequin opened this issue Apr 20, 2004 · 15 comments
Closed

Py_INCREF/DECREF available as macros only #40170

etrepum mannequin opened this issue Apr 20, 2004 · 15 comments
Assignees

Comments

@etrepum
Copy link
Mannequin

etrepum mannequin commented Apr 20, 2004

BPO 938302
Nosy @sjoerdmullender, @theller, @nascheme, @etrepum
Files
  • python-2.4-exported-ref-funcs.patch: exported functions for reference counting
  • python-2.4-exported-ref-funcs-2.patch: exported functions for reference counting (rev 2)
  • 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/theller'
    closed_at = <Date 2004-04-22.17:28:46.000>
    created_at = <Date 2004-04-20.02:37:49.000>
    labels = []
    title = 'Py_INCREF/DECREF available as macros only'
    updated_at = <Date 2004-04-22.17:28:46.000>
    user = 'https://github.com/etrepum'

    bugs.python.org fields:

    activity = <Date 2004-04-22.17:28:46.000>
    actor = 'theller'
    assignee = 'theller'
    closed = True
    closed_date = None
    closer = None
    components = ['None']
    creation = <Date 2004-04-20.02:37:49.000>
    creator = 'bob.ippolito'
    dependencies = []
    files = ['5937', '5938']
    hgrepos = []
    issue_num = 938302
    keywords = ['patch']
    message_count = 15.0
    messages = ['45804', '45805', '45806', '45807', '45808', '45809', '45810', '45811', '45812', '45813', '45814', '45815', '45816', '45817', '45818']
    nosy_count = 4.0
    nosy_names = ['sjoerd', 'theller', 'nascheme', 'bob.ippolito']
    pr_nums = []
    priority = 'normal'
    resolution = 'accepted'
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue938302'
    versions = ['Python 2.4']

    @etrepum
    Copy link
    Mannequin Author

    etrepum mannequin commented Apr 20, 2004

    I have an application where I locate a python shared library, link to
    it, and bind a few symbols at runtime in order to run some Python
    code. I need to be able to use Py_INCREF and Py_DECREF from
    this code (to show nice output if there is an exception), but since it
    is done dynamically my code has no idea what the definitions of
    those macros were. This is especially bad because they are
    different for a debugging build.

    I would like Py_INCREF and Py_DECREF to be available as exported
    functions *somewhere* in the Python shared library.

    @etrepum etrepum mannequin closed this as completed Apr 20, 2004
    @etrepum etrepum mannequin assigned theller Apr 20, 2004
    @etrepum etrepum mannequin closed this as completed Apr 20, 2004
    @etrepum etrepum mannequin assigned theller Apr 20, 2004
    @theller
    Copy link

    theller commented Apr 20, 2004

    Logged In: YES
    user_id=11105

    Bob, I understand your desire and will support it. Would
    you like to work on a patch?

    @etrepum
    Copy link
    Mannequin Author

    etrepum mannequin commented Apr 20, 2004

    Logged In: YES
    user_id=139309

    Here's a patch.. Py_IncRef and Py_DecRef for Py_XINCREF and
    Py_XDECREF respectively.

    @theller
    Copy link

    theller commented Apr 20, 2004

    Logged In: YES
    user_id=11105

    Docs are missing ;-)

    @etrepum
    Copy link
    Mannequin Author

    etrepum mannequin commented Apr 20, 2004

    Logged In: YES
    user_id=139309

    Where do those go? Does it *need* to be well documented right away?
    It's not useful very often ;)

    @theller
    Copy link

    theller commented Apr 20, 2004

    Logged In: YES
    user_id=11105

    IMO every function should be documented, or at least listed
    in the docs.

    I'd suggest src/Doc/api/refcounting.tex, near Py_INCREF and
    Py_DECREF.

    While we're at it, did you miss Py_XDECREF by accident?

    @etrepum
    Copy link
    Mannequin Author

    etrepum mannequin commented Apr 20, 2004

    Logged In: YES
    user_id=139309

    I'm not sure what you mean by "miss Py_XDECREF", but I did make a
    typo in the patch. Yes I know in the feature request I say Py_*REF but
    the implementation says Py_X*REF. I just didn't see a reason to use the
    "fast" version when function call overhead is there anyways.

    A new patch (which should actually compile, in theory) is attached,
    including documentation(!).

    @theller
    Copy link

    theller commented Apr 20, 2004

    Logged In: YES
    user_id=11105

    And I wasn't reading your patch carefully, but now I
    understand. Heck, I didn't even know that Py_XINCREF
    existed ;-), I've never used it somewhere.

    The only suggestion I have now: I think the functions should
    be named Py_XIncRef and Py_XDecRef.

    @nascheme
    Copy link
    Member

    Logged In: YES
    user_id=35752

    There's no need to provide the Py_X* versions, IMHO. It's easy
    enough (and faster) to write "obj != NULL && Py_IncRef(obj)" in
    the extension module.

    @etrepum
    Copy link
    Mannequin Author

    etrepum mannequin commented Apr 20, 2004

    Logged In: YES
    user_id=139309

    I don't see any reason to name them Py_X*. The X doesn't really mean
    anything. I also don't see any reason that it should use Py_INCREF
    instead of Py_XINCREF. It is NOT SUPPOSED TO BE FAST, and I don't see
    any reason to leave out the convenience. If you want it to be fast, you
    need to link directly to the Python framework or at least use macros for
    a particular configuration of Python.

    This is for the very limited use case where you don't know or care how
    Python was configured. I want to use it for the OS X python bootstrap
    (something like py2exe / freeze) and the reference counting is ONLY
    used to extract information from an uncaught exception (if there is one)
    when the script fails to execute. It doesn't link directly to Python
    because the idea is that you shouldn't need a compiler to make a Python-
    based application, and there's no real good reason to wait for gcc to
    compile+link the same thing over and over again.

    @theller
    Copy link

    theller commented Apr 20, 2004

    Logged In: YES
    user_id=11105

    The X does mean something - you *can* pass a NULL pointer.
    And I think functions should do exactly the same as the
    macros with the same name - that's the reason why I
    suggested the name change.

    Whether we expose the Py_(INC|DEC)REF or the
    Py_X(INC|DEC)REF macros as functions, or both pairs, I have
    no preference.

    @sjoerdmullender
    Copy link
    Member

    Logged In: YES
    user_id=43607

    I'd say, only provide versions that are equivalent to
    Py_X{INC,DEC}REF. Defensive programming!
    Also, the extra overhead of checking for a NULL pointer is
    completely dwarfed by the overhead of calling a function.

    @etrepum
    Copy link
    Mannequin Author

    etrepum mannequin commented Apr 20, 2004

    Logged In: YES
    user_id=139309

    I think that the X prefix only applies to macros. AFAIK there are no
    Python *functions* that use the X prefix, but most of them check input. I
    am not using the MACRO CAPITALIZATION, so I don't think I should need
    to use XMACRO XPREFIXES either. They're ugly.

    sjoerd clearly explains why I chose the X versions in his comment below.

    In short, I think it's a bad idea to functionify Py_(INC|DEC)REF, but I don't
    want to pay the XPENALTY for making the right choice.

    @theller
    Copy link

    theller commented Apr 20, 2004

    Logged In: YES
    user_id=11105

    Ok, I'll buy that. And the docs Bob has provided explain
    that the functions wrap the Py_X... macros.
    If nobody objects, I can check this in tomorrow.

    @theller
    Copy link

    theller commented Apr 22, 2004

    Logged In: YES
    user_id=11105

    Checked into CVS.
    Thanks.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 9, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    None yet
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants