From 703a33673de30572960eb3fe2c36a0f51083d226 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Sun, 27 Mar 2022 15:41:13 -0700 Subject: [PATCH 01/11] Define a dedicated error type for `HandleOrNull` and `HandleOrInvalid`. Define a `NotHandle` type, that implements `std::error::Error`, and use it as the error type in `HandleOrNull` and `HandleOrInvalid`. --- library/std/src/os/windows/io/handle.rs | 26 +++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/library/std/src/os/windows/io/handle.rs b/library/std/src/os/windows/io/handle.rs index ee30cc8be6b57..1fb448be5dedf 100644 --- a/library/std/src/os/windows/io/handle.rs +++ b/library/std/src/os/windows/io/handle.rs @@ -143,17 +143,17 @@ impl BorrowedHandle<'_> { } impl TryFrom for OwnedHandle { - type Error = (); + type Error = NotHandle; #[inline] - fn try_from(handle_or_null: HandleOrNull) -> Result { + fn try_from(handle_or_null: HandleOrNull) -> Result { 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(NotHandle(())) } else { Ok(owned_handle) } @@ -201,23 +201,37 @@ impl OwnedHandle { } impl TryFrom for OwnedHandle { - type Error = (); + type Error = NotHandle; #[inline] - fn try_from(handle_or_invalid: HandleOrInvalid) -> Result { + fn try_from(handle_or_invalid: HandleOrInvalid) -> Result { 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(NotHandle(())) } else { Ok(owned_handle) } } } +/// This is the error type used by [`HandleOrInvalid`] and +/// [`HandleOrNull`] when attempting to convert into a handle, +/// to indicate that the value is not a handle. +#[unstable(feature = "io_safety", issue = "87074")] +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub struct NotHandle(()); + +#[unstable(feature = "io_safety", issue = "87074")] +impl fmt::Display for NotHandle { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + "the return value of a Windows API call indicated an error".fmt(fmt) + } +} + impl AsRawHandle for BorrowedHandle<'_> { #[inline] fn as_raw_handle(&self) -> RawHandle { From 5b3023c56420ce55d8b6761ea557fd4d6f579bdf Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Sun, 27 Mar 2022 16:53:56 -0700 Subject: [PATCH 02/11] Fix an incorrect word in a comment. --- library/std/src/os/windows/io/handle.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/std/src/os/windows/io/handle.rs b/library/std/src/os/windows/io/handle.rs index 1fb448be5dedf..1bed22a4ce50a 100644 --- a/library/std/src/os/windows/io/handle.rs +++ b/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. /// From 67994b77fda81223c9e71f0d476aa849c9be9699 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Sun, 27 Mar 2022 16:54:58 -0700 Subject: [PATCH 03/11] Move the `Error` impl for `NotHandle` out of platform-independent code. --- library/std/src/os/windows/io/handle.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/std/src/os/windows/io/handle.rs b/library/std/src/os/windows/io/handle.rs index 1bed22a4ce50a..c6b84b6e5d300 100644 --- a/library/std/src/os/windows/io/handle.rs +++ b/library/std/src/os/windows/io/handle.rs @@ -232,6 +232,9 @@ impl fmt::Display for NotHandle { } } +#[unstable(feature = "io_safety", issue = "87074")] +impl crate::error::Error for NotHandle {} + impl AsRawHandle for BorrowedHandle<'_> { #[inline] fn as_raw_handle(&self) -> RawHandle { From f934043c179eb2d9c1dda4e7a70284e0d9354c82 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 28 Mar 2022 10:56:00 -0700 Subject: [PATCH 04/11] Split `NotHandle` into `NullHandleError` and `InvalidHandleError`. Also, make the display messages more specific, and remove the `Copy` implementation. --- library/std/src/os/windows/io/handle.rs | 45 +++++++++++++++++-------- 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/library/std/src/os/windows/io/handle.rs b/library/std/src/os/windows/io/handle.rs index c6b84b6e5d300..e48f630f76e7f 100644 --- a/library/std/src/os/windows/io/handle.rs +++ b/library/std/src/os/windows/io/handle.rs @@ -143,17 +143,17 @@ impl BorrowedHandle<'_> { } impl TryFrom for OwnedHandle { - type Error = NotHandle; + type Error = NullHandleError; #[inline] - fn try_from(handle_or_null: HandleOrNull) -> Result { + fn try_from(handle_or_null: HandleOrNull) -> Result { 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(NotHandle(())) + Err(NullHandleError(())) } else { Ok(owned_handle) } @@ -201,39 +201,56 @@ impl OwnedHandle { } impl TryFrom for OwnedHandle { - type Error = NotHandle; + type Error = InvalidHandleError; #[inline] - fn try_from(handle_or_invalid: HandleOrInvalid) -> Result { + fn try_from(handle_or_invalid: HandleOrInvalid) -> Result { 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(NotHandle(())) + Err(InvalidHandleError(())) } else { Ok(owned_handle) } } } -/// This is the error type used by [`HandleOrInvalid`] and -/// [`HandleOrNull`] when attempting to convert into a handle, -/// to indicate that the value is not a handle. +/// This is the error type used by [`HandleOrNull`] when attempting to convert +/// into a handle, to indicate that the value is null. #[unstable(feature = "io_safety", issue = "87074")] -#[derive(Debug, Copy, Clone, PartialEq, Eq)] -pub struct NotHandle(()); +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct NullHandleError(()); #[unstable(feature = "io_safety", issue = "87074")] -impl fmt::Display for NotHandle { +impl fmt::Display for NullHandleError { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - "the return value of a Windows API call indicated an error".fmt(fmt) + "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 NotHandle {} +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`. +#[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] From 890125d73e939cda058cf692c6a3ce927fc0898f Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Wed, 13 Apr 2022 14:32:17 -0700 Subject: [PATCH 05/11] Add a comment explaining the `(())` idiom for empty structs. --- library/std/src/os/windows/io/handle.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/std/src/os/windows/io/handle.rs b/library/std/src/os/windows/io/handle.rs index e48f630f76e7f..e4de52612ef62 100644 --- a/library/std/src/os/windows/io/handle.rs +++ b/library/std/src/os/windows/io/handle.rs @@ -220,6 +220,7 @@ impl TryFrom for OwnedHandle { /// 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(()); @@ -237,6 +238,7 @@ 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(()); From b7ff10378c6fe9eb57aa73e6a68dafcfc18da968 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 14 Apr 2022 21:54:13 -0700 Subject: [PATCH 06/11] Update the expected stderr for coerce-issue-49593-box-never. This test's expected stderr now includes a count of the number of types that implment `Error`. This PR introduces two new types, so increment the number by two. --- .../coercion/coerce-issue-49593-box-never.nofallback.stderr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/ui/coercion/coerce-issue-49593-box-never.nofallback.stderr b/src/test/ui/coercion/coerce-issue-49593-box-never.nofallback.stderr index 1b1ce67cb0c1f..99f00034eda60 100644 --- a/src/test/ui/coercion/coerce-issue-49593-box-never.nofallback.stderr +++ b/src/test/ui/coercion/coerce-issue-49593-box-never.nofallback.stderr @@ -13,7 +13,7 @@ LL | /* *mut $0 is coerced to Box here */ Box::<_ /* ! */>::new(x BorrowError BorrowMutError Box - and 43 others + 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 @@ -31,7 +31,7 @@ LL | /* *mut $0 is coerced to *mut Error here */ raw_ptr_box::<_ /* ! */>(x) BorrowError BorrowMutError Box - and 43 others + and 45 others = note: required for the cast to the object type `(dyn std::error::Error + 'static)` error: aborting due to 2 previous errors From 19ef182655b0ffbaadf859fc774d94bb6fd7e17e Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 18 Apr 2022 17:32:25 -0700 Subject: [PATCH 07/11] Update the expected stderr for coerce-issue-49593-box-never. --- .../coercion/coerce-issue-49593-box-never.nofallback.stderr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/ui/coercion/coerce-issue-49593-box-never.nofallback.stderr b/src/test/ui/coercion/coerce-issue-49593-box-never.nofallback.stderr index 99f00034eda60..1b1ce67cb0c1f 100644 --- a/src/test/ui/coercion/coerce-issue-49593-box-never.nofallback.stderr +++ b/src/test/ui/coercion/coerce-issue-49593-box-never.nofallback.stderr @@ -13,7 +13,7 @@ LL | /* *mut $0 is coerced to Box here */ Box::<_ /* ! */>::new(x BorrowError BorrowMutError Box - and 45 others + and 43 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 @@ -31,7 +31,7 @@ LL | /* *mut $0 is coerced to *mut Error here */ raw_ptr_box::<_ /* ! */>(x) BorrowError BorrowMutError Box - and 45 others + and 43 others = note: required for the cast to the object type `(dyn std::error::Error + 'static)` error: aborting due to 2 previous errors From ef9a68f2019ff72bc85ff1bb92956141273c476c Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Tue, 19 Apr 2022 14:19:50 -0700 Subject: [PATCH 08/11] Add ignore-windows to a test. This test has an expected stderr containing text like this: ``` help: the following other types implement trait `std::error::Error`: ... and 43 others ``` However, the number 43 is platform-specific; on Windows, there are two additional types, `InvalidHandleError` and `NullHandleError`, and the number of 45. So for now, disable this test on Windows. --- src/test/ui/coercion/coerce-issue-49593-box-never.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/ui/coercion/coerce-issue-49593-box-never.rs b/src/test/ui/coercion/coerce-issue-49593-box-never.rs index 7a4324bd5adce..16efb65acb2b6 100644 --- a/src/test/ui/coercion/coerce-issue-49593-box-never.rs +++ b/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 From 9d448e849d708cf1bee9065d6dfe70fc7977c3a8 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Tue, 19 Apr 2022 14:47:33 -0700 Subject: [PATCH 09/11] Update line numbers in the expected output. --- .../coercion/coerce-issue-49593-box-never.nofallback.stderr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/ui/coercion/coerce-issue-49593-box-never.nofallback.stderr b/src/test/ui/coercion/coerce-issue-49593-box-never.nofallback.stderr index 1b1ce67cb0c1f..1daa91f025a89 100644 --- a/src/test/ui/coercion/coerce-issue-49593-box-never.nofallback.stderr +++ b/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 here */ Box::<_ /* ! */>::new(x) | ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::error::Error` is not implemented for `()` @@ -17,7 +17,7 @@ LL | /* *mut $0 is coerced to Box 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 `()` From 839cd04cc44202784436bdd07fd4899f2d67d43a Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Tue, 26 Apr 2022 09:52:22 -0700 Subject: [PATCH 10/11] Add `only-windows` versions of the coerce-issue-49593-box-never test. --- ...-49593-box-never-windows.nofallback.stderr | 39 +++++++++++++ .../coerce-issue-49593-box-never-windows.rs | 58 +++++++++++++++++++ 2 files changed, 97 insertions(+) create mode 100644 src/test/ui/coercion/coerce-issue-49593-box-never-windows.nofallback.stderr create mode 100644 src/test/ui/coercion/coerce-issue-49593-box-never-windows.rs diff --git a/src/test/ui/coercion/coerce-issue-49593-box-never-windows.nofallback.stderr b/src/test/ui/coercion/coerce-issue-49593-box-never-windows.nofallback.stderr new file mode 100644 index 0000000000000..e40e96599fa0f --- /dev/null +++ b/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.rs:18:53 + | +LL | /* *mut $0 is coerced to Box 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 + BorrowError + BorrowMutError + Box + 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.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 + BorrowError + BorrowMutError + Box + 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`. diff --git a/src/test/ui/coercion/coerce-issue-49593-box-never-windows.rs b/src/test/ui/coercion/coerce-issue-49593-box-never-windows.rs new file mode 100644 index 0000000000000..95d3935caa9ef --- /dev/null +++ b/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) -> *mut T { + panic!() +} + +fn foo(x: !) -> Box { + /* *mut $0 is coerced to Box 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 */ = None; + let mut first_iter = false; + loop { + if !first_iter { + let y: Box + = /* Box<$0> is coerced to Box here */ Box::new(x.unwrap()); + } + + x = Some(S); + first_iter = true; + } + + let mut y : Option = None; + // assert types are equal + mem::swap(&mut x, &mut y); +} + +fn main() { +} From 531c9373be094ad4e3ee28464dfacbb2fdea50a5 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Tue, 26 Apr 2022 12:47:24 -0700 Subject: [PATCH 11/11] Fix the filename in the expected error message. --- .../coerce-issue-49593-box-never-windows.nofallback.stderr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/ui/coercion/coerce-issue-49593-box-never-windows.nofallback.stderr b/src/test/ui/coercion/coerce-issue-49593-box-never-windows.nofallback.stderr index e40e96599fa0f..ab9cc707aa0be 100644 --- a/src/test/ui/coercion/coerce-issue-49593-box-never-windows.nofallback.stderr +++ b/src/test/ui/coercion/coerce-issue-49593-box-never-windows.nofallback.stderr @@ -1,5 +1,5 @@ error[E0277]: the trait bound `(): std::error::Error` is not satisfied - --> $DIR/coerce-issue-49593-box-never.rs:18:53 + --> $DIR/coerce-issue-49593-box-never-windows.rs:18:53 | LL | /* *mut $0 is coerced to Box here */ Box::<_ /* ! */>::new(x) | ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::error::Error` is not implemented for `()` @@ -17,7 +17,7 @@ LL | /* *mut $0 is coerced to Box 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:23:49 + --> $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 `()`