Skip to content

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

Merged
arigo merged 1 commit intopython-cffi:mainfrom
arigo:main
Dec 9, 2024
Merged

issue 147: force-compute nested structs before parent structs#148
arigo merged 1 commit intopython-cffi:mainfrom
arigo:main

Conversation

@arigo
Copy link
Copy Markdown
Contributor

@arigo arigo commented Dec 9, 2024

Occurs mainly in out-of-line ABI mode.

@mattip
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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