implement error multi provider

library/core/src/lib.rs

@@ -95,6 +95,7 @@
 //
 // Library features:
 // tidy-alphabetical-start
+#![feature(arbitrary_self_types_pointers)]
 #![feature(array_ptr_get)]
 #![feature(asm_experimental_arch)]
 #![feature(bigint_helper_methods)]

library/coretests/tests/error.rs

@@ -1,4 +1,5 @@
-use core::error::{Request, request_ref, request_value};
+use core::error::provide::{MultiRequestBuilder, MultiResponse, Request};
+use core::error::{Error, request_ref, request_value};
 
 // Test the `Request` API.
 #[derive(Debug)]
@@ -12,12 +13,23 @@ impl std::fmt::Display for SomeConcreteType {
     }
 }
 
+struct Invalid;
+#[derive(Debug, PartialEq, Eq)]
+struct Valid;
+
 impl std::error::Error for SomeConcreteType {
     fn provide<'a>(&'a self, request: &mut Request<'a>) {
         request
             .provide_ref::<String>(&self.some_string)
             .provide_ref::<str>(&self.some_string)
-            .provide_value_with::<String>(|| "bye".to_owned());
+            .provide_value_with::<String>(|| "bye".to_owned())
+            .provide_value_with::<Invalid>(|| panic!("should not be called"));
+        if request.would_be_satisfied_by_ref_of::<Invalid>() {
+            panic!("should not be satisfied");
+        }
+        if request.would_be_satisfied_by_ref_of::<Valid>() {
+            request.provide_ref(&Valid);
+        }
     }
 }
 
@@ -29,6 +41,7 @@ fn test_error_generic_member_access() {
     assert_eq!(request_ref::<String>(&*obj).unwrap(), "hello");
     assert_eq!(request_value::<String>(&*obj).unwrap(), "bye");
     assert_eq!(request_value::<u8>(&obj), None);
+    assert_eq!(request_ref::<Valid>(&obj), Some(&Valid));
 }
 
 // Test the Error.provide and request mechanisms with a by-reference trait object.
@@ -39,6 +52,7 @@ fn test_request_constructor() {
     assert_eq!(request_ref::<String>(&*obj).unwrap(), "hello");
     assert_eq!(request_value::<String>(&*obj).unwrap(), "bye");
     assert_eq!(request_value::<u8>(&obj), None);
+    assert_eq!(request_ref::<Valid>(&obj), Some(&Valid));
 }
 
 // Test the Error.provide and request mechanisms with a boxed trait object.
@@ -49,6 +63,7 @@ fn test_error_generic_member_access_boxed() {
 
     assert_eq!(request_ref::<String>(&*obj).unwrap(), "hello");
     assert_eq!(request_value::<String>(&*obj).unwrap(), "bye");
+    assert_eq!(request_ref::<Valid>(&*obj), Some(&Valid));
 
     // NOTE: Box<E> only implements Error when E: Error + Sized, which means we can't pass a
     // Box<dyn Error> to request_value.
@@ -63,4 +78,67 @@ fn test_error_generic_member_access_concrete() {
     assert_eq!(request_ref::<String>(&obj).unwrap(), "hello");
     assert_eq!(request_value::<String>(&obj).unwrap(), "bye");
     assert_eq!(request_value::<u8>(&obj), None);
+    assert_eq!(request_ref::<Valid>(&obj), Some(&Valid));
+}
+
+#[test]
+fn test_error_combined_access_concrete() {
+    let obj = SomeConcreteType { some_string: "hello".to_owned() };
+
+    let mut request = MultiRequestBuilder::new()
+        .with_value::<String>()
+        .with_ref::<String>()
+        .with_value::<u8>()
+        .with_ref::<Valid>()
+        .request(&obj);
+
+    assert_eq!(request.retrieve_ref::<String>().unwrap(), "hello");
+    assert_eq!(request.retrieve_value::<String>().unwrap(), "bye");
+    assert_eq!(request.retrieve_value::<u8>(), None);
+    assert_eq!(request.retrieve_ref::<Valid>().unwrap(), &Valid);
+
+    // second retrieve of same value returns none
+    assert_eq!(request.retrieve_ref::<Valid>(), None);
+    assert_eq!(request.retrieve_value::<String>(), None);
+    // retrieving an unknown type should return none
+    assert_eq!(request.retrieve_value::<*const ()>(), None);
+}
+
+#[test]
+fn test_error_combined_access_dyn() {
+    let obj = SomeConcreteType { some_string: "hello".to_owned() };
+    let obj: &dyn Error = &obj;
+
+    let mut request = MultiRequestBuilder::new()
+        .with_value::<String>()
+        .with_ref::<String>()
+        .with_value::<u8>()
+        .with_ref::<Valid>()
+        .request(&obj);
+
+    assert_eq!(request.retrieve_ref::<String>().unwrap(), "hello");
+    assert_eq!(request.retrieve_value::<String>().unwrap(), "bye");
+    assert_eq!(request.retrieve_value::<u8>(), None);
+    assert_eq!(request.retrieve_ref::<Valid>().unwrap(), &Valid);
+
+    // second retrieve of same value returns none
+    assert_eq!(request.retrieve_ref::<Valid>(), None);
+    assert_eq!(request.retrieve_value::<String>(), None);
+    // retrieving an unknown type should return none
+    assert_eq!(request.retrieve_value::<*const ()>(), None);
+}
+
+fn assert_copy<T: Copy>(_t: T) {}
+
+#[test]
+fn test_builder_copy_and_debug() {
+    // Check that MultiRequestBuilder implements Debug + Copy even if the contents doesn't (the exact contents don't matter)
+    // (Chain*MultiRequest don't, but their values are not really user-visible so it doesn't matter)
+    let builder = MultiRequestBuilder::new().with_value::<Valid>().with_ref::<Invalid>();
+    assert_copy(builder);
+    // check Debug
+    assert_eq!(
+        format!("{:?}", builder),
+        "MultiRequestBuilder(\"core::error::provide::ChainRefMultiRequestBuilder<coretests::error::Invalid, core::error::provide::ChainValMultiRequestBuilder<coretests::error::Valid, core::error::provide::EmptyMultiRequestBuilder>>\")"
+    );
 }

library/coretests/tests/lib.rs

@@ -45,6 +45,7 @@
 #![feature(duration_constants)]
 #![feature(duration_constructors)]
 #![feature(error_generic_member_access)]
+#![feature(error_generic_member_multi_access)]
 #![feature(exact_div)]
 #![feature(exact_size_is_empty)]
 #![feature(extend_one)]
@@ -175,6 +176,7 @@ mod clone;
 mod cmp;
 mod const_ptr;
 mod convert;
+mod error;
 mod ffi;
 mod floats;
 mod fmt;

src/tools/rust-analyzer/crates/ide-db/src/generated/lints.rs

@@ -5871,6 +5871,32 @@ The tracking issue for this feature is: [#99301]
 
 [#99301]: https://github.com/rust-lang/rust/issues/99301
 
+------------------------
+"##,
+        default_severity: Severity::Allow,
+        warn_since: None,
+        deny_since: None,
+    },
+    Lint {
+        label: "error_generic_member_multi_access",
+        description: r##"# `error_generic_member_multi_access`
+
+The tracking issue for this feature is: [#99301]
+
+[#99301]: https://github.com/rust-lang/rust/issues/99301
+
+------------------------
+"##,
+        default_severity: Severity::Allow,
+        warn_since: None,
+        deny_since: None,
+    },
+    Lint {
+        label: "error_generic_member_access_internals",
+        description: r##"# `error_generic_member_access_internals`
+
+This feature has no tracking issue, and is therefore likely internal to the compiler, not being intended for general use.
+
 ------------------------
 "##,
         default_severity: Severity::Allow,

tests/codegen-llvm/error-provide.rs

@@ -2,7 +2,11 @@
 
 //@ compile-flags: -Copt-level=3
 #![crate_type = "lib"]
-#![feature(error_generic_member_access)]
+#![feature(error_generic_member_access, error_generic_member_multi_access)]
+
+extern crate core;
+
+use core::error::provide::MultiResponse;
 use std::error::Request;
 use std::fmt;
 
@@ -30,21 +34,52 @@ impl fmt::Display for MyError {
 }
 
 impl std::error::Error for MyError {
-    // CHECK-LABEL: @provide
     #[no_mangle]
     fn provide<'a>(&'a self, request: &mut Request<'a>) {
-        // LLVM should be able to optimize multiple .provide_* calls into a switch table
-        // and eliminate redundant ones, rather than compare one-by-one.
-
-        // CHECK-NEXT: start:
-        // CHECK-NEXT: %[[SCRUTINEE:[^ ]+]] = load i128, ptr
-        // CHECK-NEXT: switch i128 %[[SCRUTINEE]], label %{{.*}} [
-        // CHECK-COUNT-3: i128 {{.*}}, label %{{.*}}
-        // CHECK-NEXT: ]
+        // In an ideal world, LLVM would be able to generate a jump table here.
+        // Currently it can not, mostly because it can't prove that the tag id
+        // is not modified. However, this shouldn't matter much since this
+        // API shouldn't be called many times - when requesting a large number
+        // of items, MultiRequestBuilder should be used.
+        //
+        // We could make a MIR optimization pass that flattens
+        // the reads of the tag id - this is sound, since it is mutably borrowed,
+        // but this has a fairly low cost/benefit ratio - `provide` should
+        // only be called O(1) times per error constructed, and it's already
+        // not much slower than constructing the error (and much faster
+        // if an allocation, backtrace or formatting is involved).
         request
             .provide_ref::<MyBacktrace1>(&self.backtrace1)
             .provide_ref::<MyBacktrace3>(&self.other)
             .provide_ref::<MyBacktrace2>(&self.backtrace2)
             .provide_ref::<MyBacktrace3>(&self.backtrace3);
     }
 }
+
+pub fn provide_multi(
+    e: &dyn std::error::Error,
+) -> (Option<&[u8; 0]>, Option<&[u8; 1]>, Option<&[u8; 2]>) {
+    let mut request = core::error::provide::MultiRequestBuilder::new()
+        .with_ref::<[u8; 0]>()
+        .with_ref::<[u8; 1]>()
+        .with_ref::<[u8; 2]>()
+        .with_ref::<[u8; 3]>()
+        .with_ref::<[u8; 4]>()
+        .with_ref::<[u8; 5]>()
+        .with_ref::<[u8; 6]>()
+        .with_ref::<[u8; 7]>()
+        .request(e);
+    (request.retrieve_ref(), request.retrieve_ref(), request.retrieve_ref())
+}
+
+// Check that the virtual function generated has a switch
+
+// CHECK: define {{.*}}4core5error7provide{{.*}}21ChainRefMultiResponse
+// CHECK-NEXT: start:
+// CHECK-NEXT: %[[SCRUTINEE:[^ ]+]] = load i128, ptr
+// CHECK-NEXT: switch i128 %[[SCRUTINEE]], label %{{.*}} [
+// The request we write has 8 arms. However, LLVM puts the "bottom-most" 3 arms in
+// a separate branch, before it produces a switch. This still leaves us with
+// O(log n) performance [LLVM generates a binary tree for ]
+// CHECK-COUNT-5: i128 {{.*}}, label %{{.*}}
+// CHECK-NEXT: ]

tests/ui/span/issue-71363.stderr

@@ -10,7 +10,7 @@ help: the trait `std::fmt::Display` is not implemented for `MyError`
 3 | struct MyError;
   | ^^^^^^^^^^^^^^
 note: required by a bound in `std::error::Error`
- --> $SRC_DIR/core/src/error.rs:LL:COL
+ --> $SRC_DIR/core/src/error/mod.rs:LL:COL
 
 error[E0277]: `MyError` doesn't implement `Debug`
  --> $DIR/issue-71363.rs:4:28
@@ -20,7 +20,7 @@ error[E0277]: `MyError` doesn't implement `Debug`
   |
   = note: add `#[derive(Debug)]` to `MyError` or manually `impl Debug for MyError`
 note: required by a bound in `std::error::Error`
- --> $SRC_DIR/core/src/error.rs:LL:COL
+ --> $SRC_DIR/core/src/error/mod.rs:LL:COL
 help: consider annotating `MyError` with `#[derive(Debug)]`
   |
 3 + #[derive(Debug)]