Remove unsafe conversion for PCWSTR

[rylev]

As @michaelwoerister pointed out this conversion, while safe, makes it very easy to write incorrect code. I think we should avoid such code.

Here's an example of some code that uses safe Rust but creates a dangling pointer:

fn create_dangling_pcwstr_pointer() -> PCWSTR {
    let s = HSTRING::from("abc");
    PCWSTR::from(&s)
}

[michaelwoerister]

Yes, this does indeed look like a potential source of errors to me.

Overall, I don't know of a good way of dealing with PCWSTR and similar string types that are essentially raw pointers with no ownership or lifetime guarantees attached.

[kennykerr]

This has come up before (#2084). In this case, we caught it with App Verifier (#2095) which does a great job of detecting such errors.

I'm just not sure simply removing the impl is sufficient since we still need a way to perform such a conversion.

[kennykerr]

This is also related to #907 and #1896 where we really need to track lifetime even, or especially, in cases where ownership is ambiguous.

[kennykerr]

In this case, perhaps its preferable to be explicit and require something like this (from the build break)?

        let vertex_shader = unsafe {
            D3DCompileFromFile(
                PCWSTR(shaders_hlsl.as_ptr()),
                None,
                None,

[michaelwoerister]

In this case, perhaps its preferable to be explicit and require something like this (from the build break)?

        let vertex_shader = unsafe {
            D3DCompileFromFile(
                PCWSTR(shaders_hlsl.as_ptr()),
                None,
                None,

That does look better to me, yes.

An explicit to_pcwstr() method might also make sense. It could be safe, like e.g. Vec::as_ptr() is safe, but would serve as a place where documentation about safety/lifetime can be added -- but I hesitate to suggest adding ad-hoc convenience methods like this without an overall design strategy for strings and/or win32 raw-pointers.

[kennykerr]

but I hesitate to suggest adding ad-hoc convenience methods like this without an overall design strategy for strings and/or win32 raw-pointers

Right, once the overall ownership story has been sorted out, see also #2126 (comment), this should be a lot clearer. Until then, I'm happy to require PCWSTR(from.as_ptr()).

[kennykerr]

Looks like Ryan's away. I'll close this and open an issue to take care of this.