Skip to content
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

Use span of placeholders in format_args!() expansion. #109664

Merged
merged 4 commits into from
Mar 29, 2023
Merged

Use span of placeholders in format_args!() expansion. #109664

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1b3fda4
Use span of placeholders in format_args!() expansion.
m-ou-se Mar 27, 2023
7f395f1
Bless UI tests.
m-ou-se Mar 27, 2023
769886c
Bless mir-opt tests.
m-ou-se Mar 27, 2023
6c72a00
Add test for span of implicit format args captures.
m-ou-se Mar 27, 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
66 changes: 43 additions & 23 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::FxIndexSet;
use rustc_data_structures::fx::FxIndexMap;
use rustc_hir as hir;
use rustc_span::{
sym,
Expand Down Expand Up @@ -238,7 +238,7 @@ fn make_count<'hir>(
ctx: &mut LoweringContext<'_, 'hir>,
sp: Span,
count: &Option<FormatCount>,
argmap: &mut FxIndexSet<(usize, ArgumentType)>,
argmap: &mut FxIndexMap<(usize, ArgumentType), Option<Span>>,
) -> hir::Expr<'hir> {
match count {
Some(FormatCount::Literal(n)) => {
Expand All @@ -252,7 +252,7 @@ fn make_count<'hir>(
}
Some(FormatCount::Argument(arg)) => {
if let Ok(arg_index) = arg.index {
let (i, _) = argmap.insert_full((arg_index, ArgumentType::Usize));
let (i, _) = argmap.insert_full((arg_index, ArgumentType::Usize), arg.span);
let count_param = ctx.arena.alloc(ctx.expr_lang_item_type_relative(
sp,
hir::LangItem::FormatCount,
Expand Down Expand Up @@ -291,12 +291,14 @@ fn make_format_spec<'hir>(
ctx: &mut LoweringContext<'_, 'hir>,
sp: Span,
placeholder: &FormatPlaceholder,
argmap: &mut FxIndexSet<(usize, ArgumentType)>,
argmap: &mut FxIndexMap<(usize, ArgumentType), Option<Span>>,
) -> hir::Expr<'hir> {
let position = match placeholder.argument.index {
Ok(arg_index) => {
let (i, _) =
argmap.insert_full((arg_index, ArgumentType::Format(placeholder.format_trait)));
let (i, _) = argmap.insert_full(
(arg_index, ArgumentType::Format(placeholder.format_trait)),
placeholder.span,
);
ctx.expr_usize(sp, i)
}
Err(_) => ctx.expr(
Expand Down Expand Up @@ -386,15 +388,18 @@ 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 = FxIndexSet::default();
let mut argmap = FxIndexMap::default();
for piece in &fmt.template {
let FormatArgsPiece::Placeholder(placeholder) = piece else { continue };
if placeholder.format_options != Default::default() {
// 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))) {
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;
Expand Down Expand Up @@ -438,7 +443,7 @@ fn expand_format_args<'hir>(
// This is an optimization, speeding up compilation about 1-2% in some cases.
// See /~https://github.com/rust-lang/rust/pull/106770#issuecomment-1380790609
let use_simple_array = argmap.len() == arguments.len()
&& argmap.iter().enumerate().all(|(i, &(j, _))| i == j)
&& argmap.iter().enumerate().all(|(i, (&(j, _), _))| i == j)
&& arguments.iter().skip(1).all(|arg| !may_contain_yield_point(&arg.expr));

let args = if use_simple_array {
Expand All @@ -452,14 +457,19 @@ fn expand_format_args<'hir>(
let elements: Vec<_> = arguments
.iter()
.zip(argmap)
.map(|(arg, (_, ty))| {
let sp = arg.expr.span.with_ctxt(macsp.ctxt());
.map(|(arg, ((_, ty), placeholder_span))| {
let placeholder_span =
placeholder_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()),
};
let arg = ctx.lower_expr(&arg.expr);
let ref_arg = ctx.arena.alloc(ctx.expr(
sp,
arg_span,
hir::ExprKind::AddrOf(hir::BorrowKind::Ref, hir::Mutability::Not, arg),
));
make_argument(ctx, sp, ref_arg, ty)
make_argument(ctx, placeholder_span, ref_arg, ty)
})
.collect();
ctx.expr_array_ref(macsp, ctx.arena.alloc_from_iter(elements))
Expand All @@ -475,16 +485,26 @@ 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)| {
let arg = &arguments[arg_index];
let sp = arg.expr.span.with_ctxt(macsp.ctxt());
let args_ident_expr = ctx.expr_ident(macsp, args_ident, args_hir_id);
let arg = ctx.arena.alloc(ctx.expr(
sp,
hir::ExprKind::Field(args_ident_expr, Ident::new(sym::integer(arg_index), macsp)),
));
make_argument(ctx, sp, arg, ty)
}));
let args = ctx.arena.alloc_from_iter(argmap.iter().map(
|(&(arg_index, ty), &placeholder_span)| {
let arg = &arguments[arg_index];
let placeholder_span =
placeholder_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()),
};
let args_ident_expr = ctx.expr_ident(macsp, args_ident, args_hir_id);
let arg = ctx.arena.alloc(ctx.expr(
arg_span,
hir::ExprKind::Field(
args_ident_expr,
Ident::new(sym::integer(arg_index), macsp),
),
));
make_argument(ctx, placeholder_span, arg, ty)
},
));
let elements: Vec<_> = arguments
.iter()
.map(|arg| {
Expand Down
44 changes: 22 additions & 22 deletions tests/mir-opt/sroa/lifetimes.foo.ScalarReplacementOfAggregates.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@
let mut _17: &[core::fmt::ArgumentV1<'_>; 2]; // in scope 0 at $SRC_DIR/std/src/macros.rs:LL:COL
let _18: &[core::fmt::ArgumentV1<'_>; 2]; // in scope 0 at $SRC_DIR/std/src/macros.rs:LL:COL
let _19: [core::fmt::ArgumentV1<'_>; 2]; // in scope 0 at $SRC_DIR/std/src/macros.rs:LL:COL
let mut _20: core::fmt::ArgumentV1<'_>; // in scope 0 at $DIR/lifetimes.rs:+10:21: +10:22
let mut _21: &std::boxed::Box<dyn std::fmt::Display>; // in scope 0 at $DIR/lifetimes.rs:+10:21: +10:22
let _22: &std::boxed::Box<dyn std::fmt::Display>; // in scope 0 at $DIR/lifetimes.rs:+10:21: +10:22
let mut _23: core::fmt::ArgumentV1<'_>; // in scope 0 at $DIR/lifetimes.rs:+10:25: +10:26
let mut _24: &u32; // in scope 0 at $DIR/lifetimes.rs:+10:25: +10:26
let _25: &u32; // in scope 0 at $DIR/lifetimes.rs:+10:25: +10:26
let mut _20: core::fmt::ArgumentV1<'_>; // in scope 0 at $DIR/lifetimes.rs:+10:20: +10:23
let mut _21: &std::boxed::Box<dyn std::fmt::Display>; // in scope 0 at $DIR/lifetimes.rs:+10:20: +10:23
let _22: &std::boxed::Box<dyn std::fmt::Display>; // in scope 0 at $DIR/lifetimes.rs:+10:20: +10:23
let mut _23: core::fmt::ArgumentV1<'_>; // in scope 0 at $DIR/lifetimes.rs:+10:24: +10:27
let mut _24: &u32; // in scope 0 at $DIR/lifetimes.rs:+10:24: +10:27
let _25: &u32; // in scope 0 at $DIR/lifetimes.rs:+10:24: +10:27
let mut _27: bool; // in scope 0 at $DIR/lifetimes.rs:+12:1: +12:2
let mut _28: isize; // in scope 0 at $DIR/lifetimes.rs:+12:1: +12:2
let mut _29: isize; // in scope 0 at $DIR/lifetimes.rs:+12:1: +12:2
Expand Down Expand Up @@ -108,34 +108,34 @@
StorageLive(_17); // scope 4 at $SRC_DIR/std/src/macros.rs:LL:COL
StorageLive(_18); // scope 4 at $SRC_DIR/std/src/macros.rs:LL:COL
StorageLive(_19); // scope 4 at $SRC_DIR/std/src/macros.rs:LL:COL
StorageLive(_20); // scope 4 at $DIR/lifetimes.rs:+10:21: +10:22
StorageLive(_21); // scope 4 at $DIR/lifetimes.rs:+10:21: +10:22
StorageLive(_22); // scope 4 at $DIR/lifetimes.rs:+10:21: +10:22
_22 = &_8; // scope 4 at $DIR/lifetimes.rs:+10:21: +10:22
_21 = &(*_22); // scope 4 at $DIR/lifetimes.rs:+10:21: +10:22
_20 = core::fmt::ArgumentV1::<'_>::new_display::<Box<dyn std::fmt::Display>>(move _21) -> bb3; // scope 4 at $DIR/lifetimes.rs:+10:21: +10:22
StorageLive(_20); // scope 4 at $DIR/lifetimes.rs:+10:20: +10:23
StorageLive(_21); // scope 4 at $DIR/lifetimes.rs:+10:20: +10:23
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::<Box<dyn std::fmt::Display>>(move _21) -> bb3; // scope 4 at $DIR/lifetimes.rs:+10:20: +10:23
// mir::Constant
// + span: $DIR/lifetimes.rs:27:21: 27:22
// + span: $DIR/lifetimes.rs:27:20: 27:23
// + user_ty: UserType(4)
// + literal: Const { ty: for<'b> fn(&'b Box<dyn std::fmt::Display>) -> core::fmt::ArgumentV1<'b> {core::fmt::ArgumentV1::<'_>::new_display::<Box<dyn std::fmt::Display>>}, val: Value(<ZST>) }
}

bb3: {
StorageDead(_21); // scope 4 at $DIR/lifetimes.rs:+10:21: +10:22
StorageLive(_23); // scope 4 at $DIR/lifetimes.rs:+10:25: +10:26
StorageLive(_24); // scope 4 at $DIR/lifetimes.rs:+10:25: +10:26
StorageLive(_25); // scope 4 at $DIR/lifetimes.rs:+10:25: +10:26
_25 = &_6; // scope 4 at $DIR/lifetimes.rs:+10:25: +10:26
_24 = &(*_25); // scope 4 at $DIR/lifetimes.rs:+10:25: +10:26
_23 = core::fmt::ArgumentV1::<'_>::new_display::<u32>(move _24) -> bb4; // scope 4 at $DIR/lifetimes.rs:+10:25: +10:26
StorageDead(_21); // scope 4 at $DIR/lifetimes.rs:+10:22: +10:23
StorageLive(_23); // scope 4 at $DIR/lifetimes.rs:+10:24: +10:27
StorageLive(_24); // scope 4 at $DIR/lifetimes.rs:+10:24: +10:27
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::<u32>(move _24) -> bb4; // scope 4 at $DIR/lifetimes.rs:+10:24: +10:27
// mir::Constant
// + span: $DIR/lifetimes.rs:27:25: 27:26
// + 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::<u32>}, val: Value(<ZST>) }
}

bb4: {
StorageDead(_24); // scope 4 at $DIR/lifetimes.rs:+10:25: +10:26
StorageDead(_24); // scope 4 at $DIR/lifetimes.rs:+10:26: +10:27
_19 = [move _20, move _23]; // scope 4 at $SRC_DIR/std/src/macros.rs:LL:COL
StorageDead(_23); // scope 4 at $SRC_DIR/std/src/macros.rs:LL:COL
StorageDead(_20); // scope 4 at $SRC_DIR/std/src/macros.rs:LL:COL
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/consts/const-eval/format.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0015]: cannot call non-const formatting macro in constant functions
--> $DIR/format.rs:2:20
--> $DIR/format.rs:2:13
|
LL | panic!("{:?}", 0);
| ^
| ^^^^
|
= note: calls in constant functions are limited to constant functions, tuple structs and tuple variants
= note: this error originates in the macro `$crate::const_format_args` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)
Expand All @@ -17,10 +17,10 @@ LL | panic!("{:?}", 0);
= note: this error originates in the macro `$crate::const_format_args` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0015]: cannot call non-const formatting macro in constant functions
--> $DIR/format.rs:8:22
--> $DIR/format.rs:8:15
|
LL | println!("{:?}", 0);
| ^
| ^^^^
|
= note: calls in constant functions are limited to constant functions, tuple structs and tuple variants
= note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
Expand Down
22 changes: 22 additions & 0 deletions tests/ui/fmt/format-args-argument-span.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// check-compile

struct DisplayOnly;

impl std::fmt::Display for DisplayOnly {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
unimplemented!()
}
}

fn main() {
let x = Some(1);
println!("{x:?} {x} {x:?}");
//~^ ERROR: `Option<{integer}>` doesn't implement `std::fmt::Display`
println!("{x:?} {x} {x:?}", x = Some(1));
//~^ ERROR: `Option<{integer}>` doesn't implement `std::fmt::Display`
let x = DisplayOnly;
println!("{x} {x:?} {x}");
//~^ ERROR: `DisplayOnly` doesn't implement `Debug`
println!("{x} {x:?} {x}", x = DisplayOnly);
//~^ ERROR: `DisplayOnly` doesn't implement `Debug`
}
51 changes: 51 additions & 0 deletions tests/ui/fmt/format-args-argument-span.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
error[E0277]: `Option<{integer}>` doesn't implement `std::fmt::Display`
--> $DIR/format-args-argument-span.rs:13:21
|
LL | println!("{x:?} {x} {x:?}");
| ^^^ `Option<{integer}>` cannot be formatted with the default formatter
|
= help: the trait `std::fmt::Display` is not implemented for `Option<{integer}>`
= note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
= note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: `Option<{integer}>` doesn't implement `std::fmt::Display`
--> $DIR/format-args-argument-span.rs:15:37
|
LL | println!("{x:?} {x} {x:?}", x = Some(1));
| ^^^^^^^ `Option<{integer}>` cannot be formatted with the default formatter
|
= help: the trait `std::fmt::Display` is not implemented for `Option<{integer}>`
= note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
= note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: `DisplayOnly` doesn't implement `Debug`
--> $DIR/format-args-argument-span.rs:18:19
|
LL | println!("{x} {x:?} {x}");
| ^^^^^ `DisplayOnly` cannot be formatted using `{:?}`
|
= help: the trait `Debug` is not implemented for `DisplayOnly`
= note: add `#[derive(Debug)]` to `DisplayOnly` or manually `impl Debug for DisplayOnly`
= note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider annotating `DisplayOnly` with `#[derive(Debug)]`
|
LL | #[derive(Debug)]
|

error[E0277]: `DisplayOnly` doesn't implement `Debug`
--> $DIR/format-args-argument-span.rs:20:35
|
LL | println!("{x} {x:?} {x}", x = DisplayOnly);
| ^^^^^^^^^^^ `DisplayOnly` cannot be formatted using `{:?}`
|
= help: the trait `Debug` is not implemented for `DisplayOnly`
= note: add `#[derive(Debug)]` to `DisplayOnly` or manually `impl Debug for DisplayOnly`
= note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider annotating `DisplayOnly` with `#[derive(Debug)]`
|
LL | #[derive(Debug)]
|

error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0277`.
14 changes: 6 additions & 8 deletions tests/ui/fmt/ifmt-bad-arg.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,10 +300,9 @@ error[E0308]: mismatched types
--> $DIR/ifmt-bad-arg.rs:78:32
|
LL | println!("{} {:.*} {}", 1, 3.2, 4);
| ^^^
| |
| expected `&usize`, found `&{float}`
| arguments to this function are incorrect
| -- ^^^ expected `&usize`, found `&{float}`
| |
| arguments to this function are incorrect
|
= note: expected reference `&usize`
found reference `&{float}`
Expand All @@ -315,10 +314,9 @@ error[E0308]: mismatched types
--> $DIR/ifmt-bad-arg.rs:81:35
|
LL | println!("{} {:07$.*} {}", 1, 3.2, 4);
| ^^^
| |
| expected `&usize`, found `&{float}`
| arguments to this function are incorrect
| -- ^^^ expected `&usize`, found `&{float}`
| |
| arguments to this function are incorrect
|
= note: expected reference `&usize`
found reference `&{float}`
Expand Down
4 changes: 3 additions & 1 deletion tests/ui/fmt/ifmt-unimpl.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ error[E0277]: the trait bound `str: UpperHex` is not satisfied
--> $DIR/ifmt-unimpl.rs:2:21
|
LL | format!("{:X}", "3");
| ^^^ the trait `UpperHex` is not implemented for `str`
| ---- ^^^ the trait `UpperHex` is not implemented for `str`
| |
| required by a bound introduced by this call
|
= help: the following other types implement trait `UpperHex`:
&T
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/suggestions/issue-97760.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0277]: `<impl IntoIterator as IntoIterator>::Item` doesn't implement `std::fmt::Display`
--> $DIR/issue-97760.rs:4:20
--> $DIR/issue-97760.rs:4:19
|
LL | println!("{x}");
| ^ `<impl IntoIterator as IntoIterator>::Item` cannot be formatted with the default formatter
| ^^^ `<impl IntoIterator as IntoIterator>::Item` cannot be formatted with the default formatter
|
= help: the trait `std::fmt::Display` is not implemented for `<impl IntoIterator as IntoIterator>::Item`
= note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
Expand Down