Navigation Menu

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

Jump to bottom

Clarify refcounting semantics of PyDict_SetItem[String] #83334

Closed
ncoghlan opened this issue Dec 29, 2019 · 6 comments
Closed

Clarify refcounting semantics of PyDict_SetItem[String] #83334

ncoghlan opened this issue Dec 29, 2019 · 6 comments
Labels
3.8 only security fixes 3.9 only security fixes docs Documentation in the Doc dir type-feature A feature request or enhancement

Comments

@ncoghlan
Copy link
Contributor

BPO 39153
Nosy @ncoghlan, @serhiy-storchaka, @corona10, @miss-islington
PRs
  • bpo-39153: Clarify refcounting semantics #18220
  • [3.8] bpo-39153: Clarify C API *SetItem refcounting semantics (GH-18220) #18246
    		•

    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 2020-01-30.15:49:50.472>
created_at = <Date 2019-12-29.13:33:59.858>
labels = ['type-feature', '3.8', '3.9', 'docs']
title = 'Clarify refcounting semantics of PyDict_SetItem[String]'
updated_at = <Date 2020-01-30.15:49:50.470>
user = 'https://github.com/ncoghlan'

    bugs.python.org fields:

    activity = <Date 2020-01-30.15:49:50.470>
actor = 'corona10'
assignee = 'docs@python'
closed = True
closed_date = <Date 2020-01-30.15:49:50.472>
closer = 'corona10'
components = ['Documentation']
creation = <Date 2019-12-29.13:33:59.858>
creator = 'ncoghlan'
dependencies = []
files = []
hgrepos = []
issue_num = 39153
keywords = ['patch']
message_count = 6.0
messages = ['358988', '359008', '359018', '360953', '360955', '361074']
nosy_count = 5.0
nosy_names = ['ncoghlan', 'docs@python', 'serhiy.storchaka', 'corona10', 'miss-islington']
pr_nums = ['18220', '18246']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue39153'
versions = ['Python 3.8', 'Python 3.9']
    @ncoghlan
    Copy link
    Contributor Author

    The documentation for PyList_SetItem is explicit that it steals a reference to the passed in value, and drops the reference for any existing entry: https://docs.python.org/3.3/c-api/list.html?highlight=m#PyList_SetItem

    The documentation for PyDict_SetItem leaves the semantics unspecified, forcing the reader to either make assumptions, or else go read the source code (as was done for the SO answer at https://stackoverflow.com/questions/40700251/reference-counting-using-pydict-setitemstring)

    Since the default assumption is actually correct, I don't think a Sphinx note is warranted, but an extra explicit sentence would be helpful.

    PySequence_SetItem has such a sentence already: "This function does *not* steal a reference to v."

    My suggestion is that we also add that sentence to the documentation for:

    • PyObject_SetItem
    • PyMapping_SetItemString
    • PyDict_SetItem
    • PyDict_SetItemString

    @ncoghlan ncoghlan added docs Documentation in the Doc dir 3.8 only security fixes 3.9 only security fixes labels Dec 29, 2019
    @ncoghlan ncoghlan added type-feature A feature request or enhancement docs Documentation in the Doc dir 3.8 only security fixes 3.9 only security fixes labels Dec 29, 2019
    @ncoghlan ncoghlan added the type-feature A feature request or enhancement label Dec 29, 2019
    @serhiy-storchaka
    Copy link
    Member

    The documentation for PyList_SetItem is explicit because its behavior is an exception from common rule and differs from PyList_SET_ITEM, so both should be documented explicitly.

    @ncoghlan
    Copy link
    Contributor Author

    Right, that's why I don't think the other "*SetItem*" operations should get a Sphinx note - just a sentence, as was already done for PySequence_SetItem.

    If it weren't for PyList_SetItem being different, none of the others would need the clarification at all.

    @ncoghlan
    Copy link
    Contributor Author

    New changeset e1e8000 by Joannah Nanjekye in branch 'master':
    bpo-39153: Clarify C API *SetItem refcounting semantics (GH-18220)
    e1e8000

    @miss-islington
    Copy link
    Contributor

    New changeset 526523c by Miss Islington (bot) in branch '3.8':
    bpo-39153: Clarify C API *SetItem refcounting semantics (GH-18220)
    526523c

    @corona10
    Copy link
    Member

    @nanjekyejoannah
    Thanks for the update :)

    @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
    3.8 only security fixes 3.9 only security fixes docs Documentation in the Doc dir type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    4 participants
    @ncoghlan @serhiy-storchaka @corona10 @miss-islington