fix: generate signed byte types for VT_I1

comtypes/test/test_cpp_byte_compatibility.py

@@ -0,0 +1,149 @@
+"""Byte and string compatibility through generated interfaces and native COM."""
+
+import unittest
+from ctypes import (
+    byref,
+    c_byte,
+    c_char_p,
+    c_void_p,
+    cast,
+    create_string_buffer,
+    pointer,
+)
+
+from comtypes import CLSCTX_LOCAL_SERVER
+from comtypes.client import CreateObject, GetModule
+from comtypes.malloc import _CoTaskMemFree
+
+try:
+    GetModule(("{07D2AEE5-1DF8-4D2C-953A-554ADFD25F99}", 1, 0, 0))
+    from comtypes.gen.ComtypesCppTestSrvLib import (
+        CoByteCompatibilityTest,
+        IByteCompatibilityTest,
+    )
+
+    IMPORT_SERVER_FAILED = False
+except (ImportError, OSError):
+    IMPORT_SERVER_FAILED = True
+
+
+@unittest.skipIf(IMPORT_SERVER_FAILED, "Requires the C++ out-of-process COM server.")
+class _NativeByteTest(unittest.TestCase):
+    def setUp(self):
+        self.server = CreateObject(
+            CoByteCompatibilityTest,
+            clsctx=CLSCTX_LOCAL_SERVER,
+            interface=IByteCompatibilityTest,
+        )
+
+
+class ByteCompatibilityTest(_NativeByteTest):
+    def test_negative_int8(self):
+        for value in (-128, -7, -1):
+            with self.subTest(value=value):
+                self.assertEqual(self.server.EchoInt8(value), value)
+
+    def test_nonnegative_int8(self):
+        for value in (0, 1, 127):
+            with self.subTest(value=value):
+                self.assertEqual(self.server.EchoInt8(value), value)
+
+    def test_uint8(self):
+        for value in (0, 1, 42, 128, 255):
+            with self.subTest(value=value):
+                self.assertEqual(self.server.EchoUint8(value), value)
+
+    def test_lpstr(self):
+        for value in (b"", b"hello", b"\x80\xff"):
+            with self.subTest(value=value):
+                result = c_char_p()
+                try:
+                    # Use the generated raw method to retain the allocated address.
+                    # The high-level result is bytes, which loses that address.
+                    self.server._IByteCompatibilityTest__com_EchoLpStr(
+                        value, byref(result)
+                    )
+                    self.assertEqual(result.value, value)
+                finally:
+                    _CoTaskMemFree(cast(result, c_void_p))
+
+
+class LegacyMappingCompatibilityTest(_NativeByteTest):
+    """Calls accepted by the old mapping that regress with the proposed mapping."""
+
+    @unittest.expectedFailure
+    def test_char_pointer_bytes_input(self):
+        # Plain char* was previously inferred as STRING. Changing VT_I1 to
+        # signed char loses this implicit bytes input, unlike explicit LPSTR.
+        # Keep the compatibility regression visible for the review decision.
+        self.assertEqual(self.server.ReadCharPointer(b"h"), 104)
+
+    @unittest.expectedFailure
+    def test_character_bytes_output_buffer(self):
+        # Old STRING output accepts a char buffer. Reserve the terminator
+        # locally because a plain char* marshals only one character.
+        result = create_string_buffer(2)
+        self.server._IByteCompatibilityTest__com_GetCharacterBytes(104, result)
+        self.assertEqual(result.value, b"h")
+
+
+class ProposedMappingCompatibilityTest(_NativeByteTest):
+    """Numeric arguments and results supported by the proposed signed mapping."""
+
+    def test_char_pointer_input(self):
+        for value, expected in ((0, 0), (104, 104), (-128, 128), (-1, 255)):
+            with self.subTest(value=value):
+                self.assertEqual(
+                    self.server.ReadCharPointer(pointer(c_byte(value))), expected
+                )
+
+    def test_char_reference_input(self):
+        for value, expected in ((0, 0), (104, 104), (-128, 128), (-1, 255)):
+            with self.subTest(value=value):
+                self.assertEqual(
+                    self.server.ReadCharPointer(byref(c_byte(value))), expected
+                )
+
+    def test_char_array_input(self):
+        for value, expected in ((0, 0), (104, 104), (-128, 128), (-1, 255)):
+            with self.subTest(value=value):
+                self.assertEqual(
+                    self.server.ReadCharPointer((c_byte * 1)(value)), expected
+                )
+
+    def test_character_pointer_output(self):
+        for value, expected in ((0, 0), (1, 1), (104, 104), (128, -128), (255, -1)):
+            with self.subTest(value=value):
+                # Plain char* output uses the signed numeric mapping as well.
+                self.assertEqual(self.server.GetCharacterBytes(value), expected)
+
+    def test_character_pointer_output_buffer(self):
+        for value, expected in ((0, 0), (104, 104), (128, -128), (255, -1)):
+            with self.subTest(value=value):
+                result = pointer(c_byte())
+                self.server._IByteCompatibilityTest__com_GetCharacterBytes(
+                    value, result
+                )
+                self.assertEqual(result.contents.value, expected)
+
+    def test_character_reference_output_buffer(self):
+        for value, expected in ((0, 0), (104, 104), (128, -128), (255, -1)):
+            with self.subTest(value=value):
+                result = c_byte()
+                self.server._IByteCompatibilityTest__com_GetCharacterBytes(
+                    value, byref(result)
+                )
+                self.assertEqual(result.value, expected)
+
+    def test_character_array_output_buffer(self):
+        for value, expected in ((0, 0), (104, 104), (128, -128), (255, -1)):
+            with self.subTest(value=value):
+                result = (c_byte * 1)()
+                self.server._IByteCompatibilityTest__com_GetCharacterBytes(
+                    value, result
+                )
+                self.assertEqual(result[0], expected)
+
+
+if __name__ == "__main__":
+    unittest.main()

comtypes/test/test_cpp_safearray.py

@@ -35,6 +35,7 @@ def setUp(self):
         )
         # Fixed expected values come from the C++ fixture, not Python input.
         self.cases_1d = [
+            (self.server.GetInt8Array, "int8", (-7, 0, 42, -7, 0, 42)),
             (self.server.GetUint8Array, "uint8", (0, 42, 255, 0, 42, 255)),
             (self.server.GetInt16Array, "int16", (-7, 0, 42, -7, 0, 42)),
             (self.server.GetUint16Array, "uint16", (0, 42, 65535, 0, 42, 65535)),
@@ -63,6 +64,7 @@ def setUp(self):
         ]
         # SAFEARRAY storage is column-major; the fixture has two rows.
         self.cases_2d = [
+            (self.server.GetInt8Array, "int8", ((-7, 42, 0), (0, -7, 42))),
             (self.server.GetUint8Array, "uint8", ((0, 255, 42), (42, 0, 255))),
             (self.server.GetInt16Array, "int16", ((-7, 42, 0), (0, -7, 42))),
             (self.server.GetUint16Array, "uint16", ((0, 65535, 42), (42, 0, 65535))),
@@ -140,18 +142,6 @@ def test_ndarray_values_2d(self):
                 numpy.testing.assert_array_equal(result, expected)
                 self.assertEqual(method(2, 6), expected)
 
-    def test_int8_codegen_1d(self):
-        # VT_I1 is currently parsed as c_char (see #935), whose pointer slice
-        # returns bytes. Check the real COM/code-generator path and raw bytes;
-        # signed int8/NumPy dtype assertions await the separate c_char/c_byte fix.
-        result = self.server.GetInt8Array(1, 6)
-        self.assertEqual(result, (249, 0, 42, 249, 0, 42))
-
-    def test_int8_codegen_2d(self):
-        # As above, 249 is the raw byte for -7, not a signed dtype assertion.
-        result = self.server.GetInt8Array(2, 6)
-        self.assertEqual(result, ((249, 42, 0), (0, 249, 42)))
-
 
 if __name__ == "__main__":
     unittest.main()

comtypes/tools/tlbparser.py

@@ -21,6 +21,7 @@ def PTR(typ):
 
 # basic C data types, with size and alignment in bits
 char_type = typedesc.FundamentalType("char", 8, 8)
+schar_type = typedesc.FundamentalType("signed char", 8, 8)
 uchar_type = typedesc.FundamentalType("unsigned char", 8, 8)
 wchar_t_type = typedesc.FundamentalType("wchar_t", 16, 16)
 short_type = typedesc.FundamentalType("short int", 16, 16)
@@ -81,7 +82,7 @@ def midlSAFEARRAY(typ):
     automation.VT_VARIANT: VARIANT_type,  # 12
     automation.VT_UNKNOWN: PTR(IUNKNOWN_type),  # 13
     automation.VT_DECIMAL: DECIMAL_type,  # 14
-    automation.VT_I1: char_type,  # 16
+    automation.VT_I1: schar_type,  # 16
     automation.VT_UI1: uchar_type,  # 17
     automation.VT_UI2: ushort_type,  # 18
     automation.VT_UI4: ulong_type,  # 19

docs/source/byte_compatibility.rst

@@ -0,0 +1,76 @@
+Signed 8-bit values from type libraries
+=======================================
+
+This page describes the compatibility impact of the proposed ``VT_I1``
+type-library mapping change. The change is under review; the examples below
+describe regenerated modules using that mapping.
+
+Generated types
+---------------
+
+``GetModule`` now represents ``VT_I1`` with ``ctypes.c_byte`` (a signed 8-bit
+integer) instead of ``ctypes.c_char``. This affects scalar values, pointers,
+and SAFEARRAY elements described by ``VT_I1`` in a type library. For example,
+an IDL ``small`` scalar and a plain ``char*`` pointer use this mapping.
+Explicit string types such as ``LPSTR`` still generate ``STRING``
+(``ctypes.c_char_p``); ``VT_UI1`` still generates ``ctypes.c_ubyte``.
+
+The change applies when modules are generated again. Existing generated
+source files and interfaces defined directly in Python retain their declared
+types. Mixing old generated modules with new ones can therefore produce
+different Python argument and return types for the same type library.
+
+Arguments
+---------
+
+Pass a Python integer in the range -128 to 127, or a ``ctypes.c_byte`` value,
+to a signed 8-bit scalar argument. The generated retval for a signed scalar
+is also numeric: an ``EchoInt8(-7)`` call returns the integer ``-7``.
+Single-character ``bytes`` and ``ctypes.c_char`` arguments do not match the
+new scalar type.
+
+A plain ``char*`` argument previously generated as ``STRING`` now generates
+as ``POINTER(c_byte)``. Existing calls that pass ``b"h"`` to such an argument
+raise ``ctypes.ArgumentError``. Use a pointer, a reference, or an array of
+``c_byte`` values instead, according to the native method's buffer contract::
+
+    from ctypes import byref, c_byte, pointer
+
+    value = c_byte(ord("h"))
+    server.ReadCharPointer(pointer(value))
+    server.ReadCharPointer(byref(value))
+    server.ReadCharPointer((c_byte * 1)(ord("h")))
+
+These examples refer to a method that reads exactly one character. For
+methods that read more data, provide the required buffer size and lifetime.
+The pointer type alone does not describe a string's length or termination.
+Explicit ``LPSTR`` arguments continue to accept ``bytes`` as before.
+
+Return values
+-------------
+
+A plain ``char*`` output also changes from ``STRING`` to ``POINTER(c_byte)``.
+For a method returning one character through that parameter, a generated
+high-level call now returns an integer. The value 104 returns as 104, and
+255 returns as -1. Under the old mapping the automatic output allocation
+could instead fail with ``TypeError: c_char_p 'out' parameter must be passed
+as default value``.
+
+Calls to the generated raw COM method previously accepted a writable
+``ctypes.create_string_buffer`` for that output. The new pointer type rejects
+that character array. Supply ``byref(c_byte())`` or a ``c_byte`` array for a
+single-character output, and read the resulting signed number. To recover
+the corresponding one-byte ``bytes`` value, use ``bytes([value & 0xff])``.
+
+For larger returned buffers, read the result using the native method's
+documented length. ``ctypes.string_at(result, length)`` makes a ``bytes`` copy
+from a pointer of known length. Use NUL-terminated string conversion only
+when the native contract guarantees termination. A plain character pointer
+does not guarantee a terminator. Free or release native allocations as
+specified by that contract after copying the data.
+
+Explicit ``LPSTR`` output types retain their existing string conversion.
+Changing interface declarations by hand should likewise follow the actual
+native contract. Type libraries describe both numeric ``small`` and plain
+character data using ``VT_I1``, so this mapping change cannot by itself
+preserve every existing implicit string conversion.

docs/source/index.rst

@@ -30,6 +30,7 @@ Functionalities
     npsupport
     threading
     type_checking
+    byte_compatibility
 
 
 Links

source/CppTestSrv/CoByteCompatibilityTest.cpp

@@ -0,0 +1,84 @@
+#include <objbase.h>
+#include <string.h>
+#include "CoByteCompatibilityTest.h"
+
+HRESULT CByteCompatibilityTest::CreateInstance(IUnknown* pUnknownOuter, CUnknown** ppNewComponent)
+{
+	if (pUnknownOuter != NULL)
+	{
+		return CLASS_E_NOAGGREGATION ;
+	}
+	*ppNewComponent = new CByteCompatibilityTest(pUnknownOuter) ;
+	return S_OK ;
+}
+
+HRESULT __stdcall CByteCompatibilityTest::NondelegatingQueryInterface(const IID& iid, void** ppv)
+{
+	if (iid == IID_IByteCompatibilityTest)
+	{
+		return FinishQI(static_cast<IByteCompatibilityTest*>(this), ppv) ;
+	}
+	return CUnknown::NondelegatingQueryInterface(iid, ppv) ;
+}
+
+HRESULT __stdcall CByteCompatibilityTest::EchoInt8(char value, char* result)
+{
+	if (result == NULL)
+	{
+		return E_POINTER ;
+	}
+	*result = value ;
+	return S_OK ;
+}
+
+HRESULT __stdcall CByteCompatibilityTest::EchoUint8(byte value, byte* result)
+{
+	if (result == NULL)
+	{
+		return E_POINTER ;
+	}
+	*result = value ;
+	return S_OK ;
+}
+
+HRESULT __stdcall CByteCompatibilityTest::EchoLpStr(LPSTR value, LPSTR* result)
+{
+	if (result == NULL)
+	{
+		return E_POINTER ;
+	}
+	*result = NULL ;
+	if (value == NULL)
+	{
+		return E_INVALIDARG ;
+	}
+	size_t size = strlen(value) + 1 ;
+	*result = static_cast<LPSTR>(CoTaskMemAlloc(size)) ;
+	if (*result == NULL)
+	{
+		return E_OUTOFMEMORY ;
+	}
+	memcpy(*result, value, size) ;
+	return S_OK ;
+}
+
+HRESULT __stdcall CByteCompatibilityTest::ReadCharPointer(unsigned char* value, byte* result)
+{
+	if (value == NULL || result == NULL)
+	{
+		return E_POINTER ;
+	}
+	// A raw char* marshals one character, not a NUL-terminated string.
+	*result = static_cast<byte>(*value) ;
+	return S_OK ;
+}
+
+HRESULT __stdcall CByteCompatibilityTest::GetCharacterBytes(byte value, unsigned char* result)
+{
+	if (result == NULL)
+	{
+		return E_POINTER ;
+	}
+	*result = value ;
+	return S_OK ;
+}

source/CppTestSrv/CoByteCompatibilityTest.h

@@ -0,0 +1,20 @@
+// A separate COM component for byte and narrow-string compatibility.
+#include "CUnknown.h"
+#include "Iface.h"
+
+class CByteCompatibilityTest : public CUnknown, public IByteCompatibilityTest
+{
+public:
+	static HRESULT CreateInstance(IUnknown* pUnknownOuter, CUnknown** ppNewComponent) ;
+
+private:
+	DECLARE_IUNKNOWN
+	virtual HRESULT __stdcall NondelegatingQueryInterface(const IID& iid, void** ppv) ;
+	virtual HRESULT __stdcall EchoInt8(char value, char* result) ;
+	virtual HRESULT __stdcall EchoUint8(byte value, byte* result) ;
+	virtual HRESULT __stdcall EchoLpStr(LPSTR value, LPSTR* result) ;
+	virtual HRESULT __stdcall ReadCharPointer(unsigned char* value, byte* result) ;
+	virtual HRESULT __stdcall GetCharacterBytes(byte value, unsigned char* result) ;
+
+	CByteCompatibilityTest(IUnknown* pUnknownOuter) : CUnknown(pUnknownOuter) {} ;
+} ;