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

Document PyFrame_FastToLocals() and PyFrame_FastToLocalsWithError() #63630

Closed
vstinner opened this issue Oct 29, 2013 · 15 comments
Closed

Document PyFrame_FastToLocals() and PyFrame_FastToLocalsWithError() #63630

vstinner opened this issue Oct 29, 2013 · 15 comments
Labels
docs Documentation in the Doc dir

Comments

@vstinner
Copy link
Member

BPO 19431
Nosy @birkenfeld, @pitrou, @vstinner, @jsbueno, @serhiy-storchaka, @csabella
Files
  • c_api_frame.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 2018-09-19.23:18:18.892>
    created_at = <Date 2013-10-29.09:55:35.849>
    labels = ['docs']
    title = 'Document PyFrame_FastToLocals() and PyFrame_FastToLocalsWithError()'
    updated_at = <Date 2018-09-20.13:06:58.331>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2018-09-20.13:06:58.331>
    actor = 'vstinner'
    assignee = 'docs@python'
    closed = True
    closed_date = <Date 2018-09-19.23:18:18.892>
    closer = 'vstinner'
    components = ['Documentation']
    creation = <Date 2013-10-29.09:55:35.849>
    creator = 'vstinner'
    dependencies = []
    files = ['32537']
    hgrepos = []
    issue_num = 19431
    keywords = ['patch']
    message_count = 15.0
    messages = ['201618', '202387', '300917', '300941', '308053', '308054', '308055', '308077', '308112', '308113', '308118', '308128', '309855', '325822', '325868']
    nosy_count = 7.0
    nosy_names = ['georg.brandl', 'pitrou', 'vstinner', 'jsbueno', 'docs@python', 'serhiy.storchaka', 'cheryl.sabella']
    pr_nums = []
    priority = 'normal'
    resolution = 'wont fix'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue19431'
    versions = ['Python 3.3', 'Python 3.4']

    @vstinner
    Copy link
    Member Author

    (Copy of an email) Georg Brandl via python.org

    Am 29.10.2013 01:19, schrieb victor.stinner:

    http://hg.python.org/cpython/rev/4ef4578db38a
    changeset: 86715:4ef4578db38a
    user: Victor Stinner <victor.stinner@gmail.com>
    date: Tue Oct 29 01:19:37 2013 +0100
    summary:
    Issue bpo-18408: Add a new PyFrame_FastToLocalsWithError() function to handle
    exceptions when merging fast locals into f_locals of a frame.
    PyEval_GetLocals() now raises an exception and return NULL on failure.

    You'll have to either make this private or document it.

    @vstinner vstinner added the docs Documentation in the Doc dir label Oct 29, 2013
    @vstinner
    Copy link
    Member Author

    vstinner commented Nov 7, 2013

    c_api_frame.patch: document some C functions of the frame object in the C API.

    @csabella
    Copy link
    Contributor

    Victor,

    Should there be a PR for this?

    @serhiy-storchaka
    Copy link
    Member

    PyFrameObject already is documented in Doc/c-api/veryhigh.rst.
    PyFrame_GetLineNumber() already is documented in Doc/c-api/reflection.rst.

    PyFrame_FastToLocals() and PyFrame_LocalsToFast() are not documented and have weird interface. I think the use of them should be discouraged.

    @vstinner
    Copy link
    Member Author

    Should there be a PR for this?

    Feel free to create a PR from my old (4 years old) patch. Just mention my name in the commit message please.

    @vstinner
    Copy link
    Member Author

    PyFrame_FastToLocals() and PyFrame_LocalsToFast() are not documented and have weird interface. I think the use of them should be discouraged.

    I suggest to document them, but explain in the documentation that they must not be used :-)

    @vstinner
    Copy link
    Member Author

    Ah, PyFrame_GetLineNumber is now documented at Doc/c-api/reflection.rst. But PyFrame_New() is still not documented. So my patch is not completely useful.

    @cheryl: Maybe convert the PR without PyFrame_FastToLocals() and PyFrame_FastToLocalsWithError().

    @csabella
    Copy link
    Contributor

    Thanks Victor and Serhiy.

    > @cheryl: Maybe convert the PR without PyFrame_FastToLocals() and PyFrame_FastToLocalsWithError().

    If I only convert PyFrame_New() then I would need to add it to as existing page since the patch created a new page for Frame Objects. I think the right place would be under the current PyFrameObject in Doc/c-api/veryhigh.rst?

    Thanks!

    @vstinner
    Copy link
    Member Author

    If I only convert PyFrame_New() then I would need to add it to as existing page since the patch created a new page for Frame Objects. I think the right place would be under the current PyFrameObject in Doc/c-api/veryhigh.rst?

    Not, it's a very high level API, it's a low level API which should be referenced in the concrete.rst page, as I didn in my patch.

    I think it's ok to have a page with only two functions: PyFrame_New() and PyFrame_GetLineNumber().

    Move https://docs.python.org/dev/c-api/reflection.html#c.PyFrame_GetLineNumber into your new doc, but leave something like "See also :c:func:`PyFrame_GetLineNumber`." there.

    @vstinner
    Copy link
    Member Author

    Serhiy: how are you supposed to modify local variables of a frame when these variables are stored in "fast locals"? Even if it's a rare useful, I think that it's ok to expose PyFrame_FastToLocalsWithError(), and maybe also PyFrame_FastToLocals().

    It is useful you want write a debugger in pure C, and don't want to bother with fast locals special case.

    @serhiy-storchaka
    Copy link
    Member

    frameobject.h is not included in any header file. Some effort was spent for avoiding including it in ceval.h, genobject.h, pystate.h and traceback.h. The whole content of frameobject.h is not available in the limited API. I'm not sure about the status of this API. This is not a very hight level API, but rather a very low level API. If document it, it should be documented on a separate page or on a page together with other low-level API.

    Serhiy: how are you supposed to modify local variables of a frame when these variables are stored in "fast locals"?

    Currently you should call PyFrame_FastToLocalsWithError(), modify directly f_locals, and call PyFrame_LocalsToFast(). This is very low-level manipulation. It exposes implementation details (at least you need to access PyFrameObject attributes directly). It should be documented that the most of PyFrame_* API is low-level and implementation specific. PyFrame_GetLineNumber() is an exception.

    @vstinner
    Copy link
    Member Author

    Currently you should call PyFrame_FastToLocalsWithError(), modify directly f_locals, and call PyFrame_LocalsToFast().

    When happens if PyFrame_LocalsToFast() isn't called? Does it crash or is it just slower?

    @jsbueno
    Copy link
    Mannequin

    jsbueno mannequin commented Jan 12, 2018

    This discussion is fresh, so maybe it is worth asking here prior to python-ideas:

    In Python we can change any global variable, object attribute or mapping-value with function calls. Locals and nonlocals are the only exceptions and from time to time that gets in the way of clever oneliners, and it is just plain asymmetric.

    What do you say of adding a wrapper to this as an oficial Python function in the stdlib? Maybe inspect.setlocal() that could set f_locals and call this?? That would provide a workaround to the asymmetry that locals currently experiment.

    It would not impose any extra security risks, since this can be called via ctypes already, and also it is not any more subject to abuse than setattr or globals()[...] = can already be abused.

    @vstinner
    Copy link
    Member Author

    It seems like these functions should be documented, so I close the issue.

    @vstinner
    Copy link
    Member Author

    I closed the issue because we decided to not document the function, but I'm still interested to mark the API is private. Such change is more sensitive, so I added it my larger "New C API" project which works on finding a way to deprecate functions and provide maybe helpers for backward and forward compatibility:
    https://pythoncapi.readthedocs.io/bad_api.html#for-internal-use-only

    @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
    docs Documentation in the Doc dir
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants