Skip to content

Optimize format_args placeholders without options: Display::simple_fmt #104525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

Optimize format_args placeholders without options: Display::simple_fmt #104525

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d4ed723
Optimize formatting str/String without options.
m-ou-se Apr 24, 2023
35ba401
Update tests.
m-ou-se Apr 24, 2023
36494db
Update miri test.
m-ou-se Apr 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 50 additions & 36 deletions compiler/rustc_ast_lowering/src/format.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<Span>,
}

/// Generate a hir expression for a format_args Count.
///
/// Generates:
Expand All @@ -238,7 +248,7 @@ fn make_count<'hir>(
ctx: &mut LoweringContext<'_, 'hir>,
sp: Span,
count: &Option<FormatCount>,
argmap: &mut FxIndexMap<(usize, ArgumentType), Option<Span>>,
argmap: &mut FxIndexMap<(usize, ArgumentType), ArgMeta>,
) -> hir::Expr<'hir> {
match count {
Some(FormatCount::Literal(n)) => {
Expand All @@ -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,
Expand Down Expand Up @@ -291,14 +304,13 @@ fn make_format_spec<'hir>(
ctx: &mut LoweringContext<'_, 'hir>,
sp: Span,
placeholder: &FormatPlaceholder,
argmap: &mut FxIndexMap<(usize, ArgumentType), Option<Span>>,
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(
Expand Down Expand Up @@ -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 });
}
}
}
}
Expand Down Expand Up @@ -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()),
Expand All @@ -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))
Expand All @@ -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()),
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -996,6 +996,7 @@ symbols! {
new_lower_hex,
new_octal,
new_pointer,
new_simple_display,
new_unchecked,
new_upper_exp,
new_upper_hex,
Expand Down
1 change: 1 addition & 0 deletions library/alloc/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
4 changes: 4 additions & 0 deletions library/alloc/src/string.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down
49 changes: 48 additions & 1 deletion library/core/src/fmt/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<T: 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<'_> {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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<T: ?Sized + Display> 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<T: ?Sized + Display> 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 ! {
Expand Down Expand Up @@ -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")]
Expand All @@ -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")]
Expand Down
54 changes: 28 additions & 26 deletions src/tools/miri/tests/fail/panic/double_panic.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,57 +12,59 @@ stack backtrace:
at RUSTLIB/std/src/sys_common/backtrace.rs:LL:CC
4: <std::sys_common::backtrace::_print::DisplayBacktrace as std::fmt::Display>::fmt
at RUSTLIB/std/src/sys_common/backtrace.rs:LL:CC
5: std::fmt::write
5: <std::sys_common::backtrace::_print::DisplayBacktrace as std::fmt::Display>::simple_fmt
at RUSTLIB/core/src/fmt/mod.rs:LL:CC
6: <std::sys::PLATFORM::stdio::Stderr as std::io::Write>::write_fmt
6: std::fmt::write
at RUSTLIB/core/src/fmt/mod.rs:LL:CC
7: <std::sys::PLATFORM::stdio::Stderr as std::io::Write>::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: <Foo as std::ops::Drop>::drop
16: <Foo as std::ops::Drop>::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: <fn() as std::ops::FnOnce<()>>::call_once - shim(fn())
19: <fn() as std::ops::FnOnce<()>>::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
Expand Down
Loading