Add a helper for ctypes function annotations

[LeonarddeR]

Link to issue number:

Related to #11125

Summary of the issue:

Windows DLL functions are a headache for type checkers, as these functions are not annotated with types.

Description of user facing changes:

None

Description of developer facing changes:

Better type checking for functions exported from dlls.

Description of development approach:

Create a function decorator that ensures the following:

1. The function has proper argument names and types and return type.
2. The underlying ctypes function pointer is properly annotated with restype and argtypes
3. Proper support for input and output parameters.

Testing strategy:

• Unit tests
• Integrate the prototype into existing module,, e.g. visionEnhancementProviders.screenCurtain.Magnification.

Known issues with pull request:

None

Code Review Checklist:

• Documentation:
  • Change log entry
  • User Documentation
  • Developer / Technical Documentation
  • Context sensitive help for GUI changes
• Testing:
  • Unit tests
  • System (end to end) tests
  • Manual testing
• UX of all users considered:
  • Speech
  • Braille
  • Low Vision
  • Different web browsers
  • Localization in other languages / culture than English
• API is compatible with existing add-ons.
• Security precautions taken.

@coderabbitai summary

[SaschaCowley]

@LeonarddeR I've just skimmed the code here. I've not provided a thorough review because I know you said this was very much a prototype. Here are my thoughts (I've not included nitpick comments or anything):

1. What do you think the performance overhead of this is going to be? If it is going to be significant, do you think there's the opportunity for caching or other strategies to mitigate it? I don't think we need to go down that road just yet (premature optimisation and all that), but it's something to think about.
2. I noticed you used ctypes.cdll. I assume this strategy will work equally well for ctypes.windll. Is that right?
3. I noticed several uses of sunder-attributes of ctypes (`ctypes._*) in the code. Are those attributes intended for public use? I am concerned they could change at any moment and break this code.
4. Have you looked into whether there are alternatives to this that do not involve writing and maintaining a whole load of boilerplate? For instance, could we use a tool to generate .pyi files automatically with type information for files with ctypes bindings in the winBindings package, NVDAHelper.localLib module, and wherever else necessary?

@seanbudd and @michaelDCurran any thoughts here? We have spoken about wanting something like this internally. Do you think this approach is viable?

[gexgd0419]

There are third-party modules like py-win32more, which has Python bindings for most of the Win32 APIs. The function prototypes are generated from win32metadata, and are categorized into several .py files, with decorators that parse the annotation and turn the "placeholder" functions into callable functions.

Some of the placeholder functions inside `win32more.Windows.Win32.Storage.FileSystem`

@winfunctype('KERNEL32.dll')
def CreateDirectoryA(lpPathName: win32more.Windows.Win32.Foundation.PSTR, lpSecurityAttributes: POINTER(win32more.Windows.Win32.Security.SECURITY_ATTRIBUTES)) -> win32more.Windows.Win32.Foundation.BOOL: ...
@winfunctype('KERNEL32.dll')
def CreateDirectoryW(lpPathName: win32more.Windows.Win32.Foundation.PWSTR, lpSecurityAttributes: POINTER(win32more.Windows.Win32.Security.SECURITY_ATTRIBUTES)) -> win32more.Windows.Win32.Foundation.BOOL: ...
CreateDirectory = UnicodeAlias('CreateDirectoryW')
@winfunctype('KERNEL32.dll')
def CreateFileA(lpFileName: win32more.Windows.Win32.Foundation.PSTR, dwDesiredAccess: UInt32, dwShareMode: win32more.Windows.Win32.Storage.FileSystem.FILE_SHARE_MODE, lpSecurityAttributes: POINTER(win32more.Windows.Win32.Security.SECURITY_ATTRIBUTES), dwCreationDisposition: win32more.Windows.Win32.Storage.FileSystem.FILE_CREATION_DISPOSITION, dwFlagsAndAttributes: win32more.Windows.Win32.Storage.FileSystem.FILE_FLAGS_AND_ATTRIBUTES, hTemplateFile: win32more.Windows.Win32.Foundation.HANDLE) -> win32more.Windows.Win32.Foundation.HANDLE: ...
@winfunctype('KERNEL32.dll')
def CreateFileW(lpFileName: win32more.Windows.Win32.Foundation.PWSTR, dwDesiredAccess: UInt32, dwShareMode: win32more.Windows.Win32.Storage.FileSystem.FILE_SHARE_MODE, lpSecurityAttributes: POINTER(win32more.Windows.Win32.Security.SECURITY_ATTRIBUTES), dwCreationDisposition: win32more.Windows.Win32.Storage.FileSystem.FILE_CREATION_DISPOSITION, dwFlagsAndAttributes: win32more.Windows.Win32.Storage.FileSystem.FILE_FLAGS_AND_ATTRIBUTES, hTemplateFile: win32more.Windows.Win32.Foundation.HANDLE) -> win32more.Windows.Win32.Foundation.HANDLE: ...
CreateFile = UnicodeAlias('CreateFileW')
@winfunctype('KERNEL32.dll')
def DefineDosDeviceW(dwFlags: win32more.Windows.Win32.Storage.FileSystem.DEFINE_DOS_DEVICE_FLAGS, lpDeviceName: win32more.Windows.Win32.Foundation.PWSTR, lpTargetPath: win32more.Windows.Win32.Foundation.PWSTR) -> win32more.Windows.Win32.Foundation.BOOL: ...
DefineDosDevice = UnicodeAlias('DefineDosDeviceW')
@winfunctype('KERNEL32.dll')
def DeleteFileA(lpFileName: win32more.Windows.Win32.Foundation.PSTR) -> win32more.Windows.Win32.Foundation.BOOL: ...
@winfunctype('KERNEL32.dll')
def DeleteFileW(lpFileName: win32more.Windows.Win32.Foundation.PWSTR) -> win32more.Windows.Win32.Foundation.BOOL: ...
DeleteFile = UnicodeAlias('DeleteFileW')

win32more uses winfunctype as the decorator that translates those into real external functions, kind of like what is done in this PR.

But here are some caveats:

• win32more has nearly all of the API functions, and are categorized into many submodules, such as win32more.Windows.Win32.Foundation, win32more.Windows.Win32.System.Com, win32more.Windows.Win32.System.Memory, win32more.Windows.Win32.Storage.FileSystem, to name a few. You will have to look for a specific submodule before using some API function.
• There's no fancy processing for out parameters or error handling, and you are expected to use the functions like in C or C++: check the return value for error, pass pointer to variable for out parameters, etc.
• My VS Code environment uses the actual type returned by the decorators when showing the parameter hints, instead of using the decorated functions. winfunctype returns a ForeignFunction object to actually handle the external call, so the signature of ForeignFunction.__call__(self, *args, **kwargs) is used, which is just useless and defeats the purpose of creating placeholder functions for type hints.

So in order not to make VS Code "smart" enough to use the real underlying type, it might be better to check for TYPE_CHECKING in the decorator, and just return the placeholder function itself during static type checking.

[LeonarddeR]

So in order not to make VS Code "smart" enough to use the real underlying type, it might be better to check for TYPE_CHECKING in the decorator, and just return the placeholder function itself during static type checking.

This is a good idea, I was already considering that. Should at least improve performance during the type checking itself.

[LeonarddeR]

@SaschaCowley wrote:

1. What do you think the performance overhead of this is going to be? If it is going to be significant, do you think there's the opportunity for caching or other strategies to mitigate it? I don't think we need to go down that road just yet (premature optimisation and all that), but it's something to think about.

I don't think it will cost that much performance. We'll probably have to benchmark it if we want to know.

2. I noticed you used `ctypes.cdll`. I assume this strategy will work equally well for `ctypes.windll`. Is that right?

ctypes.CDLL is the base class for ctypes.WinDLL.

3. I noticed several uses of sunder-attributes of ctypes (`ctypes._*) in the code. Are those attributes intended for public use? I am concerned they could change at any moment and break this code.

Most of them are documented, see for example _SimpleCData.

I just discovered that ctypes._SimpleCData is not abstract enough, ugh. It is not easy to typehint ctypes code that easily. Thats why I came with a Protocol for FromParam.

4. Have you looked into whether there are alternatives to this that do not involve writing and maintaining a whole load of boilerplate? For instance, could we use a tool to generate `.pyi` files automatically with type information for files with ctypes bindings in the `winBindings` package, `NVDAHelper.localLib` module, and wherever else necessary?

Thanks @gexgd0419 for your answer on this. I don't have much to add.

[AppVeyorBot]

• PASS: Translation comments check.
• PASS: License check.
• PASS: Unit tests.
• FAIL: System tests (tags: installer NVDA). See test results for more information.
• Build (for testing PR): https://ci.appveyor.com/api/buildjobs/ftbf23ro6qrbeqgy/artifacts/output/nvda_snapshot_pr18534-37522,bb1e129e.exe
• CI timing (mins):
  INIT 0.0,
  INSTALL_START 2.8,
  INSTALL_END 1.2,
  BUILD_START 0.0,
  BUILD_END 30.4,
  TESTSETUP_START 0.0,
  TESTSETUP_END 0.5,
  TEST_START 0.0,
  TEST_END 22.2,
  FINISH_END 0.1

See test results for failed build of commit bb1e129e3a

[LeonarddeR]

Added tests and example in the screenCurtain module should give you an idea on how this is supposed to work.

[LeonarddeR]

Temporarily marking this as ready to trigger a copilot review

[gexgd0419]

I remembered that ctypes.POINTER, as a function, might not play well with some static type checkers. For example, VS Code shows "Unknown" for POINTER types.

See the comment where I discovered that problem: #17592 (comment)

In that case, I ended up using something like the following: (the stackoverflow answer)

if TYPE_CHECKING:
	LP_c_ubyte = _Pointer[c_ubyte]
else:
	LP_c_ubyte = POINTER(c_ubyte)

But this is not generic enough, and requires a separate alias for each pointer type. So later in #18300, I tried something like this:

if TYPE_CHECKING:
	from ctypes import _Pointer
else:
	# ctypes._Pointer is not available at run time.
	# A custom class is created to make `_Pointer[type]` still work.
	class _Pointer:
		def __class_getitem__(cls, item: type) -> type:
			return POINTER(item)

I hope there's a unified way of annotating ctypes pointer types.

[LeonarddeR]

Goot point @gexgd0419
This is one of the reasons why I support both an annotation like POINTER(c_int) | c_int and typing.Annotated[c_int, POINTER(c_int)]
In the first instance, the decorator will take the first ctypes type it fins. In the second instance, the decorator will take the second parameter to Annotated, which is metadata completely ignored by the type system.
As @michaelDCurran pointed out in https://github.com/nvaccess/nvda/pull/18207/files#r2174356632, ctypes does automatic byref on parameters that have a POINTER type in argtypes. So when the argtypes tuple contains POINTER(c_int), it is perfectly valid to throw a c_int at it.
So to put it once again:

1. POINTER(c_int) | c_int tells a type checker that you can throw both a pointer to a c_int and a c_int at it
2. `typing.Annotated[c_int, POINTER(c_int)] uses the pointer type in arg types but a type hint will consider c_int as the only valid type.

We might have to choose between either one or the other to decrease complexity.

[LeonarddeR]

@gexgd0419 wrote:

if TYPE_CHECKING:
	from ctypes import _Pointer
else:
	# ctypes._Pointer is not available at run time.
	# A custom class is created to make `_Pointer[type]` still work.
	class _Pointer:
		def __class_getitem__(cls, item: type) -> type:
			return POINTER(item)

I actually like this a lot, and since the folks at python/ctypes are pretty unwilling to fix this themselves, we might want to offer this by default in ctypesUtils. syntactically, Pointer[c_int] is way nicer anyway.

[LeonarddeR]

@SaschaCowley Wrote:

@LeonarddeR have you considered making this into its own package? It seems like it could be useful to a lot more folks than just NVDA.

I'm only considering it now you're asking it, but honestly, I'm not sure whether I have time to maintain such a project. That said, If you prefer a third party package, I'm happy to consider it further.

[LeonarddeR]

Re the discussion on pywin32more.
First of all, I'm perfectly fine with dropping this draft pr in favor of whatever else is preferred over it. That said, I continued working on it after @gexgd0419 noted some issues with it.

After investigation, the following is probably of relevance to us.

1. there is no GUID type in ctypes, so we use the comtypes GUID. However, win32more has its own GUID type.
2. It supports only input parameters. While this makes sense for wrappers that stick as close as possible to the Windows API surface, there's no way to utilize the ctypes output parameter mechanism. This pr is less opinionated, i.e. you can do whatever you like.
3. It redefines structures that are already part of ctypes.wintypes, e.g. ctypes.wintypes.rect is not equal to win32more.Windows.Win32.Foundation.RECT. That's pretty likely to hit us at some point.
4. What @gexgd0419 said in Add a helper for ctypes function annotations #18534 (comment)
5. It is really a mystery for me why, for example, they do not use something like 'functools.wraps' to tidy up their functions. May be it does not work for wrapping classes.
6. One strengt of ctypesUtils is that it not only creates a new rapper, it also annotates the original function pointer with argtypes and restype. This way, some obscure add-on or hidden code path in NVDA itself will still benefit from it even though not using the new wrapper directly.
7. Win32more does COM, but without comtypes.
8. ctypesUtils has boiler plate on creation time of the function and no boiler plate at calling time, whereas win32more does the following:
   • It creates an inner delegate object the first time a function is called and that inner delegate contains the winfunctype object. Basically most things ctypesUtils does when crreating the wrapper is performed by win32more the first time the function is called.
   • Every time the function is called, it does some magic to calculate how the arguments should be passed to the inner winfunctype, including type casting to an appropriate type if necessary. I don't see why they shouldn't just trust ctypes for this.

Point 5 should be easy to fix with a pr and will probably improve type hints, but then there's still the issue with Pointer, equivalent but not equal types (RECT and GUID), not allowing pythonic data types in hints (what can be done in ctypesUtils with int | c_int.

As for the automatic generation of functions, may be copilot agent mode can play nicely here when connected to the MPC for Microsoft Learn. It ensures that AI has all the function information at hand when writing. I haven't played with this yet.

[michaelDCurran]

To get win32more to allow VS Code to correctly handle type info for the functions, I think win32more's winfunctype needs to be typed itself to to understand it returns a Callable with the same signature as what it received:
In win32more\__init__.py:

from typing import Callable, ParamSpec, TypeVar, cast

_funcParams = ParamSpec("_funcParams")
_funcReturn = TypeVar("_funcReturn")
def winfunctype(library, entry_point=None) -> Callable[[Callable[_funcParams, _funcReturn]], Callable[_funcParams, _funcReturn]]:
    def decorator(prototype: Callable[_funcParams, _funcReturn]) -> Callable[_funcParams, _funcReturn]:
        return cast(Callable[_funcParams, _funcReturn], ForeignFunction(prototype, library, entry_point, False, windll, WINFUNCTYPE))

    return decorator

My VS Code now can see the correct param and return types for MessageBoxW for example.

It would be good if we could do this in a type stub file, but I can't seem to get that to work.

[michaelDCurran]

I edited my above comment to be more accurate:

from typing import Callable, ParamSpec, TypeVar, cast

_funcParams = ParamSpec("_funcParams")
_funcReturn = TypeVar("_funcReturn")
def winfunctype(library, entry_point=None) -> Callable[[Callable[_funcParams, _funcReturn]], Callable[_funcParams, _funcReturn]]:
    def decorator(prototype: Callable[_funcParams, _funcReturn]) -> Callable[_funcParams, _funcReturn]:
        return cast(Callable[_funcParams, _funcReturn], ForeignFunction(prototype, library, entry_point, False, windll, WINFUNCTYPE))

    return decorator
    ```

[gexgd0419]

6. One strengt of ctypesUtils is that it not only creates a new rapper, it also annotates the original function pointer with argtypes and restype.

If we use out parameters, will the out parameters be annotated on the original function pointer as well?

I think that the code usually assumes there's no out parameter if argtypes is not set, so setting out parameters might break the code. Would it be better to annotate all parameters as "input" on the original functions, or provide an option for it?

[michaelDCurran]

I personally don't really like the automatic handling of out parameters here. I think that becomes a little too pythonic. I think if you want that, then write specific friendly wrappers at a higher level. At the lowest level, the goals should really just be data size / bitness compliance (E.g. 32 and 64 bit will work fine) and type checking will be accurate. But no shaddowing returns with outs or making signatures more friendly. I'd prefer it to match Microsoft docs to the letter. But just my feeling.
Also, I'm not sure I like the idea that the underlying funcpointer has its argtypes and restype changed. I'd prefer that these are entirely isolated funcpointers. Otherwise, it becomes harder to track down bugs and understand unrelated code. E.g. code might break depending on the ordering of module imports. This has already happened to me with our winBindings modules.

[SaschaCowley]

I think if you want that, then write specific friendly wrappers at a higher level. At the lowest level, the goals should really just be data size / bitness compliance (E.g. 32 and 64 bit will work fine) and type checking will be accurate. But no shaddowing returns with outs or making signatures more friendly.

I'm in agreement with Mick here. We want these to be fairly direct access to the Windows API. Pythonic wrappers should be handled separately.

[gexgd0419]

This PR can be used as a low level wrapper, and has the option to create a high level wrapper, which is just utilizing the features of ctypes anyway.

If raw, direct access is preferred, we can just use this without any out parameters, and still have the option to use this to create higher level wrappers where needed.

I think that error checking is a good addition though. In C/C++, we usually have to check the return value of API calls with ifs, but it's not so Pythonic, so converting them to exceptions might be good to avoid writing a bunch of ifs in Python code, or forgetting to check the return value and letting errors slip away. comtypes is already converting HRESULT errors to exceptions, so I think that this is acceptable.

Unless most of the developers are still accustomed to checking the return values manually according to Microsoft docs, in which case this can be confusing.

[LeonarddeR]

@michaelDCurran wrote:

I edited my above comment to be more accurate:

Have you played with functools.wraps? something with functools.wraps(prototype)(ForeignFunction(**)) should do all the types magic automatically, according to the docs.

That said, as pointed out by @gexgd0419, win32more is still very strict about the types it communicates to the type checker. only c_int, not int, only c_wchar_p, not str.

@gexgd0419 wrote:

If we use out parameters, will the out parameters be annotated on the original function pointer as well?

No. The original function pointer is already constructed, and you can only provide the parameter spec at construction time of the function pointer. So even if you provide out parameters on the function prototype and you specify annotateOriginalCFunc=True, the original pointer will treat every parameter as input parameter. It will just set restype and argtypes on it

This PR can be used as a low level wrapper, and has the option to create a high level wrapper, which is just utilizing the features of ctypes anyway.

If raw, direct access is preferred, we can just use this without any out parameters, and still have the option to use this to create higher level wrappers where needed.

This is exactly how I intended it to be: offering the broadest possible flexibility. If you want the wrappers to be c like, just don't provide out parameters and you're fine.

@michaelDCurran wrote:

But no shaddowing returns with outs or making signatures more friendly. I'd prefer it to match Microsoft docs to the letter. But just my feeling.

Note that ctypes does shadowing returns al over the place. Specify a restype of c_int and ctypes will return an python int, I believe it will even do so with c_void_p. The example in #18534 (comment) GetClientRect is hinted to return a BOOL but in fact returns an int. If a function returns a c_wchar_p, ctypes will just unpack the c_wchar_p and discard the pointer, so you can't strictly forward it to a follow up function call.

Also, I'm not sure I like the idea that the underlying funcpointer has its argtypes and restype changed. I'd prefer that these are entirely isolated funcpointers.

Note that changing restype and argtypes is exactly what winbindings is doing in the current pull requests that show it, #18571 and #18207.
That said, It makes sense to make this behavior opt in, not opt out, so I changed the defaults accordingly.

[gexgd0419]

What's the recommended way to pass a pointer to a structure (or anything else)?

@dllFunc(ctypes.windll.user32)
def GetDesktopWindow() -> Annotated[int, HWND]: ...

@dllFunc(ctypes.windll.user32)
def GetWindowRect(hWnd: int | HWND, lpRect: Pointer[RECT]) -> Annotated[int, BOOL]: ...

hwnd = GetDesktopWindow()
rc = RECT()
GetWindowRect(hwnd, rc)  # type checker error
GetWindowRect(hwnd, ctypes.pointer(rc))  # pass
GetWindowRect(hwnd, ctypes.byref(rc))  # type checker error

Here only GetWindowRect(hwnd, ctypes.pointer(rc)) is allowed by a strict type checker. ctypes.byref returns ctypes._CArgObject, which is for type checking only, and will cause an error if used at runtime.

We might need to have some kind of coding standard for this.

[LeonarddeR]

What's the recommended way to pass a pointer to a structure (or anything else)?

As input parameter: intptr: ctypesUtils.Pointer[c_int] | c_int

This tells the decorator to use the first type in argtypes. it tells the type checker that passing a c_int is also valid, since it is valid ctypes behavior to automatically byref c_int.

@dllFunc(ctypes.windll.user32)
def GetWindowRect(hWnd: int | HWND, lpRect: Pointer[RECT]) -> Annotated[int, BOOL]: ...

rc = RECT()
GetWindowRect(hwnd, rc)  # type checker error

Should be solved by lpRect: Pointer[RECT] | RECT here. The first type is in argtypes, the second type is also valid to ctypes.

GetWindowRect(hwnd, ctypes.byref(rc))  # type checker error

ctypes.byref returns ctypes._CArgObject, which is for type checking only, and will cause an error if used at runtime.

Good catch. I think I can fix that. How do you check those at runtime?

[gexgd0419]

Good catch. I think I can fix that. How do you check those at runtime?

You can use something like type(ctypes.byref(ctypes.c_int())) to get the type.

Printing the type shows <class 'CArgObject'>.

However, there's neither ctypes.CArgObject nor ctypes._CArgObject at runtime. It's internal.

So the only way I can think of, is to use something like type(ctypes.byref(ctypes.c_int())) to fetch the type, and assign it to an alias, kind of like what I did for pointer type aliases.

if TYPE_CHECKING:
    byrefType = ctypes._CArgObject  # type: ignore
else:
    byrefType = type(ctypes.byref(ctypes.c_int()))

[LeonarddeR]

@gexgd0419 This should work now. I hope that static type checking with byref is also fine now.
I'm not getting type errors at all from my vs code. I think it's because NVDA bundles pyright config and that's not very strict.

[gexgd0419]

Currently many of the Pyright rules are disabled, see #17744. The goal is to improve the existing code and turn on rules over time, but as for now, even simple type errors such as a: int = "abc" will not be reported.

If you want to check how well it will work with type checkers in the future, you can try removing the "Bad rules" lines in the pyproject.toml file, so that it will check everything.

[gexgd0419]

I tried the above to let pyright do its work, and it started complaining.

For example, this line:

@dllFunc(windll.user32, restype=BOOL, annotateOriginalCFunc=True)
def GetClientRect(hWnd: Annotated[int, HWND]) -> Annotated[RECT, OutParam("lpRect", 1)]: ...

Pylance warns that BOOL cannot be assigned to type[CType] | empty | None.

I think that it failed to associate CType with ctypes types, because it's static and probably doesn't know how CType.register work.

So how does the original restype in ctypes work?

# in _ctypes.pyi
@type_check_only
class _CData:
    ...

class _SimpleCData(_CData, Generic[_T], metaclass=_PyCSimpleType):
    ...

_CDataType: TypeAlias = _SimpleCData[Any] | _Pointer[Any] | CFuncPtr | Union | Structure | Array[Any]

class CFuncPtr(_PointerLike, _CData, metaclass=_PyCFuncPtrType):
    restype: type[_CDataType] | Callable[[int], Any] | None
    ...

I guess that if we want to make the type checker happy, we might have to use those private, type-check-only types in .pyi files, and only during type checking.

I wonder, do we still need run-time type information in this case? Or can we just deal with the static type checkers, like using a .pyi file?

[LeonarddeR]

@gexgd0419 I changed code to use these internals. Runtime type checking is less important indeed, but with using an ABC like done now, it should also work.
That said, it is very hard to get the generics right. Some pointer like types take a generic parameter, some others like _CArgObject do not.

[LeonarddeR]

In 52f291b, I added some examples To make it more tangible how CtypesUtils works at a lower level.