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

Correct definition of __slots__ on structs (must be defined in class body) #183

Open
mara004 opened this issue Aug 13, 2023 · 6 comments
Open

Comments

@mara004
Copy link
Contributor

mara004 commented Aug 13, 2023

Currently what struct output looks like with ctypesgen is this

class struct__FPDF_SYSTEMTIME(Structure):
    pass
struct__FPDF_SYSTEMTIME.__slots__ = [
    'wYear',
    'wMonth',
    'wDayOfWeek',
    'wDay',
    'wHour',
    'wMinute',
    'wSecond',
    'wMilliseconds',
]
struct__FPDF_SYSTEMTIME._fields_ = [
    ('wYear', c_ushort),
    ('wMonth', c_ushort),
    ('wDayOfWeek', c_ushort),
    ('wDay', c_ushort),
    ('wHour', c_ushort),
    ('wMinute', c_ushort),
    ('wSecond', c_ushort),
    ('wMilliseconds', c_ushort),
]
FPDF_SYSTEMTIME = struct__FPDF_SYSTEMTIME

We can see that field names are effectively listed twice, which increases file length unnecessarily.
__slots__ can be easily derived from _fields_ using a list comprehension, like this

structname.__slots__ = [n for n, _ in structname._fields_]

Apart from that, what are __slots__ supposed to achieve here? AFAICS, they're never mentioned in the ctypes docs.
And according to my testing, they don't prevent setting non-existent fields.

@nilason
Copy link
Collaborator

nilason commented Aug 13, 2023

We can see that field names are effectively listed twice, which increases file length unnecessarily. __slots__ can be easily derived from _fields_ using a list comprehension, like this

structname.__slots__ = [n for n, _ in structname._fields_]

Looks like a good idea to me.

Apart from that, what are __slots__ supposed to achieve here? AFAICS, they're never mentioned in the ctypes docs. And according to my testing, they don't prevent setting non-existent fields, although this myth exists.

It is a Python feat: https://wiki.python.org/moin/UsingSlots

@mara004
Copy link
Contributor Author

mara004 commented Aug 13, 2023

https://wiki.python.org/moin/UsingSlots indeed claims

with the default __dict__, a misspelled variable name results in the creation of a new variable, but with __slots__ it raises in an AttributeError.

This does not seem to hold true with structs, though...

@nilason
Copy link
Collaborator

nilason commented Aug 13, 2023

class struct__FPDF_SYSTEMTIME(Structure):

It is a class, isn’t it?

@mara004
Copy link
Contributor Author

mara004 commented Aug 14, 2023

It is a class, isn’t it?

I suppose it is?

>>> import pypdfium2.raw
>>> import inspect
>>> inspect.isclass(pypdfium2.raw.FPDF_SYSTEMTIME)
True

However, structs do seem a bit odd, according to https://stackoverflow.com/q/76887146/15547292 overriding __setattr__ also does not work as expected.

@mara004
Copy link
Contributor Author

mara004 commented Aug 14, 2023

Aah, the slots definition needs to be in the class body, like so

>>> import ctypes
>>> class FPDF_SYSTEMTIME (ctypes.Structure):
...     _fields_ = [("wYear", ctypes.c_ushort)]
...     __slots__ = ["wYear"]
... 
>>> st = FPDF_SYSTEMTIME()
>>> st.w_year = 1
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'FPDF_SYSTEMTIME' object has no attribute 'w_year'
>>> st.wYear = 1

Then it actually prevents bad fields as expected.

@mara004 mara004 changed the title Simplify (or remove?) definition of __slots__ on structs Correct definition of __slots__ on structs (must be defined in class body) Aug 14, 2023
@mara004
Copy link
Contributor Author

mara004 commented Nov 12, 2023

See pypdfium2-team@5148408 for corrected fix.

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

No branches or pull requests

2 participants