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

Failing cdef_multiple_inheritance_nodict test when CYTHON_USE_TYPE_SPECS=1 and CYTHON_USE_TYPE_SLOTS=0 #5696

Closed
mattip opened this issue Sep 12, 2023 · 4 comments · Fixed by #5698

Comments

@mattip
Copy link
Contributor

mattip commented Sep 12, 2023

Describe your issue

Working through the failures in PyPy when -DCYTHON_USE_TYPE_SPECS=1 (after #5594), I am down to one: cdef_multiple_inheritance_nodict. It hits this error:

unable to validate whether bases have a __dict__ when CYTHON_USE_TYPE_SLOTS is off (likely because you are building in the limited API). Therefore, all extension types with multiple bases must add 'cdef dict __dict__' in this compilation mode

from here. Am I correct that the failure is expected, and so the test should expect to hit that message when testing this configuration?

@da-woods
Copy link
Contributor

So my conclusion was that it isn't possible to use find out the dict offset without accessing tp_dictoffset. That definitely means you can't do it in the limited API.

I don't know whether we should make the guard

#if !CYTHON_USE_TYPE_SLOTS && CYTHON_COMPILING_IN_LIMITED_API

and just cheat and access tp_dictoffset directly otherwise.... (i.e. treat the macro as "avoid type slots wherever possible").

But yes - this seems like we'd currently expect it to fail. This is possibly somewhere where we could update or skip the test as well (we'll probably have to do this when we get as far as running the test in the limited api).

@scoder
Copy link
Contributor

scoder commented Sep 12, 2023

Can't type specs help here? Like in

else if (strcmp(memb->name, "__dictoffset__") == 0) {

@da-woods
Copy link
Contributor

I'd somehow missed that __dictoffset__ seems to be available via a normal Python attribute lookup. With that in mind this should be easily fixed

da-woods added a commit to da-woods/cython that referenced this issue Sep 12, 2023
Rather than just declaring it impossible.

Fixes cython#5696
scoder pushed a commit that referenced this issue Nov 1, 2023
@mattip
Copy link
Contributor Author

mattip commented Nov 1, 2023

Thanks

rinarakaki pushed a commit to rinarakaki/cython that referenced this issue Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants