Many Win32-style enums would be better promoted to module-scope constants

[damyanp]

Compare the usability of WM_QUIT with D3D12_RESOURCE_BARRIER_TYPE_TRANSITION:

let message = WM_QUIT;
let barrier_type = D3D12_RESOURCE_BARRIER_TYPE::D3D12_RESOURCE_BARRIER_TYPE_TRANSITION;

The barrier type one is much more awkward to use. IMO it would be much more convenient to be able to write:

let barrier_type = D3D12_RESOURCE_BARRIER_TYPE_TRANSITION;

As all the naming conventions were picked with the assumption that no further qualification would be required.

Alternatively, the projection could try and figure out better names:

let barrier_type = D3D12_RESOURCE_BARRIER_TYPE::TRANSITION;

[kennykerr]

Yes, I think that's a good idea. Enums from WinRT APIs tend to already provide scoped names, but Win32 enums are based on C-style enums, as you know. I could make them module-scoped but still preserve their type.

[kennykerr]

Blocked on microsoft/win32metadata#442 and microsoft/win32metadata#443

[MarijnS95]

If D3D12_RESOURCE_BARRIER_TYPE exists as a type used in structs already, I'd prefer simplified variants:

let barrier_type = D3D12_RESOURCE_BARRIER_TYPE::TRANSITION;

That then allows use of Self::TRANSITION if (extra) functions were implemented on D3D12_RESOURCE_BARRIER_TYPE.

Module-scope constants could simply reference those variants I guess?

[damyanp]

I was going to propose something like this originally, but there's a couple of drawbacks that it'd be interesting to get your take on. Firstly, it diverges from what these things are called in other tools or languages. For example, being able to search for D3D12_RESOURCE_BARRIER_TYPE_TRANSITION and get hits in C++ and Rust is pretty valuable. Tools such as PIX or the debug layer display the Win32 ABI names for these types, so as it diverges we'll be asking developers to make the translation in their head.

The second issue, which is more of an implementation detail, is that there isn't always a mechanical way to reliably generate the simplified names. This can be addressed with enough determination :)

[kennykerr]

Yes, searchability and portability is very valuable. Having already tried it, there are definitely a lot of edge cases that don't work well when you try to shorten the names. The metadata also doesn't deal with these constants consistently. Some are enums, some are constants, so in the end I'm also looking for more consistency in how this is presented in Rust. Scoping these also means that all of them must be generated as bindings. This is very problematic if you refer to an API that say uses WIN32_ERROR and suddenly your generated windows.rs file has an extra few thousand lines of code.

[MarijnS95]

For example, being able to search for D3D12_RESOURCE_BARRIER_TYPE_TRANSITION and get hits in C++ and Rust is pretty valuable.

At least in Rust there's #[doc(alias = "D3D12_RESOURCE_BARRIER_TYPE")] for exactly this purpose. That makes both the source and documentation searchable for the full name.
(I've been told recently that alias only deals with exact matches when searching the documentation, though!)

The second issue, which is more of an implementation detail, is that there isn't always a mechanical way to reliably generate the simplified names. This can be addressed with enough determination :)

We're constantly fighting with that in the Vulkan bindings as well. Theoretically this should be solved on a metadata level, ie. enum variants/constants are not replicating the name of the surrounding type. Then the projection is free to make it an associated variant (concatenated with ::), a global constant (concatenated with _), or both.

Some are enums, some are constants, so in the end I'm also looking for more consistency in how this is presented in Rust.

That would be tricky. Logic has it that enums should definitely follow the <type>_<variant> pattern and are susceptible to shortening, but then I guess certain groups of constants follow the same pattern too and should likely be shortened based on the same heuristic?

Scoping these also means that all of them must be generated as bindings.

I'm not sure about that. Consider the following in build!, here's what we could do:

• D3D12_RESOURCE_BARRIER_TYPE_TRANSITION: generates the D3D12_RESOURCE_BARRIER_TYPE and TRANSITION variant (both as variant and associated constant, pending discussion above);
• D3D12_RESOURCE_BARRIER_TYPE::*: generate the D3D12_RESOURCE_BARRIER_TYPE type and all variants (nice short-hand that we don't have currently);
• D3D12_RESOURCE_BARRIER_TYPE: alternatively:
  • Generate all variants/constants, or;
  • Generate just the type (for use in structs), without any variants/constants.

OTOH having an enum with incomplete variants might feel inconsistent and confusing.

This is very problematic if you refer to an API that say uses WIN32_ERROR and suddenly your generated windows.rs file has an extra few thousand lines of code.

This is already the case in bindings.rs, no? Not necessarily a bad thing IMO, in fact I'd appreciate even more code like an impl Display instead of having to search for the number in source or Windows docs (unless there's a Win32 function to do the conversion, iirc there is for some form of error codes at least).

Perhaps on that note it's a good idea to split up generated code across files. Referencing Ash again, we have a separate file dedicated to displaying enums/bitflags.

---

All in all I'm just grossed out at reading D3D12_RESOURCE_BARRIER_TYPE::D3D12_RESOURCE_BARRIER_TYPE_TRANSITION. DRY.

[kennykerr]

Yes, renaming stuff is hard and I ultimately went with preserving (#646) the original names everywhere (the win32 metadata project has taken the same originalist approach). The challenge is to make the bindings as rich/useful as possible while letting the developer limit/filter as much as possible to keep build time/complexity under control. That's why I'd like fine-grained filtering and not force huge chunks of code to be generated. The flip side is having pre-generated bindings that include everything (#432) but we're a ways from getting that to work.

[MarijnS95]

@kennykerr That's only casing, not stripping/deduplicating enum variant names, right? I agree on keeping the names similar, using original casing wherever possible. Tools like rust-analyzer do a good job helping one find symbols, though.

Unless I misunderstood you can still have fine-grained control over generated code if enum variants have the redundant part of the name stripped off? Again, I'm okay with module-level D3D12_RESOURCE_BARRIER_TYPE_TRANSITION or associated enum variant D3D12_RESOURCE_BARRIER_TYPE::TRANSITION (no need to strip off D3D12 even if implied by the module, I guess?), but D3D12_RESOURCE_BARRIER_TYPE::D3D12_RESOURCE_BARRIER_TYPE_TRANSITION is too verbose for me.

[kennykerr]

@damyanp All done - let me know what you think. 😉

[damyanp]

Thanks! That makes the code so much nicer to read!

It'd be nice if generating bindings for an enum also generated the bindings for all of the enum fields.

For example, if I build "WNDCLASS_STYLES", I'd want to get "CS_HREDRAW" without having to list them manually.

[kennykerr]

Yes, I considered that. The drawback is that (1) there's reduced symmetry with the Rust language between the build macro and the use declaration and (2) more importantly it dramatically increases the amount of generated code in some cases where the enums have many many members. I think we can temper this with smaller namespaces (coming from win32metadata) and being able to use a ::* wildcard if you're just getting started and don't yet know which constants you'll need.

[MarijnS95]

@kennykerr Would it be possible for the generator to distinguish between WNDCLASS_STYLES as just the type, and WNDCLASS_STYLES::* as the type and all associated enum variants? Consequently the bindings generator could emit aliases in an impl WNDCLASS_STYLES block to the module-scope constants - and where possible stripped of their common name prefix as mentioned above?

However that probably goes straight into what you want to achieve here with keeping it simple and concise, for little gain.

[MarijnS95]

@kennykerr What qualifies as "scoped enum"? Just updated a local project to this and it seems EXCEPTION_DISPOSITION would be a nice fit for ScopedEnum since its variants don't have a prefix equivalent to the enum type?

// Windows.Win32.System.Kernel.EXCEPTION_DISPOSITION
public enum EXCEPTION_DISPOSITION
{
	ExceptionContinueExecution,
	ExceptionContinueSearch,
	ExceptionNestedException,
	ExceptionCollidedUnwind
}

[kennykerr]

@MarijnS95 The ScopedEnum attribute is required. microsoft/win32metadata#443

[MarijnS95]

@kennykerr Sure, just asking whether EXCEPTION_DISPOSITION would qualify for that attribute in the first place?

[kennykerr]

It's somewhat subjective. The constant names are unlikely to collide with anything so I'm hesitant to recommend it since that would make their use more verbose. I generally recommend ScopedEnum for those types that are defined with C++'s enum class rather than C's enum.

[MarijnS95]

@kennykerr Supposedly this enum lives in a typedef enum - but the CamelCase instead of SHOUTY_SNAKE_CASE name seems to differentiate it from module-level constants. However, it's probably not worth the hassle.

Any comment on associated constants in #727 (comment) though?

[kennykerr]

Yes, its an odd case hence subjective. Feel free to raise it as an issue on the win32metadata repo. I don't really mind either way, but I'm not in favor of removing prefixes as it deviates too much from existing docs and samples in other languages.

Do you mean use WNDCLASS_STYLES::* as a shorthand for all of the constants? A concern is that it deviates from the Rust use declaration syntax which may be confusing if developers want to copy it over.

[MarijnS95]

I don't really mind either way, but I'm not in favor of removing prefixes as it deviates too much from existing docs and samples in other languages.

I was thinking about having it both ways: a module-level constant, and a prefix-stripped variant aliasing that module-level constant. It's extra lines in windows.rs though, and useless if no-one ends up using it. I would use it as I find the TYPE::Variant hierarchy nicer to read than TYPE_VARIANT despite making it harder for others to search around in my codebase.

Do you mean use WNDCLASS_STYLES::* as a shorthand for all of the constants? A concern is that it deviates from the Rust use declaration syntax which may be confusing if developers want to copy it over.

Yeah that wouldn't work in Rust - better not do that then especially if we're not generating associated constants as aliases anyway.

[roblabla]

One downside of this is that when generating Windows::Win32::System::Diagnostics::Debug::WIN32_ERROR, none of the associated constants are generated anymore. The only way to get all of them is to generate Windows::Win32::System::Diagnostics::Debug::*, which is a bit more than necessary. There are many other similar examples (like Windows::Win32::System::WindowsProgramming::VER_FLAGS).

I think when generating a scoped enum, all its module-scope constants should be generated as well.

[kennykerr]

@roblabla yes that's a tradeoff I'm still wrestling with.

@mikebattista are there plans to reduce the size of the Windows::Win32::System::Diagnostics::Debug namespace or move the common WIN32_ERROR type to a less cluttered namespace?

I think when generating a scoped enum, all its module-scope constants should be generated as well.

The trouble is that it is often the case that the particular error code isn't needed where the value is simply checked for success/failure and passed on to some Result object for propagation. Such large enums can really clutter up the the generated bindings. Generating all of the WIN32_ERROR constants amounts to over 3,000 extra lines of code.

[mikebattista]

@kennykerr we could move WIN32_ERROR and GetLastError to the Foundation namespace. Would that work?

[roblabla]

Hmm. While this is true for WIN32_ERROR, there are many other enums that are rather small and currently live in very cluttered namespaces too (VER_FLAGS above). Those would have minimal impact in being always generated when the type is present.

Another avenue that could fix it would be giving the ability to have a prefix on wildcard, e.g. build!( Windows::Win32::System::WindowsProgramming::VER_*). It's a bit of a bandaid though.

[mikebattista]

In the latest metadata, VER_FLAGS is currently in a much more scoped Windows.Win32.System.SystemInformation namespace.

The SystemServices and WindowsProgramming namespaces won't exist anymore once we're done refactoring.

[kennykerr]

Another option is to treat the scoped enums as follows: if the build macro only includes specific constants like ERROR_PATH_NOT_FOUND then only the listed constants are defined, but if the build macro includes the enum type itself like WIN32_ERROR then all of its enclosing constants are included. How does that sound? I've been trying to keep as close to Rust syntax and semantics as possible but this sounds like it could be useful.

[kennykerr]

Thanks @mikebattista, I'm not sure whether moving it would help. I am curious what the difference is between Windows::Win32::Foundation and Windows::Win32::System but that's a different topic. 😊

[mikebattista]

There's nothing in Windows.Win32.System so I'm not sure what you're referring to there.

[kennykerr]

I mean how do you decide whether something is in Foundation vs System::* or somewhere else? With WinRT, Windows.Foundation is for things that are core to WinRT (e.g. collections and async support) without which other APIs could not be defined.

[mikebattista]

There is a need to put common shared types like HANDLE, HWND, string types, etc. in a shared namespace. Foundation is currently the plan for that to align with Windows.Foundation. The expectation would be that Foundation would always be included like Windows.Foundation for WinRT or System for .NET. Given that WIN32_ERROR and GetLastError are used with many APIs, it's a possible candidate for that Foundation namespace. The goal is to improve discovery of the basic things you need and minimize needing to add seemingly obscure namespace references just to use APIs in some other completely unrelated namespace.

System.* is for system-level APIs that are mostly self-contained and not as commonly required as Foundation APIs would be: System.Memory, System.Power, System.Threading, etc. If you're using System.Memory you're not necessarily also using System.Power. If you think APIs like CreateFileMapping and VirtualAlloc should be in Foundation, that would be a separate discussion.

[kennykerr]

Ah yes, I remember us chatting about a common namespace that others could depend on to keep circular references to a minimum.

[kennykerr]

So yes, I'd err on the side of keeping it to a minimum and GetLastError doesn't seem like it needs to be there to reduce dependencies.

[mikebattista]

Thanks. It's a work in progress so happy to iterate based on feedback on where APIs belong.

For sure, though, I want to get rid of SystemServices and WindowsProgramming.