Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Bindgen cannot find nested enum Version definition #3468

Closed
MarijnS95 opened this issue Jan 31, 2025 · 15 comments · Fixed by #3471
Closed

Bindgen cannot find nested enum Version definition #3468

MarijnS95 opened this issue Jan 31, 2025 · 15 comments · Fixed by #3471
Labels
question Further information is requested

Comments

@MarijnS95
Copy link
Contributor

MarijnS95 commented Jan 31, 2025

Suggestion

Hi @kennykerr, are you planning on upgrading win32metadata any time soon? The new Agility SDK is out and I'd like to regenerate windows-rs with it (even just for local testing) but hit the following snag:

thread 'main' panicked at crates\libs\bindgen\src\tables\field.rs:29:18:
type not found: .Version
stack backtrace:
   0: std::panicking::begin_panic_handler
             at /rustc/9fc6b43126469e3858e2fe86cafb4f0fd5068869\library/std\src\panicking.rs:665
   1: core::panicking::panic_fmt
             at /rustc/9fc6b43126469e3858e2fe86cafb4f0fd5068869\library/core\src\panicking.rs:76
   2: windows_bindgen::winmd::reader::Reader::unwrap_full_name
             at .\crates\libs\bindgen\src\winmd\reader.rs:178
   3: enum2$<windows_bindgen::types::Type>::from_ref
             at .\crates\libs\bindgen\src\types\mod.rs:230
   4: enum2$<windows_bindgen::types::Type>::from_blob_impl
             at .\crates\libs\bindgen\src\types\mod.rs:285
   5: enum2$<windows_bindgen::types::Type>::from_blob
             at .\crates\libs\bindgen\src\types\mod.rs:258
   6: windows_bindgen::tables::Field::ty
             at .\crates\libs\bindgen\src\tables\field.rs:29
   7: windows_bindgen::types::cpp_const::impl$3::combine
             at .\crates\libs\bindgen\src\types\cpp_const.rs:142
   8: windows_bindgen::types::impl$3::combine
             at .\crates\libs\bindgen\src\types\mod.rs:984
   9: windows_bindgen::type_map::TypeMap::filter
             at .\crates\libs\bindgen\src\type_map.rs:40
  10: windows_bindgen::bindgen<array$<ref$<str$>,2>,ref$<str$> >
             at .\crates\libs\bindgen\src\lib.rs:171
  11: tool_bindings::main
             at .\crates\tools\bindings\src\main.rs:14
  12: core::ops::function::FnOnce::call_once<void (*)(),tuple$<> >
             at C:\Users\Marijn\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\ops\function.rs:250
  13: core::hint::black_box
             at C:\Users\Marijn\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\hint.rs:389
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
error: process didn't exit successfully: `target\debug\tool_bindings.exe` (exit code: 101)

Notice that the error doesn't include a namespace. I boiled this down to a nested enum Version definition inside a new struct GdiplusStartupInputEx:

https://github.com/microsoft/win32metadata/blob/de9da38a61daad32fbe16c470cb2b9afc3ce5aec/generation/WinSDK/RecompiledIdlHeaders/um/gdiplusinit.h#L67-L105

Is that something you think you can solve? It seems the reference to the Version type in the GdiplusStartupInputEx() constructor function lacks a namespace because it was defined relative to the current struct, but bindgen then doesn't know how to find that.

I've been getting around this by skipping the namespace with !Windows.Win32.Graphics.GdiPlus. Unfortunately skipping just this struct with !Windows.Win32.Graphics.GdiPlus.GdiplusStartupInputEx doesn't help.

@MarijnS95 MarijnS95 added the enhancement New feature or request label Jan 31, 2025
@kennykerr
Copy link
Collaborator

are you planning on upgrading win32metadata any time soon

According to this we're on the latest versions:

https://github.com/microsoft/windows-rs/tree/master/crates/libs/bindgen/default

I'll take a look at the latest github build and see if there's an issue with the parser.

@kennykerr
Copy link
Collaborator

Hmm I can't find any builds for commits/PRs on win32metadata... 🤔

@MarijnS95
Copy link
Contributor Author

Right, we'd need a metadata update first to put this in motion... I generated them myself.

I could rebase my metadata changes on top of the latest release, but that makes it harder to also contribute them back. Guess I'll have to wait for a win32metadata merge + release, the workaround is good enough for me.

@kennykerr
Copy link
Collaborator

Can you share the winmd file that you generated? I can quickly spot check that at least.

@kennykerr kennykerr added question Further information is requested and removed enhancement New feature or request labels Jan 31, 2025
@MarijnS95
Copy link
Contributor Author

@kennykerr yeah, I pushed it to MarijnS95@7c17315, you should be able to download it from there.

Notice the changes to sys.txt and windows.txt to support this.

@kennykerr
Copy link
Collaborator

Thanks!

This is what that type looks like in metadata:

public struct GdiplusStartupInputEx
{
	[Documentation("https://learn.microsoft.com/windows/win32/TaskSchd/taskschedulerschema-version-registrationinfotype-element")]
	public enum Version : uint
	{
		V2 = 2u,
		V3
	}

	public GdiplusStartupInput Base;

	public int StartupParameters;
}

Let's ignore the spurious "Documentation" attribute... The problem here is that that windows-rs only supports nested structs whereas this is a nested enum. I may just omit such nested types as they seem quite rare and provide little value compared to the complexity of trying to find a way to model this in Rust, which lacks support for nested types altogether.

@MarijnS95
Copy link
Contributor Author

@kennykerr ah yeah, good point. I was only going as far as the metadata parser not even being able to find the type used in the constructor method... But even if it did it wouldn't be a valid path in generated code. Maybe concatenate the two names, or find a way to skip that constructor entirely for now.

Thanks for looking into this!

@kennykerr
Copy link
Collaborator

There's also something fishy about the metadata. The Version type shows up in the NestedClass table here:

Image

And then it also shows up in the TypeDef table here:

Image

The problem is that nested TypeDef rows should have an empty Namespace column. So this is highly unusual.

@MarijnS95
Copy link
Contributor Author

Just in case, it was generated with the most recent win32metadata codebase on microsoft/win32metadata#2055.

@kennykerr
Copy link
Collaborator

Here's the fix: #3471

@riverar
Copy link
Collaborator

riverar commented Feb 3, 2025

@MarijnS95 Heads up, win32metadata main is not currently in a valid state; it won't build without errors and minimally requires you cherry-pick microsoft/win32metadata#2042. Sounds like you did that already?

@MarijnS95
Copy link
Contributor Author

MarijnS95 commented Feb 4, 2025

Thanks @kennykerr, I'll test that out soon as I get the chance to boot into Windows (tool_bindings runs os-agnostic 😀) and regenerate those bindings!

EDIT: Nice! I managed to regenerate the bindings without those GdiPlus exemptions (and removing the whole GdiPlus module). That'll surely help you when a new win32metadata is released too 👍


@riverar That did not occur to me, only some WinHv files had unexpected changes (also mentioned to be exempt in another PR), as you can see in my linked PR there are no other unexpected changes (i.e. it's all constrained to d3d12): https://github.com/microsoft/win32metadata/pull/2055/files

@kennykerr
Copy link
Collaborator

Yep, it just interprets metadata and generates text files. There's nothing OS specific about that. 😊

@riverar
Copy link
Collaborator

riverar commented Feb 4, 2025

@MarijnS95 As an example, your winmd is missing AllJoynConnectToBus.

@MarijnS95
Copy link
Contributor Author

@kennykerr yup, it has always worked OS-agnostic for a very long time, then win32metadata tooling however is not cross-platform (and very slow) making it hard to get outstanding PRs finalized and merged. Any help (review and answers to questions) with my pull requests over on that repository is appreciated.


@riverar yes, the generated windows-rs crate also has some shifts in AllJoyn disappearing in places, and GameInput being new. Fortunately I don't use that API and am only interested in the D3D12 changes for this Agility SDK upgrade. I'll leave the rest of the maintenance and upgrades to you folks 😉

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants