Skip to content

gh-131747: ctypes: Deprecate _pack_ implicitly setting _layout_ = 'ms' #133205

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Doc/deprecations/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions Doc/deprecations/pending-removal-in-3.19.rst
Original file line number Diff line number Diff line change
@@ -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.
16 changes: 15 additions & 1 deletion Doc/library/ctypes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2734,6 +2734,17 @@ 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_

Expand Down Expand Up @@ -2762,12 +2773,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.
Expand Down
8 changes: 8 additions & 0 deletions Doc/whatsnew/3.14.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -1739,6 +1745,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
Expand Down
21 changes: 19 additions & 2 deletions Lib/ctypes/_layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"""

import sys
import warnings

from _ctypes import CField, buffer_info
import ctypes
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down
1 change: 1 addition & 0 deletions Lib/test/test_ctypes/test_aligned_structures.py
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@ class Inner(sbase):

class Main(sbase):
_pack_ = 1
_layout_ = "ms"
_fields_ = [
("a", c_ubyte),
("b", Inner),
Expand Down
5 changes: 4 additions & 1 deletion Lib/test/test_ctypes/test_bitfields.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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),
Expand All @@ -460,6 +461,7 @@ class GoodA(Structure):

class Good(Structure):
_pack_ = 1
_layout_ = "ms"
_fields_ = [
("a", GoodA),
("b0", c_uint16, 4),
Expand All @@ -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),
Expand Down
2 changes: 2 additions & 0 deletions Lib/test/test_ctypes/test_byteswap.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),

Expand Down Expand Up @@ -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),
Expand Down
11 changes: 10 additions & 1 deletion Lib/test/test_ctypes/test_generated_structs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -155,6 +158,7 @@ def _maybe_skip():

_fields_ = [('a', c_int8), ('b', c_int64)]
_pack_ = 8
_layout_ = 'ms'

@register()
class X86_32EdgeCase(Structure):
Expand Down Expand Up @@ -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),
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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)]
Expand Down Expand Up @@ -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) || ('
Expand Down
3 changes: 3 additions & 0 deletions Lib/test/test_ctypes/test_pep3118.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
18 changes: 18 additions & 0 deletions Lib/test/test_ctypes/test_structunion.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
31 changes: 20 additions & 11 deletions Lib/test/test_ctypes/test_structures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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))
Expand All @@ -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):
Expand Down
2 changes: 2 additions & 0 deletions Lib/test/test_ctypes/test_unaligned_structures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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'``.
Loading