Auto merge of rust-lang#96195 - sunfishcode:sunfishcode/handle-or-error-type, r=joshtriplett

 Define a dedicated error type for `HandleOrNull` and `HandleOrInvalid`.

Define `NullHandleError` and `InvalidHandleError` types, that implement std::error::Error, and use them as the error types in `HandleOrNull` and `HandleOrInvalid`,

This addresses [this concern](rust-lang#87074 (comment)).

This is the same as rust-lang#95387.

r? `@joshtriplett`

Author: bors

library/std/src/os/windows/io/handle.rs

@@ -76,7 +76,7 @@ pub struct OwnedHandle {
 /// `NULL`. This ensures that such FFI calls cannot start using the handle without
 /// checking for `NULL` first.
 ///
-/// This type concerns any value other than `NULL` to be valid, including `INVALID_HANDLE_VALUE`.
+/// This type considers any value other than `NULL` to be valid, including `INVALID_HANDLE_VALUE`.
 /// This is because APIs that use `NULL` as their sentry value don't treat `INVALID_HANDLE_VALUE`
 /// as special.
 ///
@@ -96,7 +96,7 @@ pub struct HandleOrNull(OwnedHandle);
 /// `INVALID_HANDLE_VALUE`. This ensures that such FFI calls cannot start using the handle without
 /// checking for `INVALID_HANDLE_VALUE` first.
 ///
-/// This type concerns any value other than `INVALID_HANDLE_VALUE` to be valid, including `NULL`.
+/// This type considers any value other than `INVALID_HANDLE_VALUE` to be valid, including `NULL`.
 /// This is because APIs that use `INVALID_HANDLE_VALUE` as their sentry value may return `NULL`
 /// under `windows_subsystem = "windows"` or other situations where I/O devices are detached.
 ///
@@ -143,17 +143,17 @@ impl BorrowedHandle<'_> {
 }
 
 impl TryFrom<HandleOrNull> for OwnedHandle {
-    type Error = ();
+    type Error = NullHandleError;
 
     #[inline]
-    fn try_from(handle_or_null: HandleOrNull) -> Result<Self, ()> {
+    fn try_from(handle_or_null: HandleOrNull) -> Result<Self, NullHandleError> {
         let owned_handle = handle_or_null.0;
         if owned_handle.handle.is_null() {
             // Don't call `CloseHandle`; it'd be harmless, except that it could
             // overwrite the `GetLastError` error.
             forget(owned_handle);
 
-            Err(())
+            Err(NullHandleError(()))
         } else {
             Ok(owned_handle)
         }
@@ -201,23 +201,59 @@ impl OwnedHandle {
 }
 
 impl TryFrom<HandleOrInvalid> for OwnedHandle {
-    type Error = ();
+    type Error = InvalidHandleError;
 
     #[inline]
-    fn try_from(handle_or_invalid: HandleOrInvalid) -> Result<Self, ()> {
+    fn try_from(handle_or_invalid: HandleOrInvalid) -> Result<Self, InvalidHandleError> {
         let owned_handle = handle_or_invalid.0;
         if owned_handle.handle == c::INVALID_HANDLE_VALUE {
             // Don't call `CloseHandle`; it'd be harmless, except that it could
             // overwrite the `GetLastError` error.
             forget(owned_handle);
 
-            Err(())
+            Err(InvalidHandleError(()))
         } else {
             Ok(owned_handle)
         }
     }
 }
 
+/// This is the error type used by [`HandleOrNull`] when attempting to convert
+/// into a handle, to indicate that the value is null.
+// The empty field prevents constructing this, and allows extending it in the future.
+#[unstable(feature = "io_safety", issue = "87074")]
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct NullHandleError(());
+
+#[unstable(feature = "io_safety", issue = "87074")]
+impl fmt::Display for NullHandleError {
+    fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+        "A HandleOrNull could not be converted to a handle because it was null".fmt(fmt)
+    }
+}
+
+#[unstable(feature = "io_safety", issue = "87074")]
+impl crate::error::Error for NullHandleError {}
+
+/// This is the error type used by [`HandleOrInvalid`] when attempting to
+/// convert into a handle, to indicate that the value is
+/// `INVALID_HANDLE_VALUE`.
+// The empty field prevents constructing this, and allows extending it in the future.
+#[unstable(feature = "io_safety", issue = "87074")]
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct InvalidHandleError(());
+
+#[unstable(feature = "io_safety", issue = "87074")]
+impl fmt::Display for InvalidHandleError {
+    fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+        "A HandleOrInvalid could not be converted to a handle because it was INVALID_HANDLE_VALUE"
+            .fmt(fmt)
+    }
+}
+
+#[unstable(feature = "io_safety", issue = "87074")]
+impl crate::error::Error for InvalidHandleError {}
+
 impl AsRawHandle for BorrowedHandle<'_> {
     #[inline]
     fn as_raw_handle(&self) -> RawHandle {

src/test/ui/coercion/coerce-issue-49593-box-never-windows.nofallback.stderr

@@ -0,0 +1,39 @@
+error[E0277]: the trait bound `(): std::error::Error` is not satisfied
+  --> $DIR/coerce-issue-49593-box-never-windows.rs:18:53
+   |
+LL |     /* *mut $0 is coerced to Box<dyn Error> here */ Box::<_ /* ! */>::new(x)
+   |                                                     ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::error::Error` is not implemented for `()`
+   |
+   = help: the following other types implement trait `std::error::Error`:
+             !
+             &'a T
+             AccessError
+             AddrParseError
+             Arc<T>
+             BorrowError
+             BorrowMutError
+             Box<T>
+           and 45 others
+   = note: required for the cast to the object type `dyn std::error::Error`
+
+error[E0277]: the trait bound `(): std::error::Error` is not satisfied
+  --> $DIR/coerce-issue-49593-box-never-windows.rs:23:49
+   |
+LL |     /* *mut $0 is coerced to *mut Error here */ raw_ptr_box::<_ /* ! */>(x)
+   |                                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::error::Error` is not implemented for `()`
+   |
+   = help: the following other types implement trait `std::error::Error`:
+             !
+             &'a T
+             AccessError
+             AddrParseError
+             Arc<T>
+             BorrowError
+             BorrowMutError
+             Box<T>
+           and 45 others
+   = note: required for the cast to the object type `(dyn std::error::Error + 'static)`
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0277`.

src/test/ui/coercion/coerce-issue-49593-box-never-windows.rs

@@ -0,0 +1,58 @@
+// revisions: nofallback fallback
+// only-windows - the number of `Error` impls is platform-dependent
+//[fallback] check-pass
+//[nofallback] check-fail
+
+#![feature(never_type)]
+#![cfg_attr(fallback, feature(never_type_fallback))]
+#![allow(unreachable_code)]
+
+use std::error::Error;
+use std::mem;
+
+fn raw_ptr_box<T>(t: T) -> *mut T {
+    panic!()
+}
+
+fn foo(x: !) -> Box<dyn Error> {
+    /* *mut $0 is coerced to Box<dyn Error> here */ Box::<_ /* ! */>::new(x)
+    //[nofallback]~^ ERROR trait bound `(): std::error::Error` is not satisfied
+}
+
+fn foo_raw_ptr(x: !) -> *mut dyn Error {
+    /* *mut $0 is coerced to *mut Error here */ raw_ptr_box::<_ /* ! */>(x)
+    //[nofallback]~^ ERROR trait bound `(): std::error::Error` is not satisfied
+}
+
+fn no_coercion(d: *mut dyn Error) -> *mut dyn Error {
+    /* an unsize coercion won't compile here, and it is indeed not used
+       because there is nothing requiring the _ to be Sized */
+    d as *mut _
+}
+
+trait Xyz {}
+struct S;
+struct T;
+impl Xyz for S {}
+impl Xyz for T {}
+
+fn foo_no_never() {
+    let mut x /* : Option<S> */ = None;
+    let mut first_iter = false;
+    loop {
+        if !first_iter {
+            let y: Box<dyn Xyz>
+                = /* Box<$0> is coerced to Box<Xyz> here */ Box::new(x.unwrap());
+        }
+
+        x = Some(S);
+        first_iter = true;
+    }
+
+    let mut y : Option<S> = None;
+    // assert types are equal
+    mem::swap(&mut x, &mut y);
+}
+
+fn main() {
+}

src/test/ui/coercion/coerce-issue-49593-box-never.nofallback.stderr

@@ -1,5 +1,5 @@
 error[E0277]: the trait bound `(): std::error::Error` is not satisfied
-  --> $DIR/coerce-issue-49593-box-never.rs:17:53
+  --> $DIR/coerce-issue-49593-box-never.rs:18:53
    |
 LL |     /* *mut $0 is coerced to Box<dyn Error> here */ Box::<_ /* ! */>::new(x)
    |                                                     ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::error::Error` is not implemented for `()`
@@ -17,7 +17,7 @@ LL |     /* *mut $0 is coerced to Box<dyn Error> here */ Box::<_ /* ! */>::new(x
    = note: required for the cast to the object type `dyn std::error::Error`
 
 error[E0277]: the trait bound `(): std::error::Error` is not satisfied
-  --> $DIR/coerce-issue-49593-box-never.rs:22:49
+  --> $DIR/coerce-issue-49593-box-never.rs:23:49
    |
 LL |     /* *mut $0 is coerced to *mut Error here */ raw_ptr_box::<_ /* ! */>(x)
    |                                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::error::Error` is not implemented for `()`

src/test/ui/coercion/coerce-issue-49593-box-never.rs

@@ -1,4 +1,5 @@
 // revisions: nofallback fallback
+// ignore-windows - the number of `Error` impls is platform-dependent
 //[fallback] check-pass
 //[nofallback] check-fail