Menu

#28 Supported Py_LIMITED_API

None
pending
None
5
2018-08-08
2017-11-11
No

This patch provides a way to support stable Python ABI: https://www.python.org/dev/peps/pep-0384/

1 Attachments

Related

Patches: #28

Discussion

  • Barry Alan Scott

    Thank you very much for the patch this is a feature that I want PyCXX to support.

    I have commited r386 to update the build-all.sh and setup_makefile.py to support testing this feature. running ./build-all.sh will test pycxx against all installed python versions.

    I see this error with python 3.6 and will work on that.

    ./CXX/Python3/ExtensionType.hxx:313:38: error: ‘PyType_GetSlot’ was not declared in this scope
    ((freefunc)PyType_GetSlot( _self->ob_type, Py_tp_free ))( _self );
    ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    ./CXX/Python3/ExtensionType.hxx:313:38: note: suggested alternative: ‘PyType_Slot’

    Barry

     
  • Pavel Ostyakov

    Pavel Ostyakov - 2017-11-18

    You should define Py_LIMITED_API version 3.4 since limited abi for Python versions 3.3 or less does not support calling tp_dealloc: https://bugs.python.org/issue17162

    So, just define Py_LIMITED_API as 0x03040000

     
  • Barry Alan Scott

    Yep, I've added the ability to test with different values of Py_LIMITED_API and defaulted to 0x0304000 and added a error to say that it most be 3.4 or later.

    I have been busy and need to get back to this.

    I need to understand why you have reordered/added calls in some of the patch. For example to readyType() in ExtensionOldType.hxx. Either the call is not needed or being missing is a bug.

    As I say I need to find the time to get back into this.

     
  • Pavel Ostyakov

    Pavel Ostyakov - 2017-11-28

    In case of limited API, we should call PyType_FromSpec to declare a new type. This logic is implemented in readyType(), so, we call it in ExtensionOldType because users don't call readyType() when defining old-type classes using PyCXX

     
  • Barry Alan Scott

    I'd like to understand what is goig wrong here.

    Why is that protected by an #ifdef Py_LIMITED_API?
    How do I demonstrate that it is a bug without the change?

     
  • Pavel Ostyakov

    Pavel Ostyakov - 2017-11-29

    It is protected by this #ifdef because we don't need to call PyType_Ready when declaring old-type classes. However, with Py_LIMITED_API, we should dynamically allocate new type because there are no another way to declare a new type except of calling PyType_FromSpec. The best way to do it is calling readyType() in initialization.

     
  • Barry Alan Scott

    But that means that the behviour is different it Py_LIMITED_API is defined that is surely a bad thing to do?

    Are you saying that new-style classes cannot be defined with the Py_LIMITED_API?

     
  • Pavel Ostyakov

    Pavel Ostyakov - 2017-12-06

    No, the behaviour is not different for users. It's just the details of internal implementation

     
  • Barry Alan Scott

    I'm working thought the Py_LIMITED_API bit by bit getting my head around it.

    Where possible I want to have a single imlpementation of a feature by using the limited API only.
    I'm not sure about how far I will get attempting this.

     
  • Andrey Saitgalin

    Hello, Barry!

    How the things are going with this patch? This would be very useful for us, we are already using it in production libraries but it is not yet in upstream and there is no progress here. We reviewed this patch very carefully in three pairs of eyes and are really looking forward to seeing this patch accepted! :)

    Thanks much,
    Andrey

     
  • Barry Alan Scott

    Thaks for the reminder. This dropped off my todo list.
    So far I have researched the API and found out why your patch does what it does.
    I recall finding some gaps in the patch that I started fixing.

    I wil try and find the time to get this committed soon.

     
  • Andrey Saitgalin

    Hello, Barry! Just a friendly reminder :)

     
  • Andrey Saitgalin

    Hello, Barry! Any news here?

     
  • Barry Alan Scott

    I have committed r388 with a modified version of your patch.
    Please test in your environment and let me know is there are any problems.

    I found that readyType() must be called at the end of the init_type() functions.
    I have no idea why this was not neccesary in the !defined(Py_LIMITED_API) case.

    All tests pass for all python from 2.7, 3.4 to 3.7. With limit api for 3.4 to 3.7.
    There are new build scripts to run the build&test for limited and unlimited API
    cases.

     
  • Barry Alan Scott

    • status: open --> pending
    • assigned_to: Barry Alan Scott
    • Group: -->
     
  • Andrey Saitgalin

    Thanks a lot Barry! I will test all changes in our project and send a reply

     
  • Barry Alan Scott

    Andrey can I have the test report please? Its been more then a month.

     
  • Andrey Saitgalin

    Hello, Barry!

    Sorry for such a long answer. I carefully compared PyCXX trunk and our version, run tests and everything seems fine in our codebase! Thanks!

    I just want to comment some places in code which from my point of view could be handled more carefully.

    1. https://sourceforge.net/p/cxx/code/HEAD/tree/trunk/CXX/CXX/Python3/Objects.hxx#l970
      Why have you brought Complex in Python 3 under Py_LIMITED_API ifdef? Complex numbers are available in limited api as far as I can see it in Python 3 source code. For example, PyComplex_FromDoubles function is available and could be used.

    2. As I can see _PyPackageContext is not under Py_LIMITED_API ifdef now, isn't it a mistake? In Python 3 sources it is not available in public limited api:

    #ifndef Py_LIMITED_API
    PyAPI_DATA(const char *) _Py_PackageContext;
    #endif
    

    Kind regards,
    Andrey.

     
  • Barry Alan Scott

    Thanks for the review.
    1. I have put Complex back in, there are some parts of that class that need to be protected with Py_LIMITED_API that use Py_complex.
    2. I missed only one place that PyPackageCOntext was not under Py_LIMITED_API, fixed.

    commited as r394 and r395.

     

Log in to post a comment.

Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.