-
Notifications
You must be signed in to change notification settings - Fork 18.8k
Description
Let's say I wanted to add this API to x/sys/windows: https://learn.microsoft.com/en-us/windows/win32/api/socketapi/nf-socketapi-setsocketmediastreamingmode. My initial thought is to add something like this //sys line:
//sys SetSocketMediaStreamingMode(value bool) (err error) = windows.networking.SetSocketMediaStreamingModeHowever, go generate ./windows/ then fails:
Could not extract dll name from "SetSocketMediaStreamingMode(value bool) (err error) = windows.networking.SetSocketMediaStreamingMode"
This is because mkwinsyscall doesn't handle more than one ., and windows.networking.SetSocketMediaStreamingMode has two: https://github.com/golang/sys/blob/6e4d1c53cf3b2c1c6d104466749f76cc7a5c9ed8/windows/mkwinsyscall/mkwinsyscall.go#L483-L491
- doesn't break in quite the same way, but the fix is related. For example, api-ms-win-wsl-api-l1-1-0.WslRegisterDistribution isn't rejected by mkwinsyscall itself, but the generated code isn't valid because an identifier can't contain a -.
Changes
- It's easy to change the
defaultcase to support this by assuming any extra.segments are part of the DLL name. This seems like a valid assumption because the win32 methods won't have.in them. - After making that change, there are some changes needed elsewhere to escape
.somkwinsyscallgenerates buildable Go code.- To make
-work, too, it can also be escaped.
- To make
- A simple escaping rule is to replace
.and-with_.- In theory, escaped DLL names could collide, but I'm not aware of any instances where this would happen.
Effect on //sys lines being (almost entirely) readable Go code
//sys CryptAcquireContext(...) (err error) = advapi32.CryptAcquireContextW
In that line, = advapi32.CryptAcquireContextW lets us think of this as aliasing a func in a advapi32 Go package.
The proposal breaks the metaphor: = windows.networking.SetSocketMediaStreamingMode is not valid Go. Is this ok, or does mkwinsyscall need to strictly enforce the metaphor?
An alternative would be extending the metaphor by importing a "package" with a rename, but this would be more complicated to implement properly:
//sysimport windowsnetworking "windows.networking"
//sys SetSocketMediaStreamingMode(...) (...) = windowsnetworking.SetSocketMediaStreamingMode
Background
I'm trying to use mkwinsyscall for x/sys/windows: use win32metadata? #43838 by having a generator create //sys comments. When I generated //sys lines for all the API methods for Windows.Win32.winmd, I hit these . and - cases.
I don't know if it makes sense for #43838 to end up using mkwinsyscall in this way, but I figure until that proposal gets traction one way or another, it's worth asking about adding this kind of support.
- I also filed x/sys/windows: mkwinsyscall isn't diagnosable if
format.Sourcefails #57925 for an improvement to diagnosability I made while looking into this issue.