PR feedback; default; Error::empty

crates/libs/core/src/imp/factory_cache.rs

@@ -172,7 +172,7 @@ mod tests {
             if unsafe { library.as_bytes() } == &b"A.dll"[..] {
                 Ok(42)
             } else {
-                Err(crate::Error::empty())
+                Err(crate::Error::fail())
             }
         });
         assert!(matches!(end_result, Some(42)));
@@ -182,7 +182,7 @@ mod tests {
         let mut results = Vec::new();
         let end_result = search_path(path, |library| {
             results.push(unsafe { library.to_string().unwrap() });
-            crate::Result::<()>::Err(crate::Error::empty())
+            crate::Result::<()>::Err(crate::Error::fail())
         });
         assert!(end_result.is_none());
         assert_eq!(results, vec!["A.B.dll", "A.dll"]);

crates/libs/core/src/type.rs

@@ -1,4 +1,5 @@
 use super::*;
+use crate::imp::E_POINTER;
 
 #[doc(hidden)]
 pub trait TypeKind {
@@ -35,12 +36,15 @@ where
         if !abi.is_null() {
             Ok(core::mem::transmute_copy(&abi))
         } else {
-            Err(Error::empty())
+            Err(Error::from_hresult(E_POINTER))
         }
     }
 
     fn from_default(default: &Self::Default) -> Result<Self> {
-        default.as_ref().cloned().ok_or(Error::empty())
+        default
+            .as_ref()
+            .cloned()
+            .ok_or(Error::from_hresult(E_POINTER))
     }
 }
 

crates/libs/result/src/error.rs

@@ -67,10 +67,7 @@ pub type ErrorInfo = NoDetail;
 pub struct NoDetail;
 
 /// Defines the behavior of error detail objects, which are stored within `Error`.
-pub trait ErrorDetail: Sized {
-    /// Returns an empty error detail.
-    fn empty() -> Self;
-
+pub trait ErrorDetail: Sized + Default {
     /// If this implementation stores error detail in ambient (thread-local) storage, then this
     /// function transfers the error detail from ambient storage and returns it.
     fn from_ambient() -> Self;
@@ -92,9 +89,6 @@ pub trait ErrorDetail: Sized {
 }
 
 impl ErrorDetail for NoDetail {
-    fn empty() -> Self {
-        Self
-    }
     fn from_ambient() -> Self {
         Self
     }
@@ -137,10 +131,6 @@ mod win_impl {
     }
 
     impl ErrorDetail for ErrorInfo {
-        fn empty() -> Self {
-            Self { ptr: None }
-        }
-
         fn from_ambient() -> Self {
             unsafe {
                 let mut ptr = None;
@@ -220,34 +210,25 @@ mod win_impl {
     }
 }
 
-impl<Detail: ErrorDetail> ErrorT<Detail> {
-    /// Creates an error object without any failure information.
-    pub fn empty() -> Self {
+impl<Detail: Default> ErrorT<Detail> {
+    /// Creates an error object without any specific failure information.
+    ///
+    /// The `code()` for this error is `E_FAIL`.
+    pub fn fail() -> Self {
         Self {
             code: NonZeroHRESULT::E_FAIL,
-            detail: Detail::empty(),
+            detail: Detail::default(),
         }
     }
 
-    /// Creates a new error object, capturing the stack and other information about the
-    /// point of failure.
-    ///
-    /// If `code` is `S_OK`, then this is remapped to `E_FAIL`. The `Error` object cannot
-    /// store an `S_OK` value.
-    pub fn new<T: AsRef<str>>(code: HRESULT, message: T) -> Self {
-        Detail::originate_error(code, message.as_ref());
-        // This into() call will take the ambient thread-local error object, if any.
-        Self::from(code)
-    }
-
     /// Creates a new error object with an error code, but without additional error information.
     ///
     /// If `code` is `S_OK`, then this is remapped to `E_FAIL`. The `Error` object cannot
     /// store an `S_OK` value.
     pub fn from_hresult(code: HRESULT) -> Self {
         Self {
             code: NonZeroHRESULT::from_hresult_or_fail(code),
-            detail: Detail::empty(),
+            detail: Detail::default(),
         }
     }
 
@@ -266,6 +247,19 @@ impl<Detail: ErrorDetail> ErrorT<Detail> {
             unimplemented!()
         }
     }
+}
+
+impl<Detail: ErrorDetail> ErrorT<Detail> {
+    /// Creates a new error object, capturing the stack and other information about the
+    /// point of failure.
+    ///
+    /// If `code` is `S_OK`, then this is remapped to `E_FAIL`. The `Error` object cannot
+    /// store an `S_OK` value.
+    pub fn new<T: AsRef<str>>(code: HRESULT, message: T) -> Self {
+        Detail::originate_error(code, message.as_ref());
+        // This into() call will take the ambient thread-local error object, if any.
+        Self::from(code)
+    }
 
     /// The error message describing the error.
     pub fn message(&self) -> String {
@@ -276,16 +270,14 @@ impl<Detail: ErrorDetail> ErrorT<Detail> {
         // Otherwise fallback to a generic error code description.
         self.code.message()
     }
+}
 
+impl<Detail> ErrorT<Detail> {
     /// Gets access to the error detail stored in this `Error`.
     pub fn detail(&self) -> &Detail {
         &self.detail
     }
-}
 
-// This is a separate impl block because Rust 1.60.0 (our MSRV) rejects const fns that have
-// trait bounds on them, so we place it in a separate impl without any bounds.
-impl<Detail> ErrorT<Detail> {
     /// The error code describing the error.
     pub const fn code(&self) -> HRESULT {
         self.code.to_hresult()
@@ -328,19 +320,19 @@ impl<Detail: ErrorDetail> From<std::io::Error> for ErrorT<Detail> {
     }
 }
 
-impl<Detail: ErrorDetail> From<alloc::string::FromUtf16Error> for ErrorT<Detail> {
+impl<Detail: Default> From<alloc::string::FromUtf16Error> for ErrorT<Detail> {
     fn from(_: alloc::string::FromUtf16Error) -> Self {
         Self::from_hresult(HRESULT::from_win32(ERROR_NO_UNICODE_TRANSLATION))
     }
 }
 
-impl<Detail: ErrorDetail> From<alloc::string::FromUtf8Error> for ErrorT<Detail> {
+impl<Detail: Default> From<alloc::string::FromUtf8Error> for ErrorT<Detail> {
     fn from(_: alloc::string::FromUtf8Error) -> Self {
         Self::from_hresult(HRESULT::from_win32(ERROR_NO_UNICODE_TRANSLATION))
     }
 }
 
-impl<Detail: ErrorDetail> From<core::num::TryFromIntError> for ErrorT<Detail> {
+impl<Detail: Default> From<core::num::TryFromIntError> for ErrorT<Detail> {
     fn from(_: core::num::TryFromIntError) -> Self {
         Self::from_hresult(HRESULT::from_win32(ERROR_INVALID_DATA))
     }

crates/tests/implement/tests/data_object.rs

@@ -58,7 +58,7 @@ impl IDataObject_Impl for Test_Impl {
     fn EnumFormatEtc(&self, _: u32) -> Result<IEnumFORMATETC> {
         unsafe {
             (*self.0.get()).EnumFormatEtc = true;
-            Err(Error::empty())
+            Err(Error::fail())
         }
     }
 
@@ -79,7 +79,7 @@ impl IDataObject_Impl for Test_Impl {
     fn EnumDAdvise(&self) -> Result<IEnumSTATDATA> {
         unsafe {
             (*self.0.get()).EnumDAdvise = true;
-            Err(Error::empty())
+            Err(Error::fail())
         }
     }
 }

crates/tests/implement/tests/vector.rs

@@ -184,7 +184,7 @@ fn GetAt() -> Result<()> {
     ])
     .into();
     assert_eq!(v.GetAt(0)?.ToString()?, "http://test/");
-    assert_eq!(v.GetAt(1).unwrap_err().code(), E_FAIL);
+    assert_eq!(v.GetAt(1).unwrap_err().code(), E_POINTER);
 
     Ok(())
 }

crates/tests/noexcept/tests/test.rs

@@ -42,7 +42,7 @@ impl ITest_Impl for Test_Impl {
     }
     fn Test(&self) -> Result<ITest> {
         let this = self.0.read().unwrap();
-        this.2.clone().ok_or_else(Error::empty)
+        this.2.clone().ok_or_else(Error::fail)
     }
     fn SetTest(&self, value: Option<&ITest>) -> Result<()> {
         let mut this = self.0.write().unwrap();

crates/tests/result/tests/error.rs

@@ -8,14 +8,6 @@ const E_CANCELLED: HRESULT = HRESULT::from_win32(ERROR_CANCELLED);
 
 windows_targets::link!("kernel32.dll" "system" fn SetLastError(code: u32));
 
-#[test]
-fn empty() {
-    let e: Error = Error::empty();
-    assert_eq!(e.code(), E_FAIL);
-    assert!(e.detail().as_ptr().is_null());
-    assert_eq!(e.message(), "Unspecified error");
-}
-
 #[test]
 fn new() {
     let e = Error::new(E_INVALIDARG, "");

crates/tests/winrt/tests/collisions.rs

@@ -16,7 +16,7 @@ fn wifi() -> windows::core::Result<()> {
     assert!(!a.is_empty());
 
     // from_id_async from IWiFiDirectDeviceStatics
-    assert!(WiFiDirectDevice::FromIdAsync(&a)?.get() == Err(windows::core::Error::empty()));
+    assert!(WiFiDirectDevice::FromIdAsync(&a)?.get() == Err(windows::core::Error::fail()));
 
     // get_device_selector overload from IWiFiDirectDeviceStatics2 is renamed to get_device_selector2
     let c = WiFiDirectDevice::GetDeviceSelector2(WiFiDirectDeviceSelectorType::DeviceInterface)?;