Skip to content

gh-128715: Expose ctypes.CField, with info attributes#128950

Merged
encukou merged 35 commits intopython:mainfrom
encukou:ctypes-bitfield-3
Mar 24, 2025
Merged

gh-128715: Expose ctypes.CField, with info attributes#128950
encukou merged 35 commits intopython:mainfrom
encukou:ctypes-bitfield-3

Conversation

@encukou
Copy link
Member

@encukou encukou commented Jan 17, 2025

Expose the type of struct/union field descriptors (_ctypes._CField) as ctypes.CField. (This is mainly to make it easier to refer to it -- for typing and documentation. Creating CFields manually is not supported.)

Add attributes for easier introspection:

  • name & type, as defined in _fields_
  • byte_size & byte_offset, specify the chunk of memory to read/write
  • bit_size & bit_offset, which specify the shift & mask for bitfields
  • is_bitfield & is_anonymous booleans

The existing offset remains, as an alias for byte_offset.
The existing size is unchanged. Usually it is the same as byte_size, but for bitfields, it contains a bit-packed combination of bit_size & bit_offset.

Update the repr() of CField.

Use the same values internally as well. (Except bit_size, which might overflow Py_ssize_t for very large types. Instead, in C use bitfield_size, which is 0 for non-bitfields and thus serves as is_bitfield flag. Different name used clarity.)
Old names are removed from the C implementation to ensure a clean transition.
For simplicity, I keep byte_size in CFieldObject for now, even though it's redundant: it's the size of the underlying type.

Add a generic test that ensures the values are consistent, and that we don't have overlapping fields in structs. Use this check throughout test_ctypes, wherever a potentially interesting Structure or Union is created. (Tests for how simple structs behave after they're created don't need the checks, but I erred on the side of adding checks.)

Lift the restriction on maximum field size that was temporarily added in GH-126938. The max size is now Py_ssize_t.

This PR does not yet touch cfield.c getters & setters: the bit-packed “size” argument is computed and passed to those.


📚 Documentation preview 📚: https://cpython-previews--128950.org.readthedocs.build/

@encukou encukou marked this pull request as draft January 31, 2025 16:47
@encukou
Copy link
Member Author

encukou commented Feb 7, 2025

I'll continue next week :)

- Add alignment requirement
- Mention that ob_size is unreliable if you don't control it
- Add some links for context
- basicsize should include the base type in generaly not just PyObject

This adds a “by-the-way” link to `PyObject_New`, which shouldn't be
used for GC types. In order to be comfortable linking to it, I also
add a link to `PyObject_GC_New` from its docs. And the same for
`*Var` variants, while I'm here.
@picnixz
Copy link
Member

picnixz commented Feb 8, 2025

Should I review what you wrote now or do you prefer me to wait?

@encukou
Copy link
Member Author

encukou commented Feb 21, 2025

Sorry, I missed the comment! I'd appreciate a review.

The failure on 32-bit Debian is due to an existing bug, but such an exotic one that it's not worth fixing now: #130410
(It shows that the tests work, though!)

@encukou encukou marked this pull request as ready for review February 25, 2025 12:39
@encukou
Copy link
Member Author

encukou commented Mar 10, 2025

@picnixz, should I wait for your review?
No pressure, feel free to say no :)

@picnixz
Copy link
Member

picnixz commented Mar 10, 2025

Oh I forgot... I'll have maybe a bit of time before taking my plane or I'll be back on Wednesday. If you want to wait until then, I'll review it otherwise just go ahead!

Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments (hard to do a better review on a phone)

Copy link
Member Author

@encukou encukou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the review!

@encukou
Copy link
Member Author

encukou commented Mar 20, 2025

Thanks for the reviews everyone!
I'll merge tomorrow if there are no objections.

Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I've much more to say. My main concern was the unicode-exactness of the name which is now resolved so I'm fine.

@encukou encukou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 24, 2025
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @encukou for commit 5ce595c 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F128950%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 24, 2025
@encukou encukou merged commit 0e53038 into python:main Mar 24, 2025
122 of 123 checks passed
@encukou encukou deleted the ctypes-bitfield-3 branch March 24, 2025 13:18
@encukou
Copy link
Member Author

encukou commented Mar 24, 2025

Almost tomorrow.... :)

Thank you for the reviews, everyone!

diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 1, 2025
…-128950)

- Restore max field size to sys.maxsize, as in Python 3.13 & below
- PyCField: Split out bit/byte sizes/offsets.
- Expose CField's size/offset data to Python code
- Add generic checks for all the test structs/unions, using the newly exposed attrs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants