From ade4c77eed6d191448f5ca3cf36073bbe78b99c6 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Mon, 28 Apr 2025 22:13:47 +0200 Subject: [PATCH 1/5] gh-131747: ctypes: Warn when _pack_ implicitly changes default _layout_ --- Doc/deprecations/index.rst | 2 ++ Doc/deprecations/pending-removal-in-3.19.rst | 8 +++++++ Doc/library/ctypes.rst | 15 ++++++++++++- Doc/whatsnew/3.14.rst | 2 ++ Lib/ctypes/_layout.py | 21 +++++++++++++++++-- ...-04-28-18-26-37.gh-issue-131747.2AiQ9n.rst | 4 ++++ 6 files changed, 49 insertions(+), 3 deletions(-) create mode 100644 Doc/deprecations/pending-removal-in-3.19.rst create mode 100644 Misc/NEWS.d/next/C_API/2025-04-28-18-26-37.gh-issue-131747.2AiQ9n.rst diff --git a/Doc/deprecations/index.rst b/Doc/deprecations/index.rst index bac6e3f18d4594..e2a1684698c021 100644 --- a/Doc/deprecations/index.rst +++ b/Doc/deprecations/index.rst @@ -5,6 +5,8 @@ Deprecations .. include:: pending-removal-in-3.16.rst +.. include:: pending-removal-in-3.19.rst + .. include:: pending-removal-in-future.rst C API deprecations diff --git a/Doc/deprecations/pending-removal-in-3.19.rst b/Doc/deprecations/pending-removal-in-3.19.rst new file mode 100644 index 00000000000000..f163cce7c710ec --- /dev/null +++ b/Doc/deprecations/pending-removal-in-3.19.rst @@ -0,0 +1,8 @@ +Pending removal in Python 3.19 +------------------------------ + +* :mod:`ctypes`: + + * Implicitly switching to the MSVC-compatible struct layout by setting + :attr:`Structure._pack_` but not :attr:`Structure._layout_` + on non-Windows platforms. diff --git a/Doc/library/ctypes.rst b/Doc/library/ctypes.rst index 0bf88b0e3b66c6..934f25a0a07e18 100644 --- a/Doc/library/ctypes.rst +++ b/Doc/library/ctypes.rst @@ -2734,6 +2734,16 @@ fields, or any other data types containing pointer type fields. when :attr:`_fields_` is assigned, otherwise it will have no effect. Setting this attribute to 0 is the same as not setting it at all. + This is only implemented for the MSVC-compatible memory layout. + + .. deprecated-removed:: next 3.19 + + For historical reasons, if `_pack_` is non-zero, the MSVC-compatible + layout will be used by default. + On non-Windows platforms, this default is deprecated and is slated to + become an error in Python 3.19. + If it is intended, set :attr:`~Structure._layout_` to `'ms'`` + explicitly. .. attribute:: _align_ @@ -2762,12 +2772,15 @@ fields, or any other data types containing pointer type fields. Currently the default will be: - On Windows: ``"ms"`` - - When :attr:`~Structure._pack_` is specified: ``"ms"`` + - When :attr:`~Structure._pack_` is specified: ``"ms"``. + (This is deprecated; see :attr:`~Structure._pack_` documentation.) - Otherwise: ``"gcc-sysv"`` :attr:`!_layout_` must already be defined when :attr:`~Structure._fields_` is assigned, otherwise it will have no effect. + .. versionadded:: next + .. attribute:: _anonymous_ An optional sequence that lists the names of unnamed (anonymous) fields. diff --git a/Doc/whatsnew/3.14.rst b/Doc/whatsnew/3.14.rst index dad63e47a62dba..aea6bf66efda2c 100644 --- a/Doc/whatsnew/3.14.rst +++ b/Doc/whatsnew/3.14.rst @@ -1737,6 +1737,8 @@ Deprecated .. include:: ../deprecations/pending-removal-in-3.16.rst +.. include:: ../deprecations/pending-removal-in-3.19.rst + .. include:: ../deprecations/pending-removal-in-future.rst Removed diff --git a/Lib/ctypes/_layout.py b/Lib/ctypes/_layout.py index 0719e72cfed312..2048ccb6a1c93f 100644 --- a/Lib/ctypes/_layout.py +++ b/Lib/ctypes/_layout.py @@ -5,6 +5,7 @@ """ import sys +import warnings from _ctypes import CField, buffer_info import ctypes @@ -66,9 +67,26 @@ def get_layout(cls, input_fields, is_struct, base): # For clarity, variables that count bits have `bit` in their names. + pack = getattr(cls, '_pack_', None) + layout = getattr(cls, '_layout_', None) if layout is None: - if sys.platform == 'win32' or getattr(cls, '_pack_', None): + if sys.platform == 'win32': + gcc_layout = False + elif pack: + if is_struct: + base_type_name = 'Structure' + else: + base_type_name = 'Union' + warnings._deprecated( + '_pack_ without _layout_', + f"Due to '_pack_', the '{cls.__name__}' {base_type_name} will " + + "use memory layout compatible with MSVC (Windows). " + + "If this is intended, set _layout_ to 'ms'. " + + "The implicit default is deprecated and slated to become " + + "an error in Python {remove}.", + remove=(3, 19), + ) gcc_layout = False else: gcc_layout = True @@ -95,7 +113,6 @@ def get_layout(cls, input_fields, is_struct, base): else: big_endian = sys.byteorder == 'big' - pack = getattr(cls, '_pack_', None) if pack is not None: try: pack = int(pack) diff --git a/Misc/NEWS.d/next/C_API/2025-04-28-18-26-37.gh-issue-131747.2AiQ9n.rst b/Misc/NEWS.d/next/C_API/2025-04-28-18-26-37.gh-issue-131747.2AiQ9n.rst new file mode 100644 index 00000000000000..999b2642cb7bc1 --- /dev/null +++ b/Misc/NEWS.d/next/C_API/2025-04-28-18-26-37.gh-issue-131747.2AiQ9n.rst @@ -0,0 +1,4 @@ +On non-Windows platforms, deprecate using :attr:`ctypes.Structure._pack_` to +use a Windows-compatible layout on non-Windows platforms. The layout should +be specified explicitly by setting :attr:`ctypes.Structure._layout_` to +``'ms'``. From 850a16f9295e733317cdf3a731f55a7421bb8a82 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 30 Apr 2025 13:59:09 +0200 Subject: [PATCH 2/5] Add test & adjust existing cases --- .../test_ctypes/test_aligned_structures.py | 1 + Lib/test/test_ctypes/test_bitfields.py | 5 ++- Lib/test/test_ctypes/test_byteswap.py | 2 ++ .../test_ctypes/test_generated_structs.py | 11 ++++++- Lib/test/test_ctypes/test_pep3118.py | 3 ++ Lib/test/test_ctypes/test_structunion.py | 18 +++++++++++ Lib/test/test_ctypes/test_structures.py | 31 ++++++++++++------- .../test_ctypes/test_unaligned_structures.py | 2 ++ 8 files changed, 60 insertions(+), 13 deletions(-) diff --git a/Lib/test/test_ctypes/test_aligned_structures.py b/Lib/test/test_ctypes/test_aligned_structures.py index 0c563ab80559a6..50b4d729b9db8a 100644 --- a/Lib/test/test_ctypes/test_aligned_structures.py +++ b/Lib/test/test_ctypes/test_aligned_structures.py @@ -316,6 +316,7 @@ class Inner(sbase): class Main(sbase): _pack_ = 1 + _layout_ = "ms" _fields_ = [ ("a", c_ubyte), ("b", Inner), diff --git a/Lib/test/test_ctypes/test_bitfields.py b/Lib/test/test_ctypes/test_bitfields.py index dc81e752567c42..518f838219eba0 100644 --- a/Lib/test/test_ctypes/test_bitfields.py +++ b/Lib/test/test_ctypes/test_bitfields.py @@ -430,6 +430,7 @@ class TestStruct(Structure): def test_gh_84039(self): class Bad(Structure): _pack_ = 1 + _layout_ = "ms" _fields_ = [ ("a0", c_uint8, 1), ("a1", c_uint8, 1), @@ -443,9 +444,9 @@ class Bad(Structure): ("b1", c_uint16, 12), ] - class GoodA(Structure): _pack_ = 1 + _layout_ = "ms" _fields_ = [ ("a0", c_uint8, 1), ("a1", c_uint8, 1), @@ -460,6 +461,7 @@ class GoodA(Structure): class Good(Structure): _pack_ = 1 + _layout_ = "ms" _fields_ = [ ("a", GoodA), ("b0", c_uint16, 4), @@ -475,6 +477,7 @@ class Good(Structure): def test_gh_73939(self): class MyStructure(Structure): _pack_ = 1 + _layout_ = "ms" _fields_ = [ ("P", c_uint16), ("L", c_uint16, 9), diff --git a/Lib/test/test_ctypes/test_byteswap.py b/Lib/test/test_ctypes/test_byteswap.py index 072c60d53dd8cb..c4b97fa9f0e395 100644 --- a/Lib/test/test_ctypes/test_byteswap.py +++ b/Lib/test/test_ctypes/test_byteswap.py @@ -270,6 +270,7 @@ def test_unaligned_nonnative_struct_fields(self): class S(base): _pack_ = 1 + _layout_ = "ms" _fields_ = [("b", c_byte), ("h", c_short), @@ -297,6 +298,7 @@ def test_unaligned_native_struct_fields(self): class S(Structure): _pack_ = 1 + _layout_ = "ms" _fields_ = [("b", c_byte), ("h", c_short), diff --git a/Lib/test/test_ctypes/test_generated_structs.py b/Lib/test/test_ctypes/test_generated_structs.py index 9a8102219d8769..aa448fad5bbae6 100644 --- a/Lib/test/test_ctypes/test_generated_structs.py +++ b/Lib/test/test_ctypes/test_generated_structs.py @@ -125,18 +125,21 @@ class Nested(Structure): class Packed1(Structure): _fields_ = [('a', c_int8), ('b', c_int64)] _pack_ = 1 + _layout_ = 'ms' @register() class Packed2(Structure): _fields_ = [('a', c_int8), ('b', c_int64)] _pack_ = 2 + _layout_ = 'ms' @register() class Packed3(Structure): _fields_ = [('a', c_int8), ('b', c_int64)] _pack_ = 4 + _layout_ = 'ms' @register() @@ -155,6 +158,7 @@ def _maybe_skip(): _fields_ = [('a', c_int8), ('b', c_int64)] _pack_ = 8 + _layout_ = 'ms' @register() class X86_32EdgeCase(Structure): @@ -366,6 +370,7 @@ class Example_gh_95496(Structure): @register() class Example_gh_84039_bad(Structure): _pack_ = 1 + _layout_ = 'ms' _fields_ = [("a0", c_uint8, 1), ("a1", c_uint8, 1), ("a2", c_uint8, 1), @@ -380,6 +385,7 @@ class Example_gh_84039_bad(Structure): @register() class Example_gh_84039_good_a(Structure): _pack_ = 1 + _layout_ = 'ms' _fields_ = [("a0", c_uint8, 1), ("a1", c_uint8, 1), ("a2", c_uint8, 1), @@ -392,6 +398,7 @@ class Example_gh_84039_good_a(Structure): @register() class Example_gh_84039_good(Structure): _pack_ = 1 + _layout_ = 'ms' _fields_ = [("a", Example_gh_84039_good_a), ("b0", c_uint16, 4), ("b1", c_uint16, 12)] @@ -399,6 +406,7 @@ class Example_gh_84039_good(Structure): @register() class Example_gh_73939(Structure): _pack_ = 1 + _layout_ = 'ms' _fields_ = [("P", c_uint16), ("L", c_uint16, 9), ("Pro", c_uint16, 1), @@ -419,6 +427,7 @@ class Example_gh_86098(Structure): @register() class Example_gh_86098_pack(Structure): _pack_ = 1 + _layout_ = 'ms' _fields_ = [("a", c_uint8, 8), ("b", c_uint8, 8), ("c", c_uint32, 16)] @@ -528,7 +537,7 @@ def dump_ctype(tp, struct_or_union_tag='', variable_name='', semi=''): pushes.append(f'#pragma pack(push, {pack})') pops.append(f'#pragma pack(pop)') layout = getattr(tp, '_layout_', None) - if layout == 'ms' or pack: + if layout == 'ms': # The 'ms_struct' attribute only works on x86 and PowerPC requires.add( 'defined(MS_WIN32) || (' diff --git a/Lib/test/test_ctypes/test_pep3118.py b/Lib/test/test_ctypes/test_pep3118.py index 06b2ccecade44e..11a0744f5a8e36 100644 --- a/Lib/test/test_ctypes/test_pep3118.py +++ b/Lib/test/test_ctypes/test_pep3118.py @@ -81,6 +81,7 @@ class Point(Structure): class PackedPoint(Structure): _pack_ = 2 + _layout_ = 'ms' _fields_ = [("x", c_long), ("y", c_long)] class PointMidPad(Structure): @@ -88,6 +89,7 @@ class PointMidPad(Structure): class PackedPointMidPad(Structure): _pack_ = 2 + _layout_ = 'ms' _fields_ = [("x", c_byte), ("y", c_uint64)] class PointEndPad(Structure): @@ -95,6 +97,7 @@ class PointEndPad(Structure): class PackedPointEndPad(Structure): _pack_ = 2 + _layout_ = 'ms' _fields_ = [("x", c_uint64), ("y", c_byte)] class Point2(Structure): diff --git a/Lib/test/test_ctypes/test_structunion.py b/Lib/test/test_ctypes/test_structunion.py index 8d8b7e5e995132..5b21d48d99cef7 100644 --- a/Lib/test/test_ctypes/test_structunion.py +++ b/Lib/test/test_ctypes/test_structunion.py @@ -11,6 +11,8 @@ Py_TPFLAGS_DISALLOW_INSTANTIATION, Py_TPFLAGS_IMMUTABLETYPE) from struct import calcsize +import contextlib +from test.support import MS_WINDOWS class StructUnionTestBase: @@ -335,6 +337,22 @@ def test_methods(self): self.assertIn("from_address", dir(type(self.cls))) self.assertIn("in_dll", dir(type(self.cls))) + def test_pack_layout_switch(self): + # Setting _pack_ implicitly sets default layout to MSVC; + # this is deprecated on non-Windows platforms. + if MS_WINDOWS: + warn_context = contextlib.nullcontext() + else: + warn_context = self.assertWarns(DeprecationWarning) + with warn_context: + class X(self.cls): + _pack_ = 1 + # _layout_ missing + _fields_ = [('a', c_int8, 1), ('b', c_int16, 2)] + + # Check MSVC layout (bitfields of different types aren't combined) + self.check_sizeof(X, struct_size=3, union_size=2) + class StructureTestCase(unittest.TestCase, StructUnionTestBase): cls = Structure diff --git a/Lib/test/test_ctypes/test_structures.py b/Lib/test/test_ctypes/test_structures.py index bd7aba6376d167..5cecef21cfb301 100644 --- a/Lib/test/test_ctypes/test_structures.py +++ b/Lib/test/test_ctypes/test_structures.py @@ -25,6 +25,7 @@ class X(Structure): _fields_ = [("a", c_byte), ("b", c_longlong)] _pack_ = 1 + _layout_ = 'ms' self.check_struct(X) self.assertEqual(sizeof(X), 9) @@ -34,6 +35,7 @@ class X(Structure): _fields_ = [("a", c_byte), ("b", c_longlong)] _pack_ = 2 + _layout_ = 'ms' self.check_struct(X) self.assertEqual(sizeof(X), 10) self.assertEqual(X.b.offset, 2) @@ -45,6 +47,7 @@ class X(Structure): _fields_ = [("a", c_byte), ("b", c_longlong)] _pack_ = 4 + _layout_ = 'ms' self.check_struct(X) self.assertEqual(sizeof(X), min(4, longlong_align) + longlong_size) self.assertEqual(X.b.offset, min(4, longlong_align)) @@ -53,27 +56,33 @@ class X(Structure): _fields_ = [("a", c_byte), ("b", c_longlong)] _pack_ = 8 + _layout_ = 'ms' self.check_struct(X) self.assertEqual(sizeof(X), min(8, longlong_align) + longlong_size) self.assertEqual(X.b.offset, min(8, longlong_align)) - - d = {"_fields_": [("a", "b"), - ("b", "q")], - "_pack_": -1} - self.assertRaises(ValueError, type(Structure), "X", (Structure,), d) + with self.assertRaises(ValueError): + class X(Structure): + _fields_ = [("a", "b"), ("b", "q")] + _pack_ = -1 + _layout_ = "ms" @support.cpython_only def test_packed_c_limits(self): # Issue 15989 import _testcapi - d = {"_fields_": [("a", c_byte)], - "_pack_": _testcapi.INT_MAX + 1} - self.assertRaises(ValueError, type(Structure), "X", (Structure,), d) - d = {"_fields_": [("a", c_byte)], - "_pack_": _testcapi.UINT_MAX + 2} - self.assertRaises(ValueError, type(Structure), "X", (Structure,), d) + with self.assertRaises(ValueError): + class X(Structure): + _fields_ = [("a", c_byte)] + _pack_ = _testcapi.INT_MAX + 1 + _layout_ = "ms" + + with self.assertRaises(ValueError): + class X(Structure): + _fields_ = [("a", c_byte)] + _pack_ = _testcapi.UINT_MAX + 2 + _layout_ = "ms" def test_initializers(self): class Person(Structure): diff --git a/Lib/test/test_ctypes/test_unaligned_structures.py b/Lib/test/test_ctypes/test_unaligned_structures.py index 58a00597ef5cc4..b5fb4c0df77453 100644 --- a/Lib/test/test_ctypes/test_unaligned_structures.py +++ b/Lib/test/test_ctypes/test_unaligned_structures.py @@ -19,10 +19,12 @@ c_ushort, c_uint, c_ulong, c_ulonglong]: class X(Structure): _pack_ = 1 + _layout_ = 'ms' _fields_ = [("pad", c_byte), ("value", typ)] class Y(SwappedStructure): _pack_ = 1 + _layout_ = 'ms' _fields_ = [("pad", c_byte), ("value", typ)] structures.append(X) From 971c779f9047d4c62ebf8d5baa41a395460813e2 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 30 Apr 2025 14:07:01 +0200 Subject: [PATCH 3/5] What's New entry --- Doc/whatsnew/3.14.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Doc/whatsnew/3.14.rst b/Doc/whatsnew/3.14.rst index aea6bf66efda2c..38091998295a9a 100644 --- a/Doc/whatsnew/3.14.rst +++ b/Doc/whatsnew/3.14.rst @@ -1672,6 +1672,12 @@ Deprecated :func:`codecs.open` is now deprecated. Use :func:`open` instead. (Contributed by Inada Naoki in :gh:`133036`.) +* :mod:`ctypes`: + On non-Windows platforms, setting :attr:`.Structure._pack_` to use a + MSVC-compatible default memory layout is deprecated in favor of setting + :attr:`.Structure._layout_` to ``'ms'``. + (Contributed by Petr Viktorin in :gh:`131747`.) + * :mod:`functools`: Calling the Python implementation of :func:`functools.reduce` with *function* or *sequence* as keyword arguments is now deprecated. From 516d563891939b0bf3bbc4ad662a541e21434758 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 30 Apr 2025 15:10:12 +0200 Subject: [PATCH 4/5] Fix Sphinx warnings --- Doc/deprecations/pending-removal-in-3.19.rst | 2 +- Doc/library/ctypes.rst | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Doc/deprecations/pending-removal-in-3.19.rst b/Doc/deprecations/pending-removal-in-3.19.rst index f163cce7c710ec..3936f63ca5b5af 100644 --- a/Doc/deprecations/pending-removal-in-3.19.rst +++ b/Doc/deprecations/pending-removal-in-3.19.rst @@ -4,5 +4,5 @@ Pending removal in Python 3.19 * :mod:`ctypes`: * Implicitly switching to the MSVC-compatible struct layout by setting - :attr:`Structure._pack_` but not :attr:`Structure._layout_` + :attr:`~ctypes.Structure._pack_` but not :attr:`~ctypes.Structure._layout_` on non-Windows platforms. diff --git a/Doc/library/ctypes.rst b/Doc/library/ctypes.rst index 934f25a0a07e18..86f4f0877526bf 100644 --- a/Doc/library/ctypes.rst +++ b/Doc/library/ctypes.rst @@ -2738,11 +2738,12 @@ fields, or any other data types containing pointer type fields. .. deprecated-removed:: next 3.19 - For historical reasons, if `_pack_` is non-zero, the MSVC-compatible + For historical reasons, if :attr:`!_pack_` is non-zero, + the MSVC-compatible layout will be used by default. On non-Windows platforms, this default is deprecated and is slated to become an error in Python 3.19. - If it is intended, set :attr:`~Structure._layout_` to `'ms'`` + If it is intended, set :attr:`~Structure._layout_` to ``'ms'`` explicitly. .. attribute:: _align_ From 48261f65d375f14430fe98f27f602ee6f5f6e281 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Mon, 5 May 2025 14:33:06 +0200 Subject: [PATCH 5/5] Update Doc/library/ctypes.rst Co-authored-by: Peter Bierma --- Doc/library/ctypes.rst | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Doc/library/ctypes.rst b/Doc/library/ctypes.rst index 9428fcf4d85381..1b78b33b69f8da 100644 --- a/Doc/library/ctypes.rst +++ b/Doc/library/ctypes.rst @@ -2759,8 +2759,7 @@ fields, or any other data types containing pointer type fields. .. deprecated-removed:: next 3.19 For historical reasons, if :attr:`!_pack_` is non-zero, - the MSVC-compatible - layout will be used by default. + the MSVC-compatible layout will be used by default. On non-Windows platforms, this default is deprecated and is slated to become an error in Python 3.19. If it is intended, set :attr:`~Structure._layout_` to ``'ms'``