Fix UnboundLocalError when all struct packings fail in calc_packing (#942)

When every packing attempt in calc_packing() raises PackingError, the
final 'raise PackingError(f"PACKING FAILED: {details}")' referenced the
'except ... as details' target after it had gone out of scope.

Per Python 3 semantics the exception target is deleted at the end of the
except block, so 'details' was unbound at the raise, producing an
UnboundLocalError that masked the real PackingError and its message.

Keep the last PackingError in a separate variable so the final raise
reports the actual layout failure. Add regression tests for calc_packing.

Fixes #937

Author: ryanda9910

comtypes/test/test_packing.py

@@ -0,0 +1,49 @@
+import unittest
+
+from comtypes.tools import typedesc
+from comtypes.tools.codegenerator.packing import PackingError, calc_packing
+
+
+# Sizes, alignments and offsets are expressed in bits, matching the values
+# produced by ``comtypes.tools.tlbparser`` (e.g. a 32-bit ``int``).
+def _make_field(name, size, align, offset):
+    typ = typedesc.FundamentalType("int", size, align)
+    return typedesc.Field(name, typ, None, offset)
+
+
+class CalcPackingTest(unittest.TestCase):
+    def test_returns_none_for_default_packing(self):
+        # A naturally laid out single-field struct needs no explicit packing.
+        field = _make_field("a", 32, 32, 0)
+        struct = typedesc.Structure(
+            "Good", align=32, members=[field], bases=[], size=32
+        )
+        self.assertIsNone(calc_packing(struct, [field]))
+
+    def test_incomplete_struct_returns_none(self):
+        field = _make_field("a", 32, 32, 0)
+        struct = typedesc.Structure(
+            "Incomplete", align=32, members=[field], bases=[], size=None
+        )
+        self.assertIsNone(calc_packing(struct, [field]))
+
+    def test_raises_packing_error_with_details_when_layout_is_inconsistent(self):
+        # The declared field offset does not match the computed layout, so every
+        # packing attempt raises ``PackingError`` and ``calc_packing`` must
+        # re-raise with the underlying reason.
+        #
+        # Regression test for gh-937: the final ``raise`` referenced the
+        # ``except ... as`` target after it had been cleared, raising
+        # ``UnboundLocalError`` and masking the real ``PackingError``.
+        field = _make_field("x", 32, 32, 64)
+        struct = typedesc.Structure("Bad", align=32, members=[field], bases=[], size=96)
+        with self.assertRaises(PackingError) as ctx:
+            calc_packing(struct, [field])
+        message = str(ctx.exception)
+        self.assertIn("PACKING FAILED", message)
+        # The original failure reason must be preserved, not lost.
+        self.assertIn("field x offset", message)
+
+
+if __name__ == "__main__":
+    unittest.main()

comtypes/tools/codegenerator/packing.py

@@ -44,18 +44,23 @@ def _calc_packing(struct, fields, pack, isStruct):
 def calc_packing(struct, fields):
     # try several packings, starting with unspecified packing
     isStruct = isinstance(struct, typedesc.Structure)
+    last_error = None
     for pack in [None, 16 * 8, 8 * 8, 4 * 8, 2 * 8, 1 * 8]:
         try:
             _calc_packing(struct, fields, pack, isStruct)
         except PackingError as details:
+            # The exception target is cleared when the ``except`` block ends
+            # (see https://docs.python.org/3/reference/compound_stmts.html#except-clause),
+            # so keep a reference for the final error message below.
+            last_error = details
             continue
         else:
             if pack is None:
                 return None
 
             return int(pack / 8)
 
-    raise PackingError(f"PACKING FAILED: {details}")
+    raise PackingError(f"PACKING FAILED: {last_error}")
 
 
 class PackingError(Exception):