Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

VARIANT in wrong namespace? #1019

Closed
kennykerr opened this issue Jul 29, 2022 · 15 comments
Closed

VARIANT in wrong namespace? #1019

kennykerr opened this issue Jul 29, 2022 · 15 comments
Assignees
Labels
namespaces Feedback on namespace names or organization rust Critical for Rust adoption

Comments

@kennykerr
Copy link
Contributor

Starting working on microsoft/windows-rs#539 and I find it a little strange that VARIANT is defined in Win32.System.Com while all of the functions for manipulating this type are in Win32.System.Ole - is this intentional?

@mikebattista mikebattista self-assigned this Aug 1, 2022
@mikebattista mikebattista added the namespaces Feedback on namespace names or organization label Aug 1, 2022
@mikebattista
Copy link
Collaborator

There are VARIANT-related functions in COM as well like VARIANT_UserMarshal. Where do you recommend they all go? Should these all be COM APIs or OLE APIs?

@sotteson1
Copy link
Contributor

If VARIANT goes into the Ole namespace, it will cause some namespace cycles with other namespaces. Maybe all those manipulation functions should go into Com, though.

@kennykerr kennykerr added the rust Critical for Rust adoption label Sep 2, 2022
@kennykerr
Copy link
Contributor Author

Not critical - I'll just close this for now.

@kennykerr
Copy link
Contributor Author

Tried again to fix microsoft/windows-rs#539 but it's really not practical to support when VARIANT and its associated functions aren't in the same namespace.

Regarding cycles, I've never been able to make that work so I'm less interested in breaking that.

I'll reopen as @riverar suggested this could be fixed.

@kennykerr kennykerr reopened this Feb 27, 2023
@mikebattista
Copy link
Collaborator

What's the recommendation here?

@riverar
Copy link
Collaborator

riverar commented Feb 28, 2023

One idea we were kicking around was putting VARIANT and associated structs/functions into a new variant-only namespace (e.g. Win32.System.Variant). Think that'll work well?

@mikebattista
Copy link
Collaborator

Sure if that works for you.

@mikebattista
Copy link
Collaborator

Does PROPVARIANT belong together with VARIANT?

@kennykerr
Copy link
Contributor Author

Although similar, the structs are independent and PROPVARIANT is specific to structured storage.

@mikebattista
Copy link
Collaborator

Thanks. I'm moving all PROPVARIANT APIs to Windows.Win32.System.Com.StructuredStorage as part of this change. VARIANT APIs will be in Windows.Win32.System.Variant.

@AArnott
Copy link
Member

AArnott commented May 10, 2023

The move to a new namespace just hit CsWin32. It's easy to accommodate, but VARIANT is the only struct in the new namespace (accompanied by 4 enums), it leaves me wondering why it needs its very own namespace. Seems like the Foundation namespace might have been more suitable.

@mikebattista
Copy link
Collaborator

There are a bunch of methods there as well.

@AArnott
Copy link
Member

AArnott commented May 10, 2023

Ah, I hadn't noticed that. Although for CsWin32 that makes little difference since we always generate all methods onto the one PInvoke class. Although I guess for our namespace wildcard matching (if anyone uses that), it could be useful. So for .NET this is a bit weird, but it's better for other projections, I guess we win some, we lose some. Not a huge deal.

@kennykerr
Copy link
Contributor Author

Just discussing this with @ChrisDenton over in microsoft/windows-rs#3282 and I think we should reconsider VARIANT and PROPVARIANT being in different namespaces. Even though the two "variant" types are distinct, the APIs cannot reasonably be used independently from each other and having them in a different namespace makes it overly hard to use either. Can we move all or most of what's in "Windows.Win32.System.Com.StructuredStorage" into the "Windows.Win32.System.Variant" namespace?

@kennykerr kennykerr reopened this Sep 14, 2024
@kennykerr
Copy link
Contributor Author

I think I'll just close this for now - I have a workaround and simply moving these types won't necessarily make the issue less problematic because Windows.Win32.System.Variant.VARIANT itself depends on Windows.Win32.System.Com and Windows.Win32.System.Ole so the web of dependencies between VARIANT and PROPVARIANT is just the tip of the iceberg.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
namespaces Feedback on namespace names or organization rust Critical for Rust adoption
Projects
None yet
Development

No branches or pull requests

5 participants