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

[C API] PyType_GetSlot cannot get tp_name #86201

Closed
fancitron mannequin opened this issue Oct 14, 2020 · 22 comments
Closed

[C API] PyType_GetSlot cannot get tp_name #86201

fancitron mannequin opened this issue Oct 14, 2020 · 22 comments
Labels
topic-C-API type-feature A feature request or enhancement

Comments

@fancitron
Copy link
Mannequin

fancitron mannequin commented Oct 14, 2020

BPO 42035
Nosy @vstinner, @encukou, @corona10, @shihai1991
PRs
  • bpo-42035: Add a PyType_GetName() to get type's short name. #23903
  • bpo-42035: Add a PyType_GetQualName() to get type's qualified name. #27551
  • bpo-42035: Enhance test_get_type_name() of _testcapi. #27649
  • 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 2021-08-17.15:33:17.695>
    created_at = <Date 2020-10-14.13:52:05.070>
    labels = ['expert-C-API', 'type-feature']
    title = '[C API] PyType_GetSlot cannot get tp_name'
    updated_at = <Date 2021-08-17.15:33:17.694>
    user = 'https://bugs.python.org/fancitron'

    bugs.python.org fields:

    activity = <Date 2021-08-17.15:33:17.694>
    actor = 'petr.viktorin'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-08-17.15:33:17.695>
    closer = 'petr.viktorin'
    components = ['C API']
    creation = <Date 2020-10-14.13:52:05.070>
    creator = 'fancitron'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 42035
    keywords = ['patch']
    message_count = 22.0
    messages = ['378616', '379102', '379109', '379291', '379416', '383070', '385274', '385281', '385282', '385390', '385433', '386080', '398292', '398311', '398466', '398468', '398766', '399261', '399749', '399755', '399757', '399760']
    nosy_count = 5.0
    nosy_names = ['vstinner', 'petr.viktorin', 'corona10', 'shihai1991', 'fancitron']
    pr_nums = ['23903', '27551', '27649']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue42035'
    versions = []

    @fancitron
    Copy link
    Mannequin Author

    fancitron mannequin commented Oct 14, 2020

    In the Limited API (where PyTypeObject is opaque), there is no way to retrieve the tp_name of a type object. The PyType_GetSlot() function doesn’t define a slot ID Py_tp_name. This makes it inconvenient to port existing code to the Limited API. Is this intentional? How to work around this?

    @fancitron fancitron mannequin added topic-C-API type-feature A feature request or enhancement labels Oct 14, 2020
    @vstinner vstinner changed the title PyType_GetSlot cannot get tp_name [C API] PyType_GetSlot cannot get tp_name Oct 14, 2020
    @vstinner vstinner changed the title PyType_GetSlot cannot get tp_name [C API] PyType_GetSlot cannot get tp_name Oct 14, 2020
    @encukou
    Copy link
    Member

    encukou commented Oct 20, 2020

    The slots are originally intended for defining types (PyType_FromSpec); PyType_GetSlot is not as useful as it could be.
    tp_name can be exposed, but it needs to also be handled properly PyType_FromSpec -- e.g. raise an error.

    @encukou
    Copy link
    Member

    encukou commented Oct 20, 2020

    Ah, scratch that: PyType_GetSlot returns a function pointer.
    To be correct, we should to expose a new function like PyType_GetName.

    It's true that CPython currently doesn't always honor the distinction between data and function pointers, but the C standard says they're distinct, so it might bite us in the future.

    @fancitron
    Copy link
    Mannequin Author

    fancitron mannequin commented Oct 22, 2020

    True enough. Btw, PyType_FromSpec accepts Py_tp_doc (char *), Py_tp_base (PyTypeObject *), etc ... so to be strictly standard compliant, a union would be necessary.

    PyType_GetName() sounds great.

    One "proper" workaround at the moment is PyObject_GetAttrString(Py_TYPE(x), "__name__") and then process the result. This is somewhat "heavy" and strips the module name. "Py_tp_name" provides a convenient, exception safe, and backward compatible way to access tp_name.

    What I actually do right now is to access the (opaque) PyTypeObject::tp_name by pointer offset. This certain defies the purpose of stable ABI!

    @encukou
    Copy link
    Member

    encukou commented Oct 23, 2020

    Yes, for some cases the ship has sailed. I don't think we can add unions now -- the stable ABI needs to be stable.

    @encukou
    Copy link
    Member

    encukou commented Dec 15, 2020

    I'll be happy to review adding a PyType_GetName function.

    @encukou
    Copy link
    Member

    encukou commented Jan 19, 2021

    Now that I see the implementation (and now that I'm spending a lot of time trying to formalize what is good stable API), I see a problem with PyType_GetName: it effectively returns a borrowed reference.
    The proposed docs say:

    Callers can hold [the retuned] pointer until the type has been deallocated.

    This is not friendly to alternate Python implementations. For example, if tp_name is stored as UTF-32, it would need to generate the char* data -- and then retain it until the class is deallocated.
    I guess the "correct" way would be to return a Py_buffer, which would (in CPython) reference the class.

    Victor, what do you think?

    @vstinner
    Copy link
    Member

    New C API functions must not return borrowed references, but strong references.

    @vstinner
    Copy link
    Member

    A type has different names:

    • short name: "name"
    • qualified name: "module.name"

    Which one should be returned by PyType_GetName()? Is there a warranty that it's always short or always qualified?

    @shihai1991
    Copy link
    Member

    New C API functions must not return borrowed references, but strong references.

    Thanks petr, victor for your suggestion. It's more friendly to users.

    Which one should be returned by PyType_GetName()? Is there a warranty that it's always short or always qualified?

    Returning short or qualified name depends on how to define the tp_name in PyType_Spec or type object. IMHO, PyType_GetName() return the original defined type name is fine to users.

    @shihai1991
    Copy link
    Member

    Wait. petr mentioned _PyType_Name in PR 23903 which use the short name. So I am not sure which way is better. Lol~

    @shihai1991
    Copy link
    Member

    I found type.__name__ has the implementation details in https://github.com/python/cpython/blob/master/Objects/typeobject.c#L486.
    IMHO, keep the consistency in PyType_GetName() is OK.

    Victor, Petr. Do you think it make senses?

    @encukou
    Copy link
    Member

    encukou commented Jul 27, 2021

    Sorry for the delay; getting 652 into Python 3.10 took up most of my time.

    So, in the current proposal:

    • PyType_GetName(t) is equivalent to PyObject_GetAttrString(t, "__name__")
    • for the qualified name you can use PyObject_GetAttrString(t, "__qualname__")
    • there is still no way to get t.tp_name

    The advantage of PyType_GetName over a regular getattr is that it's fast for heap types (there's no dict lookup, and the string object is cached as ht_name). For static types, it's a bit slower (a string object needs to be built, but still there's no dict lookup).

    If we go this way, why not add PyType_GetQualName as well?

    (Is the speed, compared to getattr, worth adding two new functions? I guess it is, barely.)

    ----

    The OP specifically requested a way to get tp_name, which the current PR does not do. I don't think it should, either:

    • there's no way to assure that char* tp_name is not deallocated before the caller is done with it (especially considering that other Python implementations should be able to implement the stable ABI, and those shouldn't need store tp_name as char*).
    • there is already name and qualname (and module); tp_name is a weird mix that I, personally, won't miss if it goes away. I think it's OK to only use it for creation (as PyType_Spec.name), and then expose the name/module derived from it. It's not how CPython works, but that's OK for the limited API.

    @shihai1991
    Copy link
    Member

    • there is still no way to get t.tp_name
      Can we create PyType_GetDataSlot function to return the data pointer?

    If we go this way, why not add PyType_GetQualName as well?
    +1. I will create another PR after PR-23903 merged.

    @encukou
    Copy link
    Member

    encukou commented Jul 29, 2021

    Can we create PyType_GetDataSlot function to return the data pointer?

    No, see the borrowed pointer issues above.

    I will create another PR after PR-23903 merged.

    Great, thank you!

    @encukou
    Copy link
    Member

    encukou commented Jul 29, 2021

    New changeset a390ebe by Hai Shi in branch 'main':
    bpo-42035: Add a PyType_GetName() to get type's short name. (GH-23903)
    a390ebe

    @shihai1991
    Copy link
    Member

    • for the qualified name you can use PyObject_GetAttrString(t, "__qualname__")

    After PR-27551 merged, we can use PyType_GetQualName() to get type's __qualname__.

    @vstinner
    Copy link
    Member

    vstinner commented Aug 9, 2021

    PyType_GetName() is a nice addition, thanks.

    @encukou
    Copy link
    Member

    encukou commented Aug 17, 2021

    New changeset 3e2c643 by Hai Shi in branch 'main':
    bpo-42035: Add PyType_GetQualName() to get a type's qualified name. (GH-27551)
    3e2c643

    @encukou
    Copy link
    Member

    encukou commented Aug 17, 2021

    New changeset fcd651d by Hai Shi in branch 'main':
    bpo-42035: Enhance test_get_type_name() of _testcapi (GH-27649)
    fcd651d

    @encukou
    Copy link
    Member

    encukou commented Aug 17, 2021

    Now, I wonder if we should also introduce PyType_GetModuleName for __module__, or stop and close this as fixed.

    @shihai1991
    Copy link
    Member

    Now, I wonder if we should also introduce PyType_GetModuleName for __module__, or stop and close this as fixed.

    IMO, I suggest to close this bpo. We can get the tp_name by the C API now. If there have user want get __module__, we can open a new bpo to discuss :)

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    topic-C-API type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants