Skip to content

Commit

Permalink
Improve patching ReferenceFix to POINTER(POINTER(interface)) type. (
Browse files Browse the repository at this point in the history
#631)

* Refactor patching `ReferenceFix`
to `POINTER(POINTER(interface))` type.

In e6281cc, the approach of overriding the `__setitem__` method of `POINTER(p)`
(i.e., `POINTER(POINTER(interface))`) was changed from directly overwriting it
to using `partial.partial`.
In f8e9855, `super` was introduced in the `__setitem__` method.
In 3f99d4c, `partial.partial` was replaced with `patcher.Patch`.

For a while, the code seemed to work, but a `NameError` started being raised
due to `super(_, self)`, so in 32d21e3, it was changed to
`super(POINTER(p), self)`.

When passing a type `T` to `POINTER`, the caching mechanism ensures that the
same `POINTER(T)` type is always returned.
Passing `POINTER(T)` to `POINTER` and obtaining `POINTER(POINTER(T))` (which is
`LP_POINTER(T)` in `repr`) can also be considered an equivalent relationship.
Looking at the implementation of `ctypes.SetPointerType` (which is undocumented
and has become deprecated), it doesn't seem intended to alter the relationship
between `T` and `POINTER(T)`.

Considering the historical background of these changes, the implementation of
`ctypes`, and its use cases, it appears that `POINTER(p)` invoked within
`__setitem__` was not intended to be evaluated later than the `POINTER(p)`
passed to `patcher.Patch`.
Even when assigning `pp = POINTER(p)` to a temporary variable and passing this
`pp` to both `patcher.Patch` and `super` within `__setitem__`, the tests pass.

Therefore, I determined that this change does not present an implementation
problem.
This also resolves the inconsistency where a `ptr_type` is actually being
passed, despite the method name is `..._to_ptrptr_type`.

* Add a `type: ignore` comment.
  • Loading branch information
junkmd authored Oct 2, 2024
1 parent 711a9d6 commit ca14c63
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions comtypes/_post_coinit/unknwn.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def __new__(cls, name, bases, namespace):

if self._case_insensitive_:
self._patch_case_insensitive_to_ptr_type(p)
self._patch_reference_fix_to_ptrptr_type(p)
self._patch_reference_fix_to_ptrptr_type(POINTER(p)) # type: ignore

return self

Expand Down Expand Up @@ -134,8 +134,8 @@ def __setattr__(self, name, value):
)

@staticmethod
def _patch_reference_fix_to_ptrptr_type(p: Type) -> None:
@patcher.Patch(POINTER(p))
def _patch_reference_fix_to_ptrptr_type(pp: Type) -> None:
@patcher.Patch(pp)
class ReferenceFix(object):
def __setitem__(self, index, value):
# We override the __setitem__ method of the
Expand All @@ -157,7 +157,7 @@ def __setitem__(self, index, value):
# CopyComPointer should do if index != 0.
if bool(value):
value.AddRef()
super(POINTER(p), self).__setitem__(index, value) # type: ignore
super(pp, self).__setitem__(index, value) # type: ignore
return
from _ctypes import CopyComPointer

Expand Down

0 comments on commit ca14c63

Please sign in to comment.