Feature request: Apply standard Rust naming conventions for types that implement ownership semantics

[tim-weis]

Motivation

The motivation behind this feature request is to make client code more comprehensible. It's currently difficult to tell ABI(-like) "all-management-on-the-client" types and Rust's "ownership-is-a-solved-problem" types apart.

Since the question recently came up as to what the ownership semantics of the various types of the windows-rs crate are, I'm proposing to encode this information by following standard Rust naming conventions.

The only types affected by this proposal (to my knowledge) are BSTR and HSTRING (to be renamed to Bstr (BStr?) and HString, respectively). COM and WinRT interfaces are already following this convention, signaling to clients that ownership is handled. P[C][W]STR may need to be considered, still.

Drawbacks

This is a breaking change. Like any breaking change, it's going to cause noise, downstream.

I believe the perturbations to be manageable. From personal experience, both rustc and clippy do a fabulous job at driving authors to make the right adjustments.

Rationale and alternatives

As an alternative to renaming the types, type aliases could be provided for easier migration. I'm not sure this is strictly necessary pre-v1.0, and type aliases tend to increase complexity.

Additional context

This consideration is certainly going to come up again when support for VARIANTs (or SAFEARRAYs, maybe) is approached. Having distinct casing for the resource management wrappers and raw ABI types makes it easier to tell them apart.

[kennykerr]

I think this has potential but is not very clearcut. Simple examples like BSTR and HSTRING have potential but there are many Win32 structs that contain types with a Drop implementation and I can't imagine renaming all of these. This is also further complicated by #1896 and where that ends up landing - I'm not yet sure.

[tim-weis]

not very clearcut

Hm, true, I hadn't considered generated types...

Renaming those is certainly out of the question. Which would leave this feature request scoped to the "support library", i.e. types that live in the windows::core namespace. Which, in turn, is ultimately down to BSTR and HSTRING then.

While I would personally still prefer having those types follow Rust's naming convention, I will admit that this leaves much of the initial motivation untapped.

Maybe a better case for renaming those two types can be made when VARIANT support (presumably struct Variant(VARIANT)) is introduced, making BSTR look somewhat out of place.

[kennykerr]

Another reason we don't follow Rust's naming conventions is that it becomes very difficult to find corresponding documentation - switching from the official docs to Rust and back becomes very challenging. For that reason alone, I'd prefer to keep the naming consistent with the Windows SDK.

Going back to your original point about the difficulty of distinguishing between types that "do the right thing" and types that expect the caller to "do the right thing", I think we should focus on making more of the types in the windows crate do the right thing in an idiomatic way by providing either strong ownership or clear borrow lifetime. That's where I would like to get to, but the metadata is currently still somewhat ambiguous so I'm still wrestling through that.

[simonbuchan]

I think we should focus on making more of the types in the windows crate do the right thing in an idiomatic way by providing either strong ownership or clear borrow lifetime. That's where I would like to get to, but the metadata is currently still somewhat ambiguous so I'm still wrestling through that.

I don't know how plausible this is in the conventional "windows is a mostly safe version of the raw bindings in windows-sys" way, without tremendous effort at least. There's plenty of APIs where there's simply no information available, documentation or otherwise, and it's very difficult to express many of the thread safety or other lifetime requirements even with dedicated manual effort.

For example RegisterWaitForSingleObject is an API used by tokio to implement async fn wait() for tokio::process::Child. This creates a HANDLE that must be passed back to UnregisterWait (not CloseHandle) to free it, returned via an out parameter because it might invoke the callback before the function even returns. UnregisterWait is not safe to call while the callback is running, as it could race with the wait thread internals - unless you are using the WT_EXECUTEINWAITTHREAD flag, where the wait thread is not going to be doing anything else. I can't see any sane way to get metadata to cover this with any sort of reasonably generic annotation, and this is hardly a unique case.

If windows is going to be mostly unsafe still, then I feel the best effort of pushing people away from unsound code by encouraging patterns that are definitely not adding unsoundness is still very valuable, things like automatic parameter conversion, lots of common owned types as wrappers to encourage creating small safety wrappers, etc.. In other words, having an owned BStr documented as a safe wrapper for the raw BSTR is not at all a bad idea, but perhaps take a page from std::os::windows which uses RawHandle, OwnedHandle, HandleOrInvalid etc, and conversions between them all.