Update metadata to 10.2.84-preview and regroup UseTrees #834

MarijnS95 · 2021-06-05T21:06:39Z

The 10.2.84-preview win32metadata just dropped and only required few extra modifications since starting on this a few weeks ago, to regroup and clean up UseTrees. Not all are done yet, but it should look a lot better already (also keeping in mind #827/#828).

Note that this uses the file from https://github.com/microsoft/win32metadata/tree/v10.2.84-preview/scripts/BaselineWinmd because the nuget package appears to miss some types from the foundation namespace 🤔 - still investigating. The broken types switched from TypeRef to TypeDef.

riverar · 2021-06-05T21:28:06Z

I didn't run into missing type issues; if there's anything here worth scavenging, feel free riverar@b2e0ade

MarijnS95 · 2021-06-05T21:31:36Z

@riverar I get the same Could not find type def `Windows.Win32.Foundation.HRESULT` error on your branch when running windows_bindings, have you ran that too?

riverar · 2021-06-05T21:35:34Z

@MarijnS95 Are you on rustc nightly? I think they broke macro ABI recently, had to switch to stable. (Was fighting those same errors!)

MarijnS95 · 2021-06-05T21:49:52Z

@MarijnS95 Are you on rustc nightly? I think they broke macro ABI recently, had to switch to stable. (Was fighting those same errors!)

I'd be surprised if it's that, given that the baseline .winmd from the repo does work correctly. But no, the compiler is on stable 1.52.1. For good measure beta and nightly fail in the same way - likely something is wrong in the .winmd instead even though the types are successfully found with IlSpy.

riverar · 2021-06-05T21:59:31Z

@MarijnS95 ~~All my tests/build steps pass locally. Hm.~~ OK, started fresh and yep, can reproduce now.

riverar · 2021-06-05T22:28:53Z

@MarijnS95 Do we know why HRESULT is on the type exclusion list here? Removing this resolves the problem, but I'm wondering if that's just a hack covering up a problem.

windows-rs/crates/gen/src/parser/type_reader.rs

Line 125 in 9bca156

("Windows.Win32.Foundation", "HRESULT"),

kennykerr · 2021-06-06T01:20:34Z

Do we know why HRESULT is on the type exclusion list here

Types that are represented both in WinRT and Win32 must be excluded and defined by the Windows crate directly.

microsoft#834 (comment)

MarijnS95 · 2021-06-06T16:21:55Z

Found the culrpit: HSTRING and HRESULT are a TypeDef now, not a TypeRef. Interesting how that changed between the baseline and the nuget release.

microsoft#834 (comment)

riverar · 2021-06-06T17:02:04Z

@MarijnS95 Nice, looks good!

kennykerr · 2021-06-07T18:18:43Z

crates/gen/src/parser/element_type.rs

-                            .resolve_type_def(type_def.namespace(), type_def.name())
-                            .into()
-                    }
+                    TypeDefOrRef::TypeDef(type_def) => match type_def.full_name() {


This is something I still need to fix. There's no guarantee the types will be TypeDefs or TypeRefs. I was hoping in practice they'd only be TypeRefs as that's simpler but as long as the TypeDef resides in the same PE file then they can be TypeDefs. Ideally the arch-specific types would be TyepRefs though. That's a bug in the metadata (see comment below).

#845 deals with these issues.

Which of the two do you want to merge first? I think I can kick off the second commit if #845 goes in prior.

OK, will complete 845.

Ah, by kicking off the second commit the metadata import from nuget got lost as well, apologies. Thanks for pushing that back @kennykerr :)

kennykerr · 2021-06-07T18:20:11Z

Note that this uses the file from https://github.com/microsoft/win32metadata/tree/v10.2.84-preview/scripts/BaselineWinmd

I can only accept the winmd from the release.

MarijnS95 · 2021-06-07T18:26:30Z

Note that this uses the file from https://github.com/microsoft/win32metadata/tree/v10.2.84-preview/scripts/BaselineWinmd

I can only accept the winmd from the release.

With aforementioned fix in the second commit this is now using the release blob - I'll remove that note from the description.

kennykerr · 2021-06-07T19:53:48Z

crates/gen/src/types/bstr.rs

@@ -24,7 +24,7 @@ pub fn gen_bstr() -> TokenStream {
                    return 0;
                }

-                unsafe { SysStringLen(self) as usize }
+                unsafe { super::System::OleAutomation::SysStringLen(self) as usize }


This seems wrong. Created an issue: microsoft/win32metadata#517

microsoft#834 (comment)

kennykerr · 2021-06-08T14:50:44Z

This looks good - I need to get the update in. Can you take care of the conflicts and then I'll merge this in.

MarijnS95 · 2021-06-08T15:39:19Z

This looks good - I need to get the update in. Can you take care of the conflicts and then I'll merge this in.

Self-induced conflicts with all the formatting 😬. Can we get #844 in first so that I can tackle them all at once?

kennykerr · 2021-06-08T15:41:12Z

Done.

microsoft#834 (comment)

MarijnS95 · 2021-06-08T20:53:15Z

@kennykerr Updated. Please give this some thorough scrutineering, most changes didn't survive autoformatting nor the removal of enum constants.

MarijnS95 · 2021-06-08T20:55:04Z

Note that some common paths in use trees have been merged, but not all/everywhere. What format is desired? Someone should do a pass to unify/normalize them all, both in build.rs as in the use bindings:: blocks in the source.

kennykerr

Looks good!

MarijnS95 force-pushed the update-metadata branch 2 times, most recently from f2fbe29 to 9bca156 Compare June 5, 2021 21:14

MarijnS95 added a commit to MarijnS95/windows-rs that referenced this pull request Jun 6, 2021

gen/element_type: HRESULT and HSTRING are now TypeDefs, not TypeRef

3ee003d

microsoft#834 (comment)

MarijnS95 added a commit to MarijnS95/windows-rs that referenced this pull request Jun 6, 2021

gen/element_type: HRESULT and HSTRING are now TypeDefs, not TypeRef

046983c

microsoft#834 (comment)

MarijnS95 force-pushed the update-metadata branch from 3ee003d to 046983c Compare June 6, 2021 16:28

kennykerr reviewed Jun 7, 2021

View reviewed changes

kennykerr mentioned this pull request Jun 7, 2021

BSTR microsoft/win32metadata#517

Closed

kennykerr reviewed Jun 7, 2021

View reviewed changes

MarijnS95 added a commit to MarijnS95/windows-rs that referenced this pull request Jun 7, 2021

gen/element_type: HRESULT and HSTRING are now TypeDefs, not TypeRef

c740de0

microsoft#834 (comment)

MarijnS95 force-pushed the update-metadata branch from 046983c to c740de0 Compare June 7, 2021 19:56

MarijnS95 added a commit to MarijnS95/windows-rs that referenced this pull request Jun 8, 2021

gen/element_type: HRESULT and HSTRING are now TypeDefs, not TypeRef

ded3bbc

microsoft#834 (comment)

MarijnS95 force-pushed the update-metadata branch from c740de0 to ded3bbc Compare June 8, 2021 20:52

Update metadata to 10.2.84-preview and regroup UseTrees

31a8050

MarijnS95 force-pushed the update-metadata branch from ded3bbc to 31a8050 Compare June 8, 2021 21:33

kennykerr approved these changes Jun 8, 2021

View reviewed changes

kennykerr merged commit 39eb4d2 into microsoft:master Jun 8, 2021

MarijnS95 deleted the update-metadata branch June 8, 2021 22:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update metadata to 10.2.84-preview and regroup UseTrees #834

Update metadata to 10.2.84-preview and regroup UseTrees #834

MarijnS95 commented Jun 5, 2021 •

edited

Loading

riverar commented Jun 5, 2021 •

edited

Loading

MarijnS95 commented Jun 5, 2021

riverar commented Jun 5, 2021 •

edited

Loading

MarijnS95 commented Jun 5, 2021

riverar commented Jun 5, 2021 •

edited

Loading

riverar commented Jun 5, 2021

kennykerr commented Jun 6, 2021

MarijnS95 commented Jun 6, 2021

riverar commented Jun 6, 2021

kennykerr Jun 7, 2021

kennykerr Jun 8, 2021

MarijnS95 Jun 8, 2021 •

edited

Loading

kennykerr Jun 8, 2021

MarijnS95 Jun 9, 2021

kennykerr commented Jun 7, 2021

MarijnS95 commented Jun 7, 2021 •

edited

Loading

kennykerr Jun 7, 2021

kennykerr commented Jun 8, 2021

MarijnS95 commented Jun 8, 2021

kennykerr commented Jun 8, 2021

MarijnS95 commented Jun 8, 2021

MarijnS95 commented Jun 8, 2021

kennykerr left a comment

Update metadata to 10.2.84-preview and regroup UseTrees #834

Update metadata to 10.2.84-preview and regroup UseTrees #834

Conversation

MarijnS95 commented Jun 5, 2021 • edited Loading

riverar commented Jun 5, 2021 • edited Loading

MarijnS95 commented Jun 5, 2021

riverar commented Jun 5, 2021 • edited Loading

MarijnS95 commented Jun 5, 2021

riverar commented Jun 5, 2021 • edited Loading

riverar commented Jun 5, 2021

kennykerr commented Jun 6, 2021

MarijnS95 commented Jun 6, 2021

riverar commented Jun 6, 2021

kennykerr Jun 7, 2021

Choose a reason for hiding this comment

kennykerr Jun 8, 2021

Choose a reason for hiding this comment

MarijnS95 Jun 8, 2021 • edited Loading

Choose a reason for hiding this comment

kennykerr Jun 8, 2021

Choose a reason for hiding this comment

MarijnS95 Jun 9, 2021

Choose a reason for hiding this comment

kennykerr commented Jun 7, 2021

MarijnS95 commented Jun 7, 2021 • edited Loading

kennykerr Jun 7, 2021

Choose a reason for hiding this comment

kennykerr commented Jun 8, 2021

MarijnS95 commented Jun 8, 2021

kennykerr commented Jun 8, 2021

MarijnS95 commented Jun 8, 2021

MarijnS95 commented Jun 8, 2021

kennykerr left a comment

Choose a reason for hiding this comment

MarijnS95 commented Jun 5, 2021 •

edited

Loading

riverar commented Jun 5, 2021 •

edited

Loading

riverar commented Jun 5, 2021 •

edited

Loading

riverar commented Jun 5, 2021 •

edited

Loading

MarijnS95 Jun 8, 2021 •

edited

Loading

MarijnS95 commented Jun 7, 2021 •

edited

Loading