`EnumWindows` is incorrectly translated to `Result<()>`

[sylveon]

Summary

Currently, EnumWindows is translated as

#[inline]
pub unsafe fn EnumWindows(
    lpenumfunc: WNDENUMPROC,
    lparam: super::super::Foundation::LPARAM,
) -> windows_core::Result<()> {
    windows_core::link!("user32.dll" "system" fn EnumWindows(lpenumfunc : WNDENUMPROC, lparam : super::super::Foundation:: LPARAM) -> windows_core::BOOL);
    unsafe { EnumWindows(lpenumfunc, lparam).ok() }
}

However, this is incorrect. The docs state

If EnumWindowsProc returns zero, the return value is also zero. In this case, the callback function should call SetLastError to obtain a meaningful error code to be returned to the caller of EnumWindows.

Which means that if lpenumfunc returns 0, code will see Err(<whatever was in SetLastError>) instead of Ok(()). Ideally, EnumWindows should either return Result<BOOL> and do the dance prescribed by docs, or return BOOL and let the user deal with it.

Crate manifest

[package]
name = "ttb_bindings"
edition = "2024"

[dependencies]
windows-core = "0.62.2"

[build-dependencies]
windows-bindgen = "0.66.0"

Crate code

use std::{env, path::Path};
use windows_bindgen;

fn main() {
    let out_dir = env::var_os("OUT_DIR").unwrap();
    let dest_path = Path::new(&out_dir).join("bindings.rs");

    windows_bindgen::bindgen([
        "--out",
        dest_path.to_string_lossy().as_ref(),
        "--no-allow",
        "--filter",
        "Windows.Win32.Foundation",
        "Windows.Win32.System.Com",
        "Windows.Win32.System.Console",
        "Windows.Win32.System.Diagnostics.Debug",
        "Windows.Win32.System.LibraryLoader",
        "Windows.Win32.System.ProcessStatus",
        "Windows.Win32.System.Threading",
        "Windows.Win32.UI.Accessibility",
        "Windows.Win32.UI.WindowsAndMessaging",
        "Windows.Win32.UI.Xaml.Diagnostics",
        // These drags in a bunch of HTML/XML stuff
        "!Windows.Win32.System.Com.Urlmon",
        "!Windows.Win32.System.Diagnostics.Debug.Extensions",
        "!Windows.Win32.System.Diagnostics.Debug.WebApp",
        // Drags in OLE
        "!Windows.Win32.System.Diagnostics.Debug.I*",
        "!Windows.Win32.System.Diagnostics.Debug.ActiveScript",
    ])
    .unwrap();

    println!("cargo::rerun-if-changed=build.rs");
}

#![allow(non_snake_case, non_upper_case_globals, non_camel_case_types)]
include!(concat!(env!("OUT_DIR"), "/bindings.rs"));

pub use self::Windows::*;
pub use windows_core as core;

[kennykerr]

Good catch - I would consider this a metadata bug as there's nothing in the metadata that describes the behavior that the windows-bindgen crate could use to generate this correctly. The metadata should presumably omit the PInvokeAttributes::SupportsLastError flag as the API does not honor this unconditionally.

[sylveon]

Yeah, I think removing SupportsLastError and letting the user deal with it is probably the way to go here, like [Get/Set]WindowLongPtr