rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
99.2k stars 12.81k forks source link

`#[repr(transparent)]` causing weird and confusing type errors #122682

Open RivenSkaye opened 8 months ago

RivenSkaye commented 8 months ago

Code

// I tried making an MRE, but it _requires_ being split crates it seems
// dep: windows = {version = "0.54", features = ["Win32_Foundation"]}
use windows::core::{Error as WErr, HRESULT};

fn some_res(succeed: i32) -> Result<(), WErr> {
    match succeed {
        1 => Err(WErr::new(HRESULT(succeed), "This doesn't work")), // Error: expected an i32
        2 => Err(WErr::new(succeed, "Neither does this")), // Error: expected a HRESULT
        3 => Err(WErr::new(succeed.into(), "Nor this")), // Error: the trait bound `HRESULT: From<i32>` is not satisfied
        255 => Err(WErr::new(255.into(), "And this fails too")), // Error: the trait bound `HRESULT: From<{integer}>` is not satisfied
        0 => Ok(()),
        _ => Err(WErr::new(HRESULT(succeed).into(), "This is what you need for it to work")),
    }
}

fn main() {
    some_res(1).expect("If you get here, compilation succeeded")
}

Current output

Error: expected an i32

Error: expected `HRESULT`
... snipped ...
help: try wrapping the expression in `windows_result::hresult::HRESULT`
    |
219 |                 return Err(WErr::new(windows_result::hresult::HRESULT(9), "File is uninitialized!"));
    |                                      +++++++++++++++++++++++++++++++++ +

Error: the trait bound `HRESULT: From<i32>` is not satisfied
Error: the trait bound `HRESULT: From<{integer}>` is not satisfied

image

Desired output

I'd expect something more clear wrt the expected type. A blurb like <type> is defined as <underlying> with transparent repr would also help to understand it might be worth looking at the code for the actual type. Suggesting the outer type with .into() could also be work considering that fixes the errors in this case, though I'm not sure it always will.

Rationale and extra context

I opened microsoft/windows-rs#2936 about this as well. It seems that crates offering repr(transparent) types are being processed very oddly by rustc. Where I'd expect a clearer error, the quick fix advises users to change wrapper->underlying and vice versa. This creates a very confusing situation. Subsequently verifying the type signature through e.g. the vscode rust-analyzer plugin doesn't help the confusion (and in fact, only makes it worse in the example of the windows-rs issue) because they also list the underlying type rather than the lib's wrapper.

Other cases

Doing this in your own crate is issue free, it's specifically when grabbing a crate from somewhere else that it breaks

Rust Version

rustc 1.78.0-nightly (3cbb93223 2024-03-13)
binary: rustc
commit-hash: 3cbb93223f33024db464a4df27a13c7cce870173
commit-date: 2024-03-13
host: x86_64-pc-windows-msvc
release: 1.78.0-nightly
LLVM version: 18.1.0

Anything else?

I'm not sure whether the type hinting shown in vscode through the plugin is rust-analyzer or just forwarded from rustc; I'll happily open an issue elsewhere if (part) this is analyzer-specific.

cargo check doesn't seem to mind HRESULT(some_i32) but RA immediately produces E0308 upon saving and I'm not sure which is correct and if there's any safety or soundness impact, which is pretty relevant in the code base this showed up in

RivenSkaye commented 8 months ago

As the above was based on old code for crate-internal use, it might be worth noting that I'm facing the same issue when using an Error type of my own, where a From<WErr> implementation produces the same result.

Was using ? for some internal functions in this code, hence propagating the errors from the windows crate. Cleaning that up now to start forwarding my errors much sooner while doing some other restructuring required for the 0.52 => 0.54 upgrade path anyway, which is why I ran into this. But in a simple pattern match against the internal error code I'm now also stuck with match value.code().into() because accessing the (public!) i32 field value.code().0 is telling me i32 has no such field

workingjubilee commented 8 months ago

@RivenSkaye cargo check is the source of truth, if cargo check and rust-analyzer disagrees then the response is not because of rustc, it is because of rust-analyzer. please file this bug against rust-analyzer.