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

Simplify use of VARIANT #539

Closed
1Dragoon opened this issue Feb 12, 2021 · 15 comments · Fixed by #2786
Closed

Simplify use of VARIANT #539

1Dragoon opened this issue Feb 12, 2021 · 15 comments · Fixed by #2786
Labels
enhancement New feature or request

Comments

@1Dragoon
Copy link

1Dragoon commented Feb 12, 2021

Need support for VARIANT type for use with certain COM objects

From #536

VARIANT support is coming. Much like BSTR, there will likely be a helper struct named Variant that provides friendlier access to the underlying union.
@kennykerr kennykerr added the enhancement New feature or request label Feb 12, 2021
@kennykerr
Copy link
Collaborator

Thanks! This depends on #417, which I'll be wrapping up soon.

@kennykerr
Copy link
Collaborator

Unions are now supported. VARIANT though has union members that need to be dropped. While the union's memory layout is now correct, it doesn't provide support for dropping, so I'll leave this issue open until I add that.

@iainnicol
Copy link

iainnicol commented Mar 9, 2021

Wrapping members in ManuallyDrop, as you mention in the other issue, sounds plausible.

But in the specific case of VARIANT, should we (additionally) manually impl Drop, by calling VariantClear?

@kennykerr
Copy link
Collaborator

Yes, the code gen should inject a Drop impl for that. It already does that for BSTR.

@dpaoliello
Copy link
Collaborator

One more request to finish fleshing out VARIANT, please implement the Default trait as well - this makes it easier (and consistent with other types) to create one to pass-by-pointer to a function.

@mwcampbell
Copy link

Also eagerly awaiting this. Will be very helpful for my upcoming implementation of UI Automation provider interfaces in AccessKit.

@kennykerr
Copy link
Collaborator

I'm getting closer to being able to tackle this. 😉

@lunixbochs
Copy link

I just implemented Raymond Chen's How can I launch an unelevated process from my elevated process and vice versa using windows-rs, which required VARIANT for the ShellExecute call.

It took a while, and I have no idea if I did the ManuallyDrop stuff correctly. Here's the result, with a basic VARIANT helper: https://gist.github.com/lunixbochs/c55f6a3b3184d800f096b52997421113

I would love first-party VARIANT, I think it would've saved me a lot of time.

If anyone else needs VARIANT support in the meantime for more than just string and i32, you should be able to copy my impl From<T> for Variant example and cross-reference these definitions to figure out what to pass to Variant::new(). If you implement one of the ManuallyDrop types, make sure to add it to the impl Drop for Variant code.

@rgwood
Copy link

rgwood commented Jun 7, 2022

A related issue I've had in this area: propvarutil.h has a bunch of VARIANT helpers but it seems like they're not all present in windows-rs.

For example, InitVariantFromInt32Array() exists but it seems like InitVariantFromInt32() does not. Not sure why.

@riverar
Copy link
Collaborator

riverar commented Jun 8, 2022

@rgwood See the Remarks on that page and this FAQ entry. 👍

This is an inline function, with its source code provided in the header.

@hcldan
Copy link

hcldan commented Dec 19, 2023

Seeing as this is not completed yet I am hoping that someone here could help me out and tell me if I'm headed in the correct direction... here's what I have for creating a boolean VARIANT:

use windows::Win32::System::Variant::{VARIANT, VARIANT_0, VARIANT_0_0, VARIANT_0_0_0, VT_BOOL};
use windows::Win32::Foundation::VARIANT_BOOL;

let b = true;
VARIANT {
	Anonymous: VARIANT_0 {
		Anonymous: ManuallyDrop::new(VARIANT_0_0 {
			vt: VT_BOOL,
			wReserved1: 0,
			wReserved2: 0,
			wReserved3: 0,
			Anonymous: VARIANT_0_0_0 {
				boolVal: VARIANT_BOOL::from(b),
			},
		})
	},
}

@hcldan
Copy link

hcldan commented Dec 19, 2023

Also... I am getting unsafe access warnings when trying to access the innards of this Variant, like when I need to check the value of something inside. The compiler tells me I need to wrap my code in unsafe blocks.

Is there a way around this? How should I be accessing this?

More specifically, if I call a COM method that returns a VARIANT that contains an IDispatch, how can I safely unwrap that and do I need to manually dispose of any memory?

@tim-weis
Copy link
Contributor

@hcldan VARIANTs are gnarly, and using them from Rust produces terrifying code.

tell me if I'm headed in the correct direction

The code you have properly instantiates a VARIANT of type VT_BOOL.

The compiler tells me I need to wrap my code in unsafe blocks.

That is correct. Unions in Rust can be initialized from safe Rust, but reading and writing union fields is an inherently unsafe operation. This cannot be changed, and the only way around this would be a safe abstraction (which is what this issue is about).

how can I safely unwrap that and do I need to manually dispose of any memory?

You cannot touch union fields in safe Rust. An unsafe block is always required. Depending on what any given VARIANT stores, you may or may not need to release (shared) ownership. You could determine this by inspecting the vt field and conditionally releasing ownership or having the system deal with the complexity. The VARIANT wrappers I'd implemented simply call VariantClear() from their Drop implementation. This is potentially more costly than strictly necessary, but it's also quite possibly more correct than any code logic I could come up with.

@mominul
Copy link
Contributor

mominul commented Dec 21, 2023

In the meantime while windows crate gain support for ergonomic usage of VARIANT, variant-rs is a good alternative to use for working with the VARIANT type.

@kennykerr
Copy link
Collaborator

See #2780 for VARIANT support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet