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

issue 147: force-compute nested structs before parent structs #148

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

arigo
Copy link
Contributor

@arigo arigo commented Dec 9, 2024

Occurs mainly in out-of-line ABI mode.

@arigo arigo merged commit b57a92c into python-cffi:main Dec 9, 2024
12 checks passed
@mattip
Copy link
Contributor

mattip commented Dec 28, 2024

Any idea how to port this to PyPy? The code there is different: there is no explicit CT_IS_OPAQUE flag. Even more complicated: I am not sure how to write a untranslated test for this.

@arigo
Copy link
Contributor Author

arigo commented Dec 29, 2024

The PyPy logic is different, so maybe there isn't anything to fix. Does the original test case in #147 work on a translated PyPy?

@arigo
Copy link
Contributor Author

arigo commented Dec 29, 2024

Found the same logic in complete_struct_or_union() in newtype.py, but also found comments in the class W_CTypeStructOrUnion that make me think that PyPy, unlike CFFI on CPython, doesn't use ftype->ct_size < 0 if the struct type is lazy. If that's correct, then there is indeed nothing to fix.

@mattip
Copy link
Contributor

mattip commented Dec 29, 2024

The test fails on py3.10. I tried various incantations to try to cause the struct to be evaluated when size == -2, which I think is supposed to be "lazy", but failed. I also could not figure out how to run the test untranslated.

@arigo
Copy link
Contributor Author

arigo commented Dec 29, 2024

Here's a blind attempt at writing an untranslated test (completely untested):

diff -r 2eefecd93d0f pypy/module/_cffi_backend/test/test_re_python.py
--- a/pypy/module/_cffi_backend/test/test_re_python.py  Mon Oct 16 08:36:02 2023 +0200
+++ b/pypy/module/_cffi_backend/test/test_re_python.py  Sun Dec 29 18:54:07 2024 +0100
@@ -32,6 +32,7 @@
         struct foo_s;
         typedef struct bar_s { int x; signed char a[]; } bar_t;
         enum foo_e { AA, BB, CC };
+        struct B { struct C* c; }; struct C { struct B b; };

         void init_test_re_python(void) { }      /* windows hack */
         void PyInit__test_re_python(void) { }   /* windows hack */
@@ -270,3 +271,15 @@

         err = lib1.dlclose(handle)
         assert err == 0
+
+    def test_rec_structs_1(self):
+        self.fix_path()
+        from re_python_pysrc import ffi
+        sz = ffi.sizeof("struct B")
+        assert sz == ffi.sizeof("int *")
+
+    def test_rec_structs_2(self):
+        self.fix_path()
+        from re_python_pysrc import ffi
+        sz = ffi.sizeof("struct C")
+        assert sz == ffi.sizeof("int *")

@mattip
Copy link
Contributor

mattip commented Jan 3, 2025

Yup, thanks @arigo that worked. The test and fix are in this commit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants