proposal: x/sys/windows: let `HRESULT` implement the `error` interface

[hajimehoshi]

Now error values from APIs in the x/sys/windows package are syscall.Errno, even though the original API returns HRESULT value. For example, CoInitializeEx originally returns HRESULT, but windows.CoInitializeEx wraps the HRESULT value as syscall.Errno and returns it. This package doesn't distinguish Errno and HRESULT as error values, and this seems odd in terms of semantics. My understanding is Errno and HRESULT are different things. There might be a historical reason why this package does so, but in this case I'd like to know.

My proposal is to let HRESULT implement the error interface and let the APIs return it (when the value is not S_OK) instead of wrapping as Errno. This simple implementation would be enough:

func (h HRESULT) Error() string {
    return fmt.Sprintf("HRESULT(%08x)", h)
}

This is a breaking change and existing error checkings might no longer work, after the APIs return HRESULT instead of Errno. However, as x/sys/windows is still v0, a breaking change should be acceptable.

The controversial point is that there are some non-zero values which can be used in a successful case (e.g. S_FALSE for CoInitializeEx). In this case, implementing error might be odd. IMO my proposal is not a perfect solution but should be better than wrapping HRESULT as Errno without conditions, and improve the current semantics confusion.

[ianlancetaylor]

CC @golang/windows

[apparentlymart]

There seems to be some general disagreement in this package about how it represents the various conventions for representing errors in different parts of the Windows API. 🤔

The script which generates all of the constants representing different errors seems to generate syscall.Errno constants for the System Error Codes, like ERROR_FILE_NOT_FOUND (syscall.Errno(2)).

That script uses Handle to represent anything that is typed as HRESULT in the Windows platform headers, such as E_ABORT (Handle(0x80004004)). That seems to disagree with the decision made in the wrapping stubs for functions that return HRESULT, such as CoInitializeEx returning syscall.Errno as discussed in the initial issue writeup. Presumably this means that the results of these functions will not compare using == with the associated constants, because the types won't match. 😬

I don't see the "errno constants" in there at all, but I think that's because they are part of the C runtime library rather than part of the platform headers, and so they aren't visible to or relevant to x/sys/windows (which isn't exposing the C runtime API at all) although apparently (per #23468) CGo on Windows is using syscall.Errno for these ones too.

x/sys/windows does use syscall.Errno to represent the Winsock equivalents of those error states, such as WSAENAMETOOLONG (syscall.Errno(10063)).

It seems like there was some discussion about the types used by this generator script in https://go-review.googlesource.com/c/sys/+/170918 .

I'd guess most or all of these inconsistencies are unfixable without breaking existing callers. 😖