Add natvis for Result, NonNull, CString, CStr, and Cow

[rylev]

This adds natvis support (used for Windows debugging) to the following types: Result, NonNull, CString, CStr, and Cow.

[rust-highfive]

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

[Mark-Simulacrum]

r? @varkor or @petrochenkov perhaps who've reviewed similar PRs before

[lzybkr]

It looks like this could cause an exception if inner was NULL.

[rylev]

I believe inner cannot be null as it's guaranteed to contain at least a null byte:

pub struct CStr {
    inner: [c_char],
}

impl CStr {
 pub fn from_bytes_with_nul(bytes: &[u8]) -> Result<&CStr, FromBytesWithNulError> {
        let nul_pos = memchr::memchr(0, bytes);
        if let Some(nul_pos) = nul_pos {
            if nul_pos + 1 != bytes.len() {
                return Err(FromBytesWithNulError::interior_nul(nul_pos));
            }
            Ok(unsafe { CStr::from_bytes_with_nul_unchecked(bytes) })
        } else {
            Err(FromBytesWithNulError::not_nul_terminated())
        }
    }
}

[lzybkr]

The documentation for CStr::from_ptr is less reassuring:

* There is no guarantee to the validity of ptr.
* The returned lifetime is not guaranteed to be the actual lifetime of ptr.
* There is no guarantee that the memory pointed to by ptr contains a valid nul terminator byte at the end of the string.
* It is not guaranteed that the memory pointed by ptr won't change before the CStr has been destroyed.

Note: This operation is intended to be a 0-cost cast but it is currently implemented with an up-front calculation of the length of the string. This is not guaranteed to always be the case.

So indeed, it looks safe now, but may not always be safe.

[rylev]

That’s the unsafe documentation of from_ptr. The ptr must meet those criteria in order for the call to from_ptr to be safe. So, users of CStr can assume that the internal pointer does meet those criteria. If it doesn’t, it’s a big somewhere else (a caller of from_ptr messed up).

[lzybkr]

Debuggers are used to help track down bugs like this, not introduce exceptions where there might not have been one before, even if the debugger is helping expose a latent bug.

At any rate, I don't have a strong preference here - I just wanted to raise the issue. While it's nice to see the length of the CStr automatically, it also doesn't feel critical if there are downsides.

[rylev]

That's fair though I think the debugger showing an error saying it can't show some detail would be a hint to the user that something is very wrong. If the debugger can't show CStr characters, then the user knows there has been an error in unsafe. I actually don't see this as a downside personally.

[varkor]

@bors r+

[bors]

📌 Commit 920e2d8 has been approved by varkor