From d4ed723d0b8eb5fbf9305b0e77b64dc07c734ca2 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Mon, 24 Apr 2023 18:19:15 +0200 Subject: [PATCH 1/3] Optimize formatting str/String without options. --- compiler/rustc_ast_lowering/src/format.rs | 86 +++++++++++++---------- compiler/rustc_span/src/symbol.rs | 1 + library/alloc/src/lib.rs | 1 + library/alloc/src/string.rs | 4 ++ library/core/src/fmt/mod.rs | 49 ++++++++++++- 5 files changed, 104 insertions(+), 37 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/format.rs b/compiler/rustc_ast_lowering/src/format.rs index ccf481cb9b39d..84f04f024bf14 100644 --- a/compiler/rustc_ast_lowering/src/format.rs +++ b/compiler/rustc_ast_lowering/src/format.rs @@ -2,7 +2,7 @@ use super::LoweringContext; use rustc_ast as ast; use rustc_ast::visit::{self, Visitor}; use rustc_ast::*; -use rustc_data_structures::fx::FxIndexMap; +use rustc_data_structures::fx::{FxIndexMap, IndexEntry}; use rustc_hir as hir; use rustc_span::{ sym, @@ -193,28 +193,38 @@ fn make_argument<'hir>( sp: Span, arg: &'hir hir::Expr<'hir>, ty: ArgumentType, + simple: bool, ) -> hir::Expr<'hir> { use ArgumentType::*; use FormatTrait::*; let new_fn = ctx.arena.alloc(ctx.expr_lang_item_type_relative( sp, hir::LangItem::FormatArgument, - match ty { - Format(Display) => sym::new_display, - Format(Debug) => sym::new_debug, - Format(LowerExp) => sym::new_lower_exp, - Format(UpperExp) => sym::new_upper_exp, - Format(Octal) => sym::new_octal, - Format(Pointer) => sym::new_pointer, - Format(Binary) => sym::new_binary, - Format(LowerHex) => sym::new_lower_hex, - Format(UpperHex) => sym::new_upper_hex, - Usize => sym::from_usize, + match (simple, ty) { + (true, Format(Display)) => sym::new_simple_display, + (false, Format(Display)) => sym::new_display, + (_, Format(Debug)) => sym::new_debug, + (_, Format(LowerExp)) => sym::new_lower_exp, + (_, Format(UpperExp)) => sym::new_upper_exp, + (_, Format(Octal)) => sym::new_octal, + (_, Format(Pointer)) => sym::new_pointer, + (_, Format(Binary)) => sym::new_binary, + (_, Format(LowerHex)) => sym::new_lower_hex, + (_, Format(UpperHex)) => sym::new_upper_hex, + (_, Usize) => sym::from_usize, }, )); ctx.expr_call_mut(sp, new_fn, std::slice::from_ref(arg)) } +/// The data we track for each unique (argument, trait) pair. +struct ArgMeta { + /// Whether all uses of this argument as this trait use no formatting options. + simple: bool, + /// The span of the first placeholder that uses this argument-trait combination. + span: Option, +} + /// Generate a hir expression for a format_args Count. /// /// Generates: @@ -238,7 +248,7 @@ fn make_count<'hir>( ctx: &mut LoweringContext<'_, 'hir>, sp: Span, count: &Option, - argmap: &mut FxIndexMap<(usize, ArgumentType), Option>, + argmap: &mut FxIndexMap<(usize, ArgumentType), ArgMeta>, ) -> hir::Expr<'hir> { match count { Some(FormatCount::Literal(n)) => { @@ -252,7 +262,10 @@ fn make_count<'hir>( } Some(FormatCount::Argument(arg)) => { if let Ok(arg_index) = arg.index { - let (i, _) = argmap.insert_full((arg_index, ArgumentType::Usize), arg.span); + let (i, _) = argmap.insert_full( + (arg_index, ArgumentType::Usize), + ArgMeta { simple: true, span: arg.span }, + ); let count_param = ctx.arena.alloc(ctx.expr_lang_item_type_relative( sp, hir::LangItem::FormatCount, @@ -291,14 +304,13 @@ fn make_format_spec<'hir>( ctx: &mut LoweringContext<'_, 'hir>, sp: Span, placeholder: &FormatPlaceholder, - argmap: &mut FxIndexMap<(usize, ArgumentType), Option>, + argmap: &mut FxIndexMap<(usize, ArgumentType), ArgMeta>, ) -> hir::Expr<'hir> { let position = match placeholder.argument.index { Ok(arg_index) => { - let (i, _) = argmap.insert_full( - (arg_index, ArgumentType::Format(placeholder.format_trait)), - placeholder.span, - ); + let i = argmap + .get_index_of(&(arg_index, ArgumentType::Format(placeholder.format_trait))) + .unwrap(); ctx.expr_usize(sp, i) } Err(_) => ctx.expr( @@ -388,21 +400,25 @@ fn expand_format_args<'hir>( // Create a list of all _unique_ (argument, format trait) combinations. // E.g. "{0} {0:x} {0} {1}" -> [(0, Display), (0, LowerHex), (1, Display)] - let mut argmap = FxIndexMap::default(); + let mut argmap = FxIndexMap::<(usize, ArgumentType), ArgMeta>::default(); for piece in &fmt.template { let FormatArgsPiece::Placeholder(placeholder) = piece else { continue }; - if placeholder.format_options != Default::default() { + let simple = placeholder.format_options == Default::default(); + if !simple { // Can't use basic form if there's any formatting options. use_format_options = true; } if let Ok(index) = placeholder.argument.index { - if argmap - .insert((index, ArgumentType::Format(placeholder.format_trait)), placeholder.span) - .is_some() - { - // Duplicate (argument, format trait) combination, - // which we'll only put once in the args array. - use_format_options = true; + match argmap.entry((index, ArgumentType::Format(placeholder.format_trait))) { + IndexEntry::Occupied(mut entry) => { + entry.get_mut().simple &= simple; + // Duplicate (argument, format trait) combination, + // which we'll only put once in the args array. + use_format_options = true; + } + IndexEntry::Vacant(entry) => { + entry.insert(ArgMeta { simple, span: placeholder.span }); + } } } } @@ -457,9 +473,8 @@ fn expand_format_args<'hir>( let elements: Vec<_> = arguments .iter() .zip(argmap) - .map(|(arg, ((_, ty), placeholder_span))| { - let placeholder_span = - placeholder_span.unwrap_or(arg.expr.span).with_ctxt(macsp.ctxt()); + .map(|(arg, ((_, ty), ArgMeta { simple, span }))| { + let placeholder_span = span.unwrap_or(arg.expr.span).with_ctxt(macsp.ctxt()); let arg_span = match arg.kind { FormatArgumentKind::Captured(_) => placeholder_span, _ => arg.expr.span.with_ctxt(macsp.ctxt()), @@ -469,7 +484,7 @@ fn expand_format_args<'hir>( arg_span, hir::ExprKind::AddrOf(hir::BorrowKind::Ref, hir::Mutability::Not, arg), )); - make_argument(ctx, placeholder_span, ref_arg, ty) + make_argument(ctx, placeholder_span, ref_arg, ty, simple) }) .collect(); ctx.expr_array_ref(macsp, ctx.arena.alloc_from_iter(elements)) @@ -486,10 +501,9 @@ fn expand_format_args<'hir>( let args_ident = Ident::new(sym::args, macsp); let (args_pat, args_hir_id) = ctx.pat_ident(macsp, args_ident); let args = ctx.arena.alloc_from_iter(argmap.iter().map( - |(&(arg_index, ty), &placeholder_span)| { + |(&(arg_index, ty), &ArgMeta { simple, span })| { let arg = &arguments[arg_index]; - let placeholder_span = - placeholder_span.unwrap_or(arg.expr.span).with_ctxt(macsp.ctxt()); + let placeholder_span = span.unwrap_or(arg.expr.span).with_ctxt(macsp.ctxt()); let arg_span = match arg.kind { FormatArgumentKind::Captured(_) => placeholder_span, _ => arg.expr.span.with_ctxt(macsp.ctxt()), @@ -502,7 +516,7 @@ fn expand_format_args<'hir>( Ident::new(sym::integer(arg_index), macsp), ), )); - make_argument(ctx, placeholder_span, arg, ty) + make_argument(ctx, placeholder_span, arg, ty, simple) }, )); let elements: Vec<_> = arguments diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 70b9088de5064..7ff865ad11875 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -996,6 +996,7 @@ symbols! { new_lower_hex, new_octal, new_pointer, + new_simple_display, new_unchecked, new_upper_exp, new_upper_hex, diff --git a/library/alloc/src/lib.rs b/library/alloc/src/lib.rs index 2c6a266e2a14c..ece53ccee4be2 100644 --- a/library/alloc/src/lib.rs +++ b/library/alloc/src/lib.rs @@ -145,6 +145,7 @@ #![feature(receiver_trait)] #![feature(saturating_int_impl)] #![feature(set_ptr_value)] +#![feature(simple_fmt)] #![feature(sized_type_properties)] #![feature(slice_from_ptr_range)] #![feature(slice_group_by)] diff --git a/library/alloc/src/string.rs b/library/alloc/src/string.rs index cf16a3424a092..e0f87bb3a3fdf 100644 --- a/library/alloc/src/string.rs +++ b/library/alloc/src/string.rs @@ -2261,6 +2261,10 @@ impl fmt::Display for String { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Display::fmt(&**self, f) } + #[inline] + fn simple_fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(&**self) + } } #[stable(feature = "rust1", since = "1.0.0")] diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index a901ae726698a..1b1e5446efdeb 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -354,6 +354,13 @@ impl<'a> ArgumentV1<'a> { arg_new!(new_lower_exp, LowerExp); arg_new!(new_upper_exp, UpperExp); + #[doc(hidden)] + #[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")] + #[inline] + pub fn new_simple_display(x: &'a T) -> Self { + Self::new(x, Display::simple_fmt) + } + #[doc(hidden)] #[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")] pub fn from_usize(x: &usize) -> ArgumentV1<'_> { @@ -822,6 +829,19 @@ pub trait Display { /// ``` #[stable(feature = "rust1", since = "1.0.0")] fn fmt(&self, f: &mut Formatter<'_>) -> Result; + + /// Formats the value using the given formatter, but may ignore all formatting options. + /// + /// This method is called when using a placeholder without any formatting options, + /// like in `println!("{value:?}")`. + /// + /// The default implementation just calls `fmt`, but this can be overridden + /// with a more optimized version that ignores all formatting options. + #[unstable(feature = "simple_fmt", issue = "105054")] + #[inline] + fn simple_fmt(&self, f: &mut Formatter<'_>) -> Result { + self.fmt(f) + } } /// `o` formatting. @@ -2418,7 +2438,26 @@ macro_rules! fmt_refs { } } -fmt_refs! { Debug, Display, Octal, Binary, LowerHex, UpperHex, LowerExp, UpperExp } +fmt_refs! { Debug, Octal, Binary, LowerHex, UpperHex, LowerExp, UpperExp } + +#[stable(feature = "rust1", since = "1.0.0")] +impl Display for &T { + fn fmt(&self, f: &mut Formatter<'_>) -> Result { + Display::fmt(&**self, f) + } + fn simple_fmt(&self, f: &mut Formatter<'_>) -> Result { + Display::simple_fmt(&**self, f) + } +} +#[stable(feature = "rust1", since = "1.0.0")] +impl Display for &mut T { + fn fmt(&self, f: &mut Formatter<'_>) -> Result { + Display::fmt(&**self, f) + } + fn simple_fmt(&self, f: &mut Formatter<'_>) -> Result { + Display::simple_fmt(&**self, f) + } +} #[unstable(feature = "never_type", issue = "35121")] impl Debug for ! { @@ -2479,6 +2518,10 @@ impl Display for str { fn fmt(&self, f: &mut Formatter<'_>) -> Result { f.pad(self) } + #[inline] + fn simple_fmt(&self, f: &mut Formatter<'_>) -> Result { + f.write_str(self) + } } #[stable(feature = "rust1", since = "1.0.0")] @@ -2505,6 +2548,10 @@ impl Display for char { f.pad(self.encode_utf8(&mut [0; 4])) } } + #[inline] + fn simple_fmt(&self, f: &mut Formatter<'_>) -> Result { + f.write_char(*self) + } } #[stable(feature = "rust1", since = "1.0.0")] From 35ba401a65003ef3a8f927bb1c96f49ccebfb501 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Mon, 24 Apr 2023 18:20:06 +0200 Subject: [PATCH 2/3] Update tests. --- .../sroa/lifetimes.foo.ScalarReplacementOfAggregates.diff | 8 ++++---- tests/ui/unpretty/flattened-format-args.stdout | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/mir-opt/sroa/lifetimes.foo.ScalarReplacementOfAggregates.diff b/tests/mir-opt/sroa/lifetimes.foo.ScalarReplacementOfAggregates.diff index 579587a430b25..98f19dafe9688 100644 --- a/tests/mir-opt/sroa/lifetimes.foo.ScalarReplacementOfAggregates.diff +++ b/tests/mir-opt/sroa/lifetimes.foo.ScalarReplacementOfAggregates.diff @@ -113,11 +113,11 @@ StorageLive(_22); // scope 4 at $DIR/lifetimes.rs:+10:20: +10:23 _22 = &_8; // scope 4 at $DIR/lifetimes.rs:+10:20: +10:23 _21 = &(*_22); // scope 4 at $DIR/lifetimes.rs:+10:20: +10:23 - _20 = core::fmt::ArgumentV1::<'_>::new_display::>(move _21) -> [return: bb3, unwind unreachable]; // scope 4 at $DIR/lifetimes.rs:+10:20: +10:23 + _20 = core::fmt::ArgumentV1::<'_>::new_simple_display::>(move _21) -> [return: bb3, unwind unreachable]; // scope 4 at $DIR/lifetimes.rs:+10:20: +10:23 // mir::Constant // + span: $DIR/lifetimes.rs:27:20: 27:23 // + user_ty: UserType(4) - // + literal: Const { ty: for<'b> fn(&'b Box) -> core::fmt::ArgumentV1<'b> {core::fmt::ArgumentV1::<'_>::new_display::>}, val: Value() } + // + literal: Const { ty: fn(&Box) -> core::fmt::ArgumentV1<'_> {core::fmt::ArgumentV1::<'_>::new_simple_display::>}, val: Value() } } bb3: { @@ -127,11 +127,11 @@ StorageLive(_25); // scope 4 at $DIR/lifetimes.rs:+10:24: +10:27 _25 = &_6; // scope 4 at $DIR/lifetimes.rs:+10:24: +10:27 _24 = &(*_25); // scope 4 at $DIR/lifetimes.rs:+10:24: +10:27 - _23 = core::fmt::ArgumentV1::<'_>::new_display::(move _24) -> [return: bb4, unwind unreachable]; // scope 4 at $DIR/lifetimes.rs:+10:24: +10:27 + _23 = core::fmt::ArgumentV1::<'_>::new_simple_display::(move _24) -> [return: bb4, unwind unreachable]; // scope 4 at $DIR/lifetimes.rs:+10:24: +10:27 // mir::Constant // + span: $DIR/lifetimes.rs:27:24: 27:27 // + user_ty: UserType(5) - // + literal: Const { ty: for<'b> fn(&'b u32) -> core::fmt::ArgumentV1<'b> {core::fmt::ArgumentV1::<'_>::new_display::}, val: Value() } + // + literal: Const { ty: fn(&u32) -> core::fmt::ArgumentV1<'_> {core::fmt::ArgumentV1::<'_>::new_simple_display::}, val: Value() } } bb4: { diff --git a/tests/ui/unpretty/flattened-format-args.stdout b/tests/ui/unpretty/flattened-format-args.stdout index a8fe8da002472..62d06742b74bc 100644 --- a/tests/ui/unpretty/flattened-format-args.stdout +++ b/tests/ui/unpretty/flattened-format-args.stdout @@ -11,6 +11,6 @@ fn main() { { ::std::io::_print(<#[lang = "format_arguments"]>::new_v1(&["a 123 b ", " xyz\n"], - &[<#[lang = "format_argument"]>::new_display(&x)])); + &[<#[lang = "format_argument"]>::new_simple_display(&x)])); }; } From 36494dbc5b894aebb47b38d519bedcb65fa1fe55 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Mon, 24 Apr 2023 18:55:35 +0200 Subject: [PATCH 3/3] Update miri test. --- .../miri/tests/fail/panic/double_panic.stderr | 54 ++++++++++--------- 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/src/tools/miri/tests/fail/panic/double_panic.stderr b/src/tools/miri/tests/fail/panic/double_panic.stderr index 6bf13f2160150..9149eb74297f6 100644 --- a/src/tools/miri/tests/fail/panic/double_panic.stderr +++ b/src/tools/miri/tests/fail/panic/double_panic.stderr @@ -12,57 +12,59 @@ stack backtrace: at RUSTLIB/std/src/sys_common/backtrace.rs:LL:CC 4: ::fmt at RUSTLIB/std/src/sys_common/backtrace.rs:LL:CC - 5: std::fmt::write + 5: ::simple_fmt at RUSTLIB/core/src/fmt/mod.rs:LL:CC - 6: ::write_fmt + 6: std::fmt::write + at RUSTLIB/core/src/fmt/mod.rs:LL:CC + 7: ::write_fmt at RUSTLIB/std/src/io/mod.rs:LL:CC - 7: std::sys_common::backtrace::_print + 8: std::sys_common::backtrace::_print at RUSTLIB/std/src/sys_common/backtrace.rs:LL:CC - 8: std::sys_common::backtrace::print + 9: std::sys_common::backtrace::print at RUSTLIB/std/src/sys_common/backtrace.rs:LL:CC - 9: std::panicking::default_hook::{closure#1} + 10: std::panicking::default_hook::{closure#1} at RUSTLIB/std/src/panicking.rs:LL:CC - 10: std::panicking::default_hook + 11: std::panicking::default_hook at RUSTLIB/std/src/panicking.rs:LL:CC - 11: std::panicking::rust_panic_with_hook + 12: std::panicking::rust_panic_with_hook at RUSTLIB/std/src/panicking.rs:LL:CC - 12: std::rt::begin_panic::{closure#0} + 13: std::rt::begin_panic::{closure#0} at RUSTLIB/std/src/panicking.rs:LL:CC - 13: std::sys_common::backtrace::__rust_end_short_backtrace + 14: std::sys_common::backtrace::__rust_end_short_backtrace at RUSTLIB/std/src/sys_common/backtrace.rs:LL:CC - 14: std::rt::begin_panic + 15: std::rt::begin_panic at RUSTLIB/std/src/panicking.rs:LL:CC - 15: ::drop + 16: ::drop at $DIR/double_panic.rs:LL:CC - 16: std::ptr::drop_in_place - shim(Some(Foo)) + 17: std::ptr::drop_in_place - shim(Some(Foo)) at RUSTLIB/core/src/ptr/mod.rs:LL:CC - 17: main + 18: main at $DIR/double_panic.rs:LL:CC - 18: >::call_once - shim(fn()) + 19: >::call_once - shim(fn()) at RUSTLIB/core/src/ops/function.rs:LL:CC - 19: std::sys_common::backtrace::__rust_begin_short_backtrace + 20: std::sys_common::backtrace::__rust_begin_short_backtrace at RUSTLIB/std/src/sys_common/backtrace.rs:LL:CC - 20: std::rt::lang_start::{closure#0} + 21: std::rt::lang_start::{closure#0} at RUSTLIB/std/src/rt.rs:LL:CC - 21: std::ops::function::impls::call_once + 22: std::ops::function::impls::call_once at RUSTLIB/core/src/ops/function.rs:LL:CC - 22: std::panicking::r#try::do_call + 23: std::panicking::r#try::do_call at RUSTLIB/std/src/panicking.rs:LL:CC - 23: std::panicking::r#try + 24: std::panicking::r#try at RUSTLIB/std/src/panicking.rs:LL:CC - 24: std::panic::catch_unwind + 25: std::panic::catch_unwind at RUSTLIB/std/src/panic.rs:LL:CC - 25: std::rt::lang_start_internal::{closure#2} + 26: std::rt::lang_start_internal::{closure#2} at RUSTLIB/std/src/rt.rs:LL:CC - 26: std::panicking::r#try::do_call + 27: std::panicking::r#try::do_call at RUSTLIB/std/src/panicking.rs:LL:CC - 27: std::panicking::r#try + 28: std::panicking::r#try at RUSTLIB/std/src/panicking.rs:LL:CC - 28: std::panic::catch_unwind + 29: std::panic::catch_unwind at RUSTLIB/std/src/panic.rs:LL:CC - 29: std::rt::lang_start_internal + 30: std::rt::lang_start_internal at RUSTLIB/std/src/rt.rs:LL:CC - 30: std::rt::lang_start + 31: std::rt::lang_start at RUSTLIB/std/src/rt.rs:LL:CC thread panicked while panicking. aborting. error: abnormal termination: the program aborted execution