From b805f305425d06337cc1b84653c2bfe7b831a772 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 31 Dec 2023 18:35:49 +0000 Subject: [PATCH 1/3] Remove support for specializing ToString outside the standard library This is the only trait specializable outside of the standard library. Before stabilizing specialization we will probably want to remove support for this. It was originally made specializable to allow a more efficient ToString in libproc_macro back when this way the only way to get any data out of a TokenStream. We now support getting individual tokens, so proc macros no longer need to call it as often. --- compiler/rustc_span/src/lib.rs | 1 - compiler/rustc_span/src/symbol.rs | 7 --- library/alloc/src/string.rs | 69 ++++++++++------------ library/proc_macro/src/bridge/symbol.rs | 6 -- library/proc_macro/src/lib.rs | 77 ++++--------------------- 5 files changed, 43 insertions(+), 117 deletions(-) diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index bd8b93bb4d1e7..6e25de847fc88 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -25,7 +25,6 @@ #![feature(hash_set_entry)] #![feature(if_let_guard)] #![feature(let_chains)] -#![feature(min_specialization)] #![feature(negative_impls)] #![feature(read_buf)] #![feature(round_char_boundary)] diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index d30b17c9cd8dd..4a4e8bfd17f31 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -2471,13 +2471,6 @@ impl fmt::Display for Symbol { } } -// takes advantage of `str::to_string` specialization -impl ToString for Symbol { - fn to_string(&self) -> String { - self.as_str().to_string() - } -} - impl HashStable for Symbol { #[inline] fn hash_stable(&self, hcx: &mut CTX, hasher: &mut StableHasher) { diff --git a/library/alloc/src/string.rs b/library/alloc/src/string.rs index c5378d78d591b..23d060d2158cd 100644 --- a/library/alloc/src/string.rs +++ b/library/alloc/src/string.rs @@ -2675,12 +2675,25 @@ pub trait ToString { #[cfg(not(no_global_oom_handling))] #[stable(feature = "rust1", since = "1.0.0")] impl ToString for T { + #[inline] + fn to_string(&self) -> String { + ::spec_to_string(self) + } +} + +#[cfg(not(no_global_oom_handling))] +trait SpecToString { + fn spec_to_string(&self) -> String; +} + +#[cfg(not(no_global_oom_handling))] +impl SpecToString for T { // A common guideline is to not inline generic functions. However, // removing `#[inline]` from this method causes non-negligible regressions. // See , the last attempt // to try to remove it. #[inline] - default fn to_string(&self) -> String { + default fn spec_to_string(&self) -> String { let mut buf = String::new(); let mut formatter = core::fmt::Formatter::new(&mut buf, core::fmt::FormattingOptions::new()); @@ -2691,42 +2704,34 @@ impl ToString for T { } } -#[doc(hidden)] #[cfg(not(no_global_oom_handling))] -#[unstable(feature = "ascii_char", issue = "110998")] -impl ToString for core::ascii::Char { +impl SpecToString for core::ascii::Char { #[inline] - fn to_string(&self) -> String { + fn spec_to_string(&self) -> String { self.as_str().to_owned() } } -#[doc(hidden)] #[cfg(not(no_global_oom_handling))] -#[stable(feature = "char_to_string_specialization", since = "1.46.0")] -impl ToString for char { +impl SpecToString for char { #[inline] - fn to_string(&self) -> String { + fn spec_to_string(&self) -> String { String::from(self.encode_utf8(&mut [0; 4])) } } -#[doc(hidden)] #[cfg(not(no_global_oom_handling))] -#[stable(feature = "bool_to_string_specialization", since = "1.68.0")] -impl ToString for bool { +impl SpecToString for bool { #[inline] - fn to_string(&self) -> String { + fn spec_to_string(&self) -> String { String::from(if *self { "true" } else { "false" }) } } -#[doc(hidden)] #[cfg(not(no_global_oom_handling))] -#[stable(feature = "u8_to_string_specialization", since = "1.54.0")] -impl ToString for u8 { +impl SpecToString for u8 { #[inline] - fn to_string(&self) -> String { + fn spec_to_string(&self) -> String { let mut buf = String::with_capacity(3); let mut n = *self; if n >= 10 { @@ -2742,12 +2747,10 @@ impl ToString for u8 { } } -#[doc(hidden)] #[cfg(not(no_global_oom_handling))] -#[stable(feature = "i8_to_string_specialization", since = "1.54.0")] -impl ToString for i8 { +impl SpecToString for i8 { #[inline] - fn to_string(&self) -> String { + fn spec_to_string(&self) -> String { let mut buf = String::with_capacity(4); if self.is_negative() { buf.push('-'); @@ -2788,11 +2791,9 @@ macro_rules! to_string_expr_wrap_in_deref { 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)*) { + impl SpecToString for to_string_str_wrap_in_ref!($($x)*) { #[inline] - fn to_string(&self) -> String { + fn spec_to_string(&self) -> String { String::from(to_string_expr_wrap_in_deref!(self ; $($x)*)) } } @@ -2816,32 +2817,26 @@ to_string_str! { x, } -#[doc(hidden)] #[cfg(not(no_global_oom_handling))] -#[stable(feature = "cow_str_to_string_specialization", since = "1.17.0")] -impl ToString for Cow<'_, str> { +impl SpecToString for Cow<'_, str> { #[inline] - fn to_string(&self) -> String { + fn spec_to_string(&self) -> String { self[..].to_owned() } } -#[doc(hidden)] #[cfg(not(no_global_oom_handling))] -#[stable(feature = "string_to_string_specialization", since = "1.17.0")] -impl ToString for String { +impl SpecToString for String { #[inline] - fn to_string(&self) -> String { + fn spec_to_string(&self) -> String { self.to_owned() } } -#[doc(hidden)] #[cfg(not(no_global_oom_handling))] -#[stable(feature = "fmt_arguments_to_string_specialization", since = "1.71.0")] -impl ToString for fmt::Arguments<'_> { +impl SpecToString for fmt::Arguments<'_> { #[inline] - fn to_string(&self) -> String { + fn spec_to_string(&self) -> String { crate::fmt::format(*self) } } diff --git a/library/proc_macro/src/bridge/symbol.rs b/library/proc_macro/src/bridge/symbol.rs index edad6e7ac393f..6a1cecd69fb5f 100644 --- a/library/proc_macro/src/bridge/symbol.rs +++ b/library/proc_macro/src/bridge/symbol.rs @@ -91,12 +91,6 @@ impl fmt::Debug for Symbol { } } -impl ToString for Symbol { - fn to_string(&self) -> String { - self.with(|s| s.to_owned()) - } -} - impl fmt::Display for Symbol { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { self.with(|s| fmt::Display::fmt(s, f)) diff --git a/library/proc_macro/src/lib.rs b/library/proc_macro/src/lib.rs index 4aa47ce4e4f51..26a09f0daca02 100644 --- a/library/proc_macro/src/lib.rs +++ b/library/proc_macro/src/lib.rs @@ -19,9 +19,6 @@ )] #![doc(rust_logo)] #![feature(rustdoc_internals)] -// This library is copied into rust-analyzer to allow loading rustc compiled proc macros. -// Please avoid unstable features where possible to minimize the amount of changes necessary -// to make it compile with rust-analyzer on stable. #![feature(staged_api)] #![feature(allow_internal_unstable)] #![feature(decl_macro)] @@ -30,7 +27,6 @@ #![feature(panic_can_unwind)] #![feature(restricted_std)] #![feature(rustc_attrs)] -#![feature(min_specialization)] #![feature(extend_one)] #![recursion_limit = "256"] #![allow(internal_features)] @@ -185,16 +181,6 @@ impl FromStr for TokenStream { } } -// N.B., the bridge only provides `to_string`, implement `fmt::Display` -// based on it (the reverse of the usual relationship between the two). -#[doc(hidden)] -#[stable(feature = "proc_macro_lib", since = "1.15.0")] -impl ToString for TokenStream { - fn to_string(&self) -> String { - self.0.as_ref().map(|t| t.to_string()).unwrap_or_default() - } -} - /// Prints the token stream as a string that is supposed to be losslessly convertible back /// into the same token stream (modulo spans), except for possibly `TokenTree::Group`s /// with `Delimiter::None` delimiters and negative numeric literals. @@ -210,7 +196,10 @@ impl ToString for TokenStream { impl fmt::Display for TokenStream { #[allow(clippy::recursive_format_impl)] // clippy doesn't see the specialization fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str(&self.to_string()) + match &self.0 { + Some(ts) => write!(f, "{}", ts.to_string()), + None => Ok(()), + } } } @@ -756,21 +745,6 @@ impl From for TokenTree { } } -// N.B., the bridge only provides `to_string`, implement `fmt::Display` -// based on it (the reverse of the usual relationship between the two). -#[doc(hidden)] -#[stable(feature = "proc_macro_lib", since = "1.15.0")] -impl ToString for TokenTree { - fn to_string(&self) -> String { - match *self { - TokenTree::Group(ref t) => t.to_string(), - TokenTree::Ident(ref t) => t.to_string(), - TokenTree::Punct(ref t) => t.to_string(), - TokenTree::Literal(ref t) => t.to_string(), - } - } -} - /// Prints the token tree as a string that is supposed to be losslessly convertible back /// into the same token tree (modulo spans), except for possibly `TokenTree::Group`s /// with `Delimiter::None` delimiters and negative numeric literals. @@ -786,7 +760,12 @@ impl ToString for TokenTree { impl fmt::Display for TokenTree { #[allow(clippy::recursive_format_impl)] // clippy doesn't see the specialization fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str(&self.to_string()) + match self { + TokenTree::Group(t) => write!(f, "{t}"), + TokenTree::Ident(t) => write!(f, "{t}"), + TokenTree::Punct(t) => write!(f, "{t}"), + TokenTree::Literal(t) => write!(f, "{t}"), + } } } @@ -912,16 +891,6 @@ impl Group { } } -// N.B., the bridge only provides `to_string`, implement `fmt::Display` -// based on it (the reverse of the usual relationship between the two). -#[doc(hidden)] -#[stable(feature = "proc_macro_lib", since = "1.15.0")] -impl ToString for Group { - fn to_string(&self) -> String { - TokenStream::from(TokenTree::from(self.clone())).to_string() - } -} - /// Prints the group as a string that should be losslessly convertible back /// into the same group (modulo spans), except for possibly `TokenTree::Group`s /// with `Delimiter::None` delimiters. @@ -929,7 +898,7 @@ impl ToString for Group { impl fmt::Display for Group { #[allow(clippy::recursive_format_impl)] // clippy doesn't see the specialization fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str(&self.to_string()) + write!(f, "{}", TokenStream::from(TokenTree::from(self.clone()))) } } @@ -1035,14 +1004,6 @@ impl Punct { } } -#[doc(hidden)] -#[stable(feature = "proc_macro_lib2", since = "1.29.0")] -impl ToString for Punct { - fn to_string(&self) -> String { - self.as_char().to_string() - } -} - /// Prints the punctuation character as a string that should be losslessly convertible /// back into the same character. #[stable(feature = "proc_macro_lib2", since = "1.29.0")] @@ -1138,14 +1099,6 @@ impl Ident { } } -#[doc(hidden)] -#[stable(feature = "proc_macro_lib2", since = "1.29.0")] -impl ToString for Ident { - fn to_string(&self) -> String { - self.0.sym.with(|sym| if self.0.is_raw { ["r#", sym].concat() } else { sym.to_owned() }) - } -} - /// Prints the identifier as a string that should be losslessly convertible back /// into the same identifier. #[stable(feature = "proc_macro_lib2", since = "1.29.0")] @@ -1520,14 +1473,6 @@ impl FromStr for Literal { } } -#[doc(hidden)] -#[stable(feature = "proc_macro_lib2", since = "1.29.0")] -impl ToString for Literal { - fn to_string(&self) -> String { - self.with_stringify_parts(|parts| parts.concat()) - } -} - /// Prints the literal as a string that should be losslessly convertible /// back into the same literal (except for possible rounding for floating point literals). #[stable(feature = "proc_macro_lib2", since = "1.29.0")] From f7b14035a4a6d8d6d2ba31c022f25964122beb0c Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 13 Dec 2024 14:43:17 +0000 Subject: [PATCH 2/3] Update test --- .../specialization/min_specialization/issue-79224.rs | 10 ++++++++++ .../min_specialization/issue-79224.stderr | 8 ++++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/tests/ui/specialization/min_specialization/issue-79224.rs b/tests/ui/specialization/min_specialization/issue-79224.rs index 6ddd3d79ccf75..15429bf60e5ab 100644 --- a/tests/ui/specialization/min_specialization/issue-79224.rs +++ b/tests/ui/specialization/min_specialization/issue-79224.rs @@ -1,6 +1,16 @@ #![feature(min_specialization)] use std::fmt::{self, Display}; +pub trait ToString { + fn to_string(&self) -> String; +} + +impl ToString for T { + default fn to_string(&self) -> String { + todo!() + } +} + pub enum Cow<'a, B: ?Sized + 'a, O = ::Owned> where B: ToOwned, diff --git a/tests/ui/specialization/min_specialization/issue-79224.stderr b/tests/ui/specialization/min_specialization/issue-79224.stderr index 84e526f459785..ffa7462da16f6 100644 --- a/tests/ui/specialization/min_specialization/issue-79224.stderr +++ b/tests/ui/specialization/min_specialization/issue-79224.stderr @@ -1,5 +1,5 @@ error[E0277]: the trait bound `B: Clone` is not satisfied - --> $DIR/issue-79224.rs:18:29 + --> $DIR/issue-79224.rs:28:29 | LL | impl Display for Cow<'_, B> { | ^^^^^^^^^^ the trait `Clone` is not implemented for `B` @@ -11,7 +11,7 @@ LL | impl Display for Cow<'_, B> { | +++++++++++++++++++ error[E0277]: the trait bound `B: Clone` is not satisfied - --> $DIR/issue-79224.rs:20:5 + --> $DIR/issue-79224.rs:30:5 | LL | fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Clone` is not implemented for `B` @@ -23,7 +23,7 @@ LL | impl Display for Cow<'_, B> { | +++++++++++++++++++ error[E0277]: the trait bound `B: Clone` is not satisfied - --> $DIR/issue-79224.rs:20:13 + --> $DIR/issue-79224.rs:30:13 | LL | fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ^^^^ the trait `Clone` is not implemented for `B` @@ -35,7 +35,7 @@ LL | impl Display for Cow<'_, B> { | +++++++++++++++++++ error[E0277]: the trait bound `B: Clone` is not satisfied - --> $DIR/issue-79224.rs:20:62 + --> $DIR/issue-79224.rs:30:62 | LL | fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ______________________________________________________________^ From 3f3f27bb84b46be8294e27c6a2d64f0c918f3679 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 14 Dec 2024 14:53:11 +0000 Subject: [PATCH 3/3] Stop handling specialization in clippy's to_string_trait_impl lint ToString can no longer be specialized, so no need to account for it in to_string_trait_impl either. --- .../clippy_lints/src/to_string_trait_impl.rs | 3 -- .../clippy/tests/ui/to_string_trait_impl.rs | 43 ------------------- .../tests/ui/to_string_trait_impl.stderr | 14 +----- 3 files changed, 1 insertion(+), 59 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/to_string_trait_impl.rs b/src/tools/clippy/clippy_lints/src/to_string_trait_impl.rs index 0361836cdec79..9596b85664b34 100644 --- a/src/tools/clippy/clippy_lints/src/to_string_trait_impl.rs +++ b/src/tools/clippy/clippy_lints/src/to_string_trait_impl.rs @@ -1,5 +1,4 @@ use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::ty::implements_trait; use rustc_hir::{Impl, Item, ItemKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::declare_lint_pass; @@ -54,8 +53,6 @@ impl<'tcx> LateLintPass<'tcx> for ToStringTraitImpl { }) = it.kind && let Some(trait_did) = trait_ref.trait_def_id() && cx.tcx.is_diagnostic_item(sym::ToString, trait_did) - && let Some(display_did) = cx.tcx.get_diagnostic_item(sym::Display) - && !implements_trait(cx, cx.tcx.type_of(it.owner_id).instantiate_identity(), display_did, &[]) { span_lint_and_help( cx, diff --git a/src/tools/clippy/tests/ui/to_string_trait_impl.rs b/src/tools/clippy/tests/ui/to_string_trait_impl.rs index 4c1202d420341..7be9f7994f08a 100644 --- a/src/tools/clippy/tests/ui/to_string_trait_impl.rs +++ b/src/tools/clippy/tests/ui/to_string_trait_impl.rs @@ -30,46 +30,3 @@ impl Bar { String::from("Bar") } } - -mod issue12263 { - pub struct MyStringWrapper<'a>(&'a str); - - impl std::fmt::Display for MyStringWrapper<'_> { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - self.0.fmt(f) - } - } - - impl ToString for MyStringWrapper<'_> { - fn to_string(&self) -> String { - self.0.to_string() - } - } - - pub struct S(T); - impl std::fmt::Display for S { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - todo!() - } - } - // no specialization if the generics differ, so lint - impl ToString for S { - fn to_string(&self) -> String { - todo!() - } - } - - pub struct S2(T); - impl std::fmt::Display for S2 { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - todo!() - } - } - - // also specialization if the generics don't differ - impl ToString for S2 { - fn to_string(&self) -> String { - todo!() - } - } -} diff --git a/src/tools/clippy/tests/ui/to_string_trait_impl.stderr b/src/tools/clippy/tests/ui/to_string_trait_impl.stderr index 304c9a5e1fb71..fe8afc215f0c8 100644 --- a/src/tools/clippy/tests/ui/to_string_trait_impl.stderr +++ b/src/tools/clippy/tests/ui/to_string_trait_impl.stderr @@ -12,17 +12,5 @@ LL | | } = note: `-D clippy::to-string-trait-impl` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::to_string_trait_impl)]` -error: direct implementation of `ToString` - --> tests/ui/to_string_trait_impl.rs:56:5 - | -LL | / impl ToString for S { -LL | | fn to_string(&self) -> String { -LL | | todo!() -LL | | } -LL | | } - | |_____^ - | - = help: prefer implementing `Display` instead - -error: aborting due to 2 previous errors +error: aborting due to 1 previous error