gh-131747: ctypes: Deprecate _pack_ implicitly setting _layout_ = 'ms' (GH-133205)

encukou · ZeroIntensity · web-flow · commit 59f78d7b0609 · 2025-05-05T15:32:06.000+02:00
On non-Windows, warn when _pack_ implicitly changes default _layout_
to 'ms'.

Co-authored-by: Peter Bierma &lt;zintensitydev@gmail.com&gt;
diff --git a/Doc/deprecations/index.rst b/Doc/deprecations/index.rst
@@ -7,6 +7,8 @@ Deprecations
 
 .. include:: pending-removal-in-3.17.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
@@ -0,0 +1,8 @@
+Pending removal in Python 3.19
+------------------------------
+
+* :mod:`ctypes`:
+
+  * Implicitly switching to the MSVC-compatible struct layout by setting
+    :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
@@ -2754,6 +2754,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 :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'``
+         explicitly.
 
    .. attribute:: _align_
 
@@ -2782,12 +2792,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
@@ -1874,6 +1874,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:`ctypes`:
   Calling :func:`ctypes.POINTER` on a string is deprecated.
   Use :ref:`ctypes-incomplete-types` for self-referential structures.
@@ -1948,6 +1954,8 @@ Deprecated
 
 .. include:: ../deprecations/pending-removal-in-3.17.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
@@ -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/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
@@ -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
@@ -269,6 +269,7 @@ def test_unaligned_nonnative_struct_fields(self):
 
         class S(base):
             _pack_ = 1
+            _layout_ = "ms"
             _fields_ = [("b", c_byte),
                         ("h", c_short),
 
@@ -296,6 +297,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
@@ -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,13 +398,15 @@ 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)]
 
 @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
@@ -81,20 +81,23 @@ class Point(Structure):
 
 class PackedPoint(Structure):
     _pack_ = 2
+    _layout_ = 'ms'
     _fields_ = [("x", c_long), ("y", c_long)]
 
 class PointMidPad(Structure):
     _fields_ = [("x", c_byte), ("y", c_uint)]
 
 class PackedPointMidPad(Structure):
     _pack_ = 2
+    _layout_ = 'ms'
     _fields_ = [("x", c_byte), ("y", c_uint64)]
 
 class PointEndPad(Structure):
     _fields_ = [("x", c_uint), ("y", c_byte)]
 
 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
@@ -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
@@ -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
@@ -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)
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
@@ -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'``.
