From 1b587a6e76200fdd8364ef910246efa11c973e7b Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Tue, 6 Aug 2024 14:33:14 -0700 Subject: [PATCH 1/4] alloc: add ToString specialization for `&&str` Fixes #128690 --- library/alloc/src/string.rs | 43 ++++++++++++++++---- tests/codegen/issues/str-to-string-128690.rs | 36 ++++++++++++++++ 2 files changed, 71 insertions(+), 8 deletions(-) create mode 100644 tests/codegen/issues/str-to-string-128690.rs diff --git a/library/alloc/src/string.rs b/library/alloc/src/string.rs index 124230812df56..d943901e9ecf9 100644 --- a/library/alloc/src/string.rs +++ b/library/alloc/src/string.rs @@ -2643,14 +2643,41 @@ impl ToString for i8 { } } -#[doc(hidden)] -#[cfg(not(no_global_oom_handling))] -#[stable(feature = "str_to_string_specialization", since = "1.9.0")] -impl ToString for str { - #[inline] - fn to_string(&self) -> String { - String::from(self) - } +// Generic/generated code can sometimes have multiple, nested references +// for strings, including `&&&str`s that would never be written +// by hand. This macro generates twelve layers of nested `&`-impl +// for primitive strings. +macro_rules! to_string_str { + {type ; x $($x:ident)*} => { + &to_string_str! { type ; $($x)* } + }; + {type ;} => { str }; + {impl ; x $($x:ident)*} => { + to_string_str! { $($x)* } + }; + {impl ;} => { }; + {$self:expr ; x $($x:ident)*} => { + *(to_string_str! { $self ; $($x)* }) + }; + {$self:expr ;} => { $self }; + {$($x:ident)*} => { + #[doc(hidden)] + #[cfg(not(no_global_oom_handling))] + #[stable(feature = "str_to_string_specialization", since = "1.9.0")] + impl ToString for to_string_str!(type ; $($x)*) { + #[inline] + fn to_string(&self) -> String { + String::from(to_string_str!(self ; $($x)*)) + } + } + to_string_str! { impl ; $($x)* } + }; +} + +to_string_str! { + x x x x + x x x x + x x x x } #[doc(hidden)] diff --git a/tests/codegen/issues/str-to-string-128690.rs b/tests/codegen/issues/str-to-string-128690.rs new file mode 100644 index 0000000000000..8b416306ba66e --- /dev/null +++ b/tests/codegen/issues/str-to-string-128690.rs @@ -0,0 +1,36 @@ +//@ compile-flags: -C opt-level=3 -Z merge-functions=disabled +#![crate_type = "lib"] + +//! Make sure str::to_string is specialized not to use fmt machinery. + +// CHECK-LABEL: define {{(dso_local )?}}void @one_ref +#[no_mangle] +pub fn one_ref(input: &str) -> String { + // CHECK-NOT: {{(call|invoke).*}}fmt + input.to_string() +} + +// CHECK-LABEL: define {{(dso_local )?}}void @two_ref +#[no_mangle] +pub fn two_ref(input: &&str) -> String { + // CHECK-NOT: {{(call|invoke).*}}fmt + input.to_string() +} + +// CHECK-LABEL: define {{(dso_local )?}}void @thirteen_ref +#[no_mangle] +pub fn thirteen_ref(input: &&&&&&&&&&&&&str) -> String { + // CHECK-NOT: {{(call|invoke).*}}fmt + input.to_string() +} + +// This is a known performance cliff because of the macro-generated +// specialized impl. If this test suddenly starts failing, +// consider removing the `to_string_str!` macro in `alloc/str/string.rs`. +// +// CHECK-LABEL: define {{(dso_local )?}}void @fourteen_ref +#[no_mangle] +pub fn fourteen_ref(input: &&&&&&&&&&&&&&str) -> String { + // CHECK: {{(call|invoke).*}}fmt + input.to_string() +} From 20c833c632d76ee78284441226f12b919318bc4b Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Tue, 6 Aug 2024 18:18:15 -0700 Subject: [PATCH 2/4] diagnostics: `Box` suggestion with multiple matching impl The two altered expectation messages both seem like improvements: - `coerce-expect-unsized-ascribed.stderr` says you can go `Box -> Box`, which you can. - `upcast_soundness_bug.stderr` used to say that you could go `Box> -> Box`, which you can't, because the type parameters are missing in the destination and the only ones that work aren't what's needed. --- .../error_reporting/infer/note_and_explain.rs | 24 +++++++------------ .../coerce-expect-unsized-ascribed.stderr | 4 ++++ tests/ui/traits/upcast_soundness_bug.stderr | 1 - 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_trait_selection/src/error_reporting/infer/note_and_explain.rs b/compiler/rustc_trait_selection/src/error_reporting/infer/note_and_explain.rs index 864510bb65047..0c140f93d94b2 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/infer/note_and_explain.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/infer/note_and_explain.rs @@ -313,11 +313,9 @@ impl Trait for X { (ty::Dynamic(t, _, ty::DynKind::Dyn), _) if let Some(def_id) = t.principal_def_id() => { - let mut impl_def_ids = vec![]; - tcx.for_each_relevant_impl(def_id, values.found, |did| { - impl_def_ids.push(did) - }); - if let [_] = &impl_def_ids[..] { + let has_non_blanket_impl = + tcx.non_blanket_impls_for_ty(def_id, values.found).next().is_some(); + if has_non_blanket_impl { let trait_name = tcx.item_name(def_id); diag.help(format!( "`{}` implements `{trait_name}` so you could box the found value \ @@ -330,11 +328,9 @@ impl Trait for X { (_, ty::Dynamic(t, _, ty::DynKind::Dyn)) if let Some(def_id) = t.principal_def_id() => { - let mut impl_def_ids = vec![]; - tcx.for_each_relevant_impl(def_id, values.expected, |did| { - impl_def_ids.push(did) - }); - if let [_] = &impl_def_ids[..] { + let has_non_blanket_impl = + tcx.non_blanket_impls_for_ty(def_id, values.expected).next().is_some(); + if has_non_blanket_impl { let trait_name = tcx.item_name(def_id); diag.help(format!( "`{}` implements `{trait_name}` so you could change the expected \ @@ -346,11 +342,9 @@ impl Trait for X { (ty::Dynamic(t, _, ty::DynKind::DynStar), _) if let Some(def_id) = t.principal_def_id() => { - let mut impl_def_ids = vec![]; - tcx.for_each_relevant_impl(def_id, values.found, |did| { - impl_def_ids.push(did) - }); - if let [_] = &impl_def_ids[..] { + let has_non_blanket_impl = + tcx.non_blanket_impls_for_ty(def_id, values.found).next().is_some(); + if has_non_blanket_impl { let trait_name = tcx.item_name(def_id); diag.help(format!( "`{}` implements `{trait_name}`, `#[feature(dyn_star)]` is likely \ diff --git a/tests/ui/coercion/coerce-expect-unsized-ascribed.stderr b/tests/ui/coercion/coerce-expect-unsized-ascribed.stderr index 646044ae41abc..0c220a13876b8 100644 --- a/tests/ui/coercion/coerce-expect-unsized-ascribed.stderr +++ b/tests/ui/coercion/coerce-expect-unsized-ascribed.stderr @@ -42,6 +42,7 @@ LL | let _ = type_ascribe!(Box::new( if true { false } else { true }), Box` found struct `Box` + = help: `bool` implements `Debug` so you could box the found value and coerce it to the trait object `Box`, you will have to change the expected type as well error[E0308]: mismatched types --> $DIR/coerce-expect-unsized-ascribed.rs:16:27 @@ -51,6 +52,7 @@ LL | let _ = type_ascribe!(Box::new( match true { true => 'a', false => 'b' | = note: expected struct `Box` found struct `Box` + = help: `char` implements `Debug` so you could box the found value and coerce it to the trait object `Box`, you will have to change the expected type as well error[E0308]: mismatched types --> $DIR/coerce-expect-unsized-ascribed.rs:18:27 @@ -96,6 +98,7 @@ LL | let _ = type_ascribe!(&if true { false } else { true }, &dyn Debug); | = note: expected reference `&dyn Debug` found reference `&bool` + = help: `bool` implements `Debug` so you could box the found value and coerce it to the trait object `Box`, you will have to change the expected type as well error[E0308]: mismatched types --> $DIR/coerce-expect-unsized-ascribed.rs:24:27 @@ -105,6 +108,7 @@ LL | let _ = type_ascribe!(&match true { true => 'a', false => 'b' }, &dyn D | = note: expected reference `&dyn Debug` found reference `&char` + = help: `char` implements `Debug` so you could box the found value and coerce it to the trait object `Box`, you will have to change the expected type as well error[E0308]: mismatched types --> $DIR/coerce-expect-unsized-ascribed.rs:26:27 diff --git a/tests/ui/traits/upcast_soundness_bug.stderr b/tests/ui/traits/upcast_soundness_bug.stderr index 5864abcdb41f5..31f77b52b5fbf 100644 --- a/tests/ui/traits/upcast_soundness_bug.stderr +++ b/tests/ui/traits/upcast_soundness_bug.stderr @@ -6,7 +6,6 @@ LL | let p = p as *const dyn Trait; // <- this is bad! | = note: expected trait object `dyn Trait` found trait object `dyn Trait` - = help: `dyn Trait` implements `Trait` so you could box the found value and coerce it to the trait object `Box`, you will have to change the expected type as well error: aborting due to 1 previous error From 3312f5d65244b4ccb035be7b4c61541afc211914 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Wed, 7 Aug 2024 09:06:49 -0700 Subject: [PATCH 3/4] alloc: make `to_string_str!` a bit less complex --- library/alloc/src/string.rs | 57 +++++++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 22 deletions(-) diff --git a/library/alloc/src/string.rs b/library/alloc/src/string.rs index d943901e9ecf9..e628be1546f76 100644 --- a/library/alloc/src/string.rs +++ b/library/alloc/src/string.rs @@ -2647,37 +2647,50 @@ impl ToString for i8 { // for strings, including `&&&str`s that would never be written // by hand. This macro generates twelve layers of nested `&`-impl // for primitive strings. -macro_rules! to_string_str { - {type ; x $($x:ident)*} => { - &to_string_str! { type ; $($x)* } - }; - {type ;} => { str }; - {impl ; x $($x:ident)*} => { - to_string_str! { $($x)* } +#[cfg(not(no_global_oom_handling))] +macro_rules! to_string_str_wrap_in_ref { + {x $($x:ident)*} => { + &to_string_str_wrap_in_ref! { $($x)* } }; - {impl ;} => { }; + {} => { str }; +} +#[cfg(not(no_global_oom_handling))] +macro_rules! to_string_expr_wrap_in_deref { {$self:expr ; x $($x:ident)*} => { - *(to_string_str! { $self ; $($x)* }) + *(to_string_expr_wrap_in_deref! { $self ; $($x)* }) }; {$self:expr ;} => { $self }; - {$($x:ident)*} => { - #[doc(hidden)] - #[cfg(not(no_global_oom_handling))] - #[stable(feature = "str_to_string_specialization", since = "1.9.0")] - impl ToString for to_string_str!(type ; $($x)*) { - #[inline] - fn to_string(&self) -> String { - String::from(to_string_str!(self ; $($x)*)) +} +#[cfg(not(no_global_oom_handling))] +macro_rules! to_string_str { + {$($($x:ident)*),+} => { + $( + #[doc(hidden)] + #[stable(feature = "str_to_string_specialization", since = "1.9.0")] + impl ToString for to_string_str_wrap_in_ref!($($x)*) { + #[inline] + fn to_string(&self) -> String { + String::from(to_string_expr_wrap_in_deref!(self ; $($x)*)) + } } - } - to_string_str! { impl ; $($x)* } + )+ }; } +#[cfg(not(no_global_oom_handling))] to_string_str! { - x x x x - x x x x - x x x x + x x x x x x x x x x x x, + x x x x x x x x x x x, + x x x x x x x x x x, + x x x x x x x x x, + x x x x x x x x, + x x x x x x x, + x x x x x x, + x x x x x, + x x x x, + x x x, + x x, + x, } #[doc(hidden)] From c6fb0f344e0c6edb34966b1d73c3c1d8c5afbe34 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Tue, 13 Aug 2024 09:33:12 -0700 Subject: [PATCH 4/4] diagnostics: use `DeepRejectCtxt` for check This makes more things match, particularly applicable blankets. --- .../error_reporting/infer/note_and_explain.rs | 37 ++++++++++++++----- .../ptr-to-trait-obj-different-args.stderr | 3 ++ tests/ui/traits/upcast_soundness_bug.stderr | 1 + 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_trait_selection/src/error_reporting/infer/note_and_explain.rs b/compiler/rustc_trait_selection/src/error_reporting/infer/note_and_explain.rs index 0c140f93d94b2..eab4addf97072 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/infer/note_and_explain.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/infer/note_and_explain.rs @@ -4,6 +4,7 @@ use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_middle::traits::{ObligationCause, ObligationCauseCode}; use rustc_middle::ty::error::{ExpectedFound, TypeError}; +use rustc_middle::ty::fast_reject::{DeepRejectCtxt, TreatParams}; use rustc_middle::ty::print::{FmtPrinter, Printer}; use rustc_middle::ty::{self, suggest_constraining_type_param, Ty}; use rustc_span::def_id::DefId; @@ -313,9 +314,15 @@ impl Trait for X { (ty::Dynamic(t, _, ty::DynKind::Dyn), _) if let Some(def_id) = t.principal_def_id() => { - let has_non_blanket_impl = - tcx.non_blanket_impls_for_ty(def_id, values.found).next().is_some(); - if has_non_blanket_impl { + let mut has_matching_impl = false; + tcx.for_each_relevant_impl(def_id, values.found, |did| { + if DeepRejectCtxt::new(tcx, TreatParams::ForLookup) + .types_may_unify(values.found, tcx.type_of(did).skip_binder()) + { + has_matching_impl = true; + } + }); + if has_matching_impl { let trait_name = tcx.item_name(def_id); diag.help(format!( "`{}` implements `{trait_name}` so you could box the found value \ @@ -328,9 +335,15 @@ impl Trait for X { (_, ty::Dynamic(t, _, ty::DynKind::Dyn)) if let Some(def_id) = t.principal_def_id() => { - let has_non_blanket_impl = - tcx.non_blanket_impls_for_ty(def_id, values.expected).next().is_some(); - if has_non_blanket_impl { + let mut has_matching_impl = false; + tcx.for_each_relevant_impl(def_id, values.expected, |did| { + if DeepRejectCtxt::new(tcx, TreatParams::ForLookup) + .types_may_unify(values.expected, tcx.type_of(did).skip_binder()) + { + has_matching_impl = true; + } + }); + if has_matching_impl { let trait_name = tcx.item_name(def_id); diag.help(format!( "`{}` implements `{trait_name}` so you could change the expected \ @@ -342,9 +355,15 @@ impl Trait for X { (ty::Dynamic(t, _, ty::DynKind::DynStar), _) if let Some(def_id) = t.principal_def_id() => { - let has_non_blanket_impl = - tcx.non_blanket_impls_for_ty(def_id, values.found).next().is_some(); - if has_non_blanket_impl { + let mut has_matching_impl = false; + tcx.for_each_relevant_impl(def_id, values.found, |did| { + if DeepRejectCtxt::new(tcx, TreatParams::ForLookup) + .types_may_unify(values.found, tcx.type_of(did).skip_binder()) + { + has_matching_impl = true; + } + }); + if has_matching_impl { let trait_name = tcx.item_name(def_id); diag.help(format!( "`{}` implements `{trait_name}`, `#[feature(dyn_star)]` is likely \ diff --git a/tests/ui/cast/ptr-to-trait-obj-different-args.stderr b/tests/ui/cast/ptr-to-trait-obj-different-args.stderr index b04289ae74748..8e60ca42f0a52 100644 --- a/tests/ui/cast/ptr-to-trait-obj-different-args.stderr +++ b/tests/ui/cast/ptr-to-trait-obj-different-args.stderr @@ -14,6 +14,7 @@ LL | let y: *const dyn Trait = x as _; | = note: expected trait object `dyn Trait` found trait object `dyn Trait` + = help: `dyn Trait` implements `Trait` so you could box the found value and coerce it to the trait object `Box`, you will have to change the expected type as well error[E0308]: mismatched types --> $DIR/ptr-to-trait-obj-different-args.rs:27:34 @@ -25,6 +26,7 @@ LL | let _: *const dyn Trait = x as _; | = note: expected trait object `dyn Trait` found trait object `dyn Trait` + = help: `dyn Trait` implements `Trait` so you could box the found value and coerce it to the trait object `Box`, you will have to change the expected type as well error[E0308]: mismatched types --> $DIR/ptr-to-trait-obj-different-args.rs:28:34 @@ -37,6 +39,7 @@ LL | let _: *const dyn Trait = t as _; | = note: expected trait object `dyn Trait` found trait object `dyn Trait` + = help: `dyn Trait` implements `Trait` so you could box the found value and coerce it to the trait object `Box`, you will have to change the expected type as well error[E0308]: mismatched types --> $DIR/ptr-to-trait-obj-different-args.rs:36:5 diff --git a/tests/ui/traits/upcast_soundness_bug.stderr b/tests/ui/traits/upcast_soundness_bug.stderr index 31f77b52b5fbf..5864abcdb41f5 100644 --- a/tests/ui/traits/upcast_soundness_bug.stderr +++ b/tests/ui/traits/upcast_soundness_bug.stderr @@ -6,6 +6,7 @@ LL | let p = p as *const dyn Trait; // <- this is bad! | = note: expected trait object `dyn Trait` found trait object `dyn Trait` + = help: `dyn Trait` implements `Trait` so you could box the found value and coerce it to the trait object `Box`, you will have to change the expected type as well error: aborting due to 1 previous error