FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Subtypes of extension type appear to inherit Py_TPFLAGS_DISALLOW_INSTANTIATION flag · Issue #96746 · python/cpython · GitHub

Repository navigation

Subtypes of extension type appear to inherit Py_TPFLAGS_DISALLOW_INSTANTIATION flag #96746

Description

When Py_TPFLAGS_BASETYPE and Py_TPFLAGS_DISALLOW_INSTANTIATION flags are set on extension type tp_flags field, attempt to create type subclass instance fails:

>>> import _foo
>>> class Bar(_foo.Foo): pass
... 
>>> Bar()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: cannot create 'Bar' instances

According to documentation, it shouldn't happen:
https://docs.python.org/3/c-api/typeobj.html#Py_TPFLAGS_DISALLOW_INSTANTIATION

Extension code example:
_foo.c.txt

Tested on 3.10.3 and 3.12.0a0.

Activity

  1. vstinner commented on Sep 12, 2022

    Member
  2. erlend-aasland commented on Oct 25, 2022

    Contributor

    Changing this behaviour might break existing code. Perhaps it would be better to update the docs to match the actual behaviour. Alternatively, we could issue a warning for this in 3.12 and change the behaviour in 3.14.

    I'm not sure what's the most correct (or less painful) solution.

  3. chgnrdv commented on Oct 26, 2022

    ContributorAuthor

    @erlend-aasland Can we update the docs for 3.10/3.11/3.12 to match behaviour, but for 3.12 mention that inheritance is deprecated and will be removed in 3.14, and make 3.12 issue a deprecation warning on attempt to instantiate subclass of non-instantiable?

  4. erlend-aasland commented on Oct 26, 2022

    Contributor

    The 3.10, 3.11 and 3.12 docs should be updated. Regarding deprecation, it is not for me to decide alone. Please raise a topic in the Core Development category on Discourse.

  5. encukou commented on Oct 27, 2022

    Member

    IMO the Python example above should fail. But the flag isn't responsible directly: this fails because the flag prevented inheriting tp_new, and so there is no C-level constructor to call. There's no way Python code can safely work around that.

    If a subclass provides a tp_new (from C), then it should be possible to instantiate it – the subclass knows how to create its instances in that case. (I haven't checked if there's a bug there as well, though.)

  6. erlend-aasland commented on Oct 27, 2022

    Contributor

    IMO the Python example above should fail. But the flag isn't responsible directly: this fails because the flag prevented inheriting tp_new, and so there is no C-level constructor to call. There's no way Python code can safely work around that.

    Ok, so we should probably just leave the current behaviour as it is, if I am reading you right. In any case, the docs should be updated to reflect the implementation.

  7. chgnrdv commented on Oct 27, 2022

    ContributorAuthor

    I agree that there is misconception on my part, and subclass doesn't actually inherit said flag (i. e. doesn't have it in its tp_flags) but have its tp_new set to value of the same field of its base class, which is NULL if base class is not instantiable.

    My opinion is that if flag is not inheritable, it shouldn't affect ability of subclasses to be instantiable. The current situation is that ability to instantiate class depends on presence of flag in base class.
    And, as far as I know, there is no way to make such subclass instantiable in pure Python.

    Also I think that non-instantiable base classes with instantiable subclasses can be useful in some situations, e. g. as abstract classes defined in extensions.

    Now I see that probably there is no bug here, but there still may be a little feature :)

  8. erlend-aasland commented on Oct 28, 2022

    Contributor

    Now I see that probably there is no bug here, but there still may be a little feature :)

    If you think such a feature is worth it, raise a discussion first on Discourse. If you gain support for you idea, create a new type-feature issue for you idea here on the tracker.

    Suggesting closing this.

  9. added
    pendingThe issue will be closed if no feedback is provided
    on Oct 28, 2022
  10. encukou commented on Nov 2, 2022

    Member

    And, as far as I know, there is no way to make such subclass instantiable in pure Python.

    That's intentional. A big use case for Py_TPFLAGS_DISALLOW_INSTANTIATION is that there's some necessary setup at the C level – setting C pointers rather than Python attributes. Allowing Python subclasses would mean that this initialization could be bypassed.

    Also I think that non-instantiable base classes with instantiable subclasses can be useful in some situations, e. g. as abstract classes defined in extensions.

    Py_TPFLAGS_DISALLOW_INSTANTIATION isn't the tool for that.
    In this case, the tp_new would check if type(self) is the base class, and raise an exception in that case.

    The flag's docs should mention that – they're currently unclear unless you know the internals already. I'll turn this into a docs issue.

    Thanks for reporting it! Hopefully we can clear up some confusion for everyone :)

  11. added
    docsDocumentation in the Doc dir
    and removed
    pendingThe issue will be closed if no feedback is provided
    on Nov 2, 2022
  12. changed the title [-]Subtypes of extension type inherit Py_TPFLAGS_DISALLOW_INSTANTIATION flag[/-] [+]Subtypes of extension type appear to inherit Py_TPFLAGS_DISALLOW_INSTANTIATION flag[/+] on Nov 2, 2022
  13. added a commit that references this issue on Nov 2, 2022
  14. encukou commented on Nov 2, 2022

    Member
  15. chgnrdv commented on Nov 2, 2022

    ContributorAuthor

    @encukou Yes, looks helpful to me. Thank you!

  16. added a commit that references this issue on Nov 7, 2022
  17. added 2 commits that reference this issue on Nov 7, 2022
  18. added 2 commits that reference this issue on Nov 7, 2022
  19. encukou commented on Nov 7, 2022

    Member

    The docs were updated. Thanks again for bringing this up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

docsDocumentation in the Doc dir

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions


    Back | FazBrowse Home | New Git URL