Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit 0bcacb3

Browse files
borsbors
committed
Auto merge of rust-lang#91359 - dtolnay:args, r=Mark-Simulacrum
Emit simpler code from format_args I made this PR so that `cargo expand` dumps a less overwhelming amount of formatting-related code. <br> `println!("rust")` **Before:** ```rust { ::std::io::_print(::core::fmt::Arguments::new_v1(&["rust\n"], &match () { _args => [], })); }; ``` **After:** ```rust { ::std::io::_print(::core::fmt::Arguments::new_v1(&["rust\n"], &[])); }; ``` `println!("{}", x)` **Before:** ```rust { ::std::io::_print(::core::fmt::Arguments::new_v1( &["", "\n"], &match (&x,) { _args => [::core::fmt::ArgumentV1::new( _args.0, ::core::fmt::Display::fmt, )], }, )); }; ``` **After:** ```rust { ::std::io::_print(::core::fmt::Arguments::new_v1( &["", "\n"], &[::core::fmt::ArgumentV1::new(&x, ::core::fmt::Display::fmt)], )); }; ```
2 parents 523be2e + 7ee21e3 commit 0bcacb3

File tree

11 files changed

+123
-91
lines changed

11 files changed

+123
-91
lines changed

‎compiler/rustc_ast/src/ast.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ use rustc_span::{Span, DUMMY_SP};
3939
use std::cmp::Ordering;
4040
use std::convert::TryFrom;
4141
use std::fmt;
42+
use std::mem;
4243

4344
#[cfg(test)]
4445
mod tests;
@@ -1276,6 +1277,19 @@ impl Expr {
12761277
ExprKind::Err => ExprPrecedence::Err,
12771278
}
12781279
}
1280+
1281+
pub fn take(&mut self) -> Self {
1282+
mem::replace(
1283+
self,
1284+
Expr {
1285+
id: DUMMY_NODE_ID,
1286+
kind: ExprKind::Err,
1287+
span: DUMMY_SP,
1288+
attrs: ThinVec::new(),
1289+
tokens: None,
1290+
},
1291+
)
1292+
}
12791293
}
12801294

12811295
/// Limit types of a range (inclusive or exclusive)

‎compiler/rustc_builtin_macros/src/format.rs‎

Lines changed: 75 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use rustc_expand::base::{self, *};
1111
use rustc_parse_format as parse;
1212
use rustc_span::symbol::{sym, Ident, Symbol};
1313
use rustc_span::{MultiSpan, Span};
14+
use smallvec::SmallVec;
1415

1516
use std::borrow::Cow;
1617
use std::collections::hash_map::Entry;
@@ -744,78 +745,95 @@ impl<'a, 'b> Context<'a, 'b> {
744745
/// Actually builds the expression which the format_args! block will be
745746
/// expanded to.
746747
fn into_expr(self) -> P<ast::Expr> {
747-
let mut args = Vec::with_capacity(
748+
let mut original_args = self.args;
749+
let mut fmt_args = Vec::with_capacity(
748750
self.arg_unique_types.iter().map(|v| v.len()).sum::<usize>() + self.count_args.len(),
749751
);
750-
let mut heads = Vec::with_capacity(self.args.len());
751752

752753
// First, build up the static array which will become our precompiled
753754
// format "string"
754755
let pieces = self.ecx.expr_vec_slice(self.fmtsp, self.str_pieces);
755756

756-
// Before consuming the expressions, we have to remember spans for
757-
// count arguments as they are now generated separate from other
758-
// arguments, hence have no access to the `P<ast::Expr>`'s.
759-
let spans_pos: Vec<_> = self.args.iter().map(|e| e.span).collect();
760-
761-
// Right now there is a bug such that for the expression:
762-
// foo(bar(&1))
763-
// the lifetime of `1` doesn't outlast the call to `bar`, so it's not
764-
// valid for the call to `foo`. To work around this all arguments to the
765-
// format! string are shoved into locals. Furthermore, we shove the address
766-
// of each variable because we don't want to move out of the arguments
767-
// passed to this function.
768-
for (i, e) in self.args.into_iter().enumerate() {
769-
for arg_ty in self.arg_unique_types[i].iter() {
770-
args.push(Context::format_arg(self.ecx, self.macsp, e.span, arg_ty, i));
771-
}
772-
// use the arg span for `&arg` so that borrowck errors
773-
// point to the specific expression passed to the macro
774-
// (the span is otherwise unavailable in MIR)
775-
heads.push(self.ecx.expr_addr_of(e.span.with_ctxt(self.macsp.ctxt()), e));
776-
}
777-
for index in self.count_args {
778-
let span = spans_pos[index];
779-
args.push(Context::format_arg(self.ecx, self.macsp, span, &Count, index));
757+
// We need to construct a &[ArgumentV1] to pass into the fmt::Arguments
758+
// constructor. In general the expressions in this slice might be
759+
// permuted from their order in original_args (such as in the case of
760+
// "{1} {0}"), or may have multiple entries referring to the same
761+
// element of original_args ("{0} {0}").
762+
//
763+
// The following vector has one item per element of our output slice,
764+
// identifying the index of which element of original_args it's passing,
765+
// and that argument's type.
766+
let mut fmt_arg_index_and_ty = SmallVec::<[(usize, &ArgumentType); 8]>::new();
767+
for (i, unique_types) in self.arg_unique_types.iter().enumerate() {
768+
fmt_arg_index_and_ty.extend(unique_types.iter().map(|ty| (i, ty)));
780769
}
770+
fmt_arg_index_and_ty.extend(self.count_args.iter().map(|&i| (i, &Count)));
781771

782-
let args_array = self.ecx.expr_vec(self.macsp, args);
783-
784-
// Constructs an AST equivalent to:
785-
//
786-
// match (&arg0, &arg1) {
787-
// (tmp0, tmp1) => args_array
788-
// }
772+
// Figure out whether there are permuted or repeated elements. If not,
773+
// we can generate simpler code.
789774
//
790-
// It was:
775+
// The sequence has no indices out of order or repeated if: for every
776+
// adjacent pair of elements, the first one's index is less than the
777+
// second one's index.
778+
let nicely_ordered =
779+
fmt_arg_index_and_ty.array_windows().all(|[(i, _i_ty), (j, _j_ty)]| i < j);
780+
781+
// We want to emit:
791782
//
792-
// let tmp0 = &arg0;
793-
// let tmp1 = &arg1;
794-
// args_array
783+
// [ArgumentV1::new(&$arg0, …), ArgumentV1::new(&$arg1, …), …]
795784
//
796-
// Because of #11585 the new temporary lifetime rule, the enclosing
797-
// statements for these temporaries become the let's themselves.
798-
// If one or more of them are RefCell's, RefCell borrow() will also
799-
// end there; they don't last long enough for args_array to use them.
800-
// The match expression solves the scope problem.
785+
// However, it's only legal to do so if $arg0, $arg1, … were written in
786+
// exactly that order by the programmer. When arguments are permuted, we
787+
// want them evaluated in the order written by the programmer, not in
788+
// the order provided to fmt::Arguments. When arguments are repeated, we
789+
// want the expression evaluated only once.
801790
//
802-
// Note, it may also very well be transformed to:
791+
// Thus in the not nicely ordered case we emit the following instead:
803792
//
804-
// match arg0 {
805-
// ref tmp0 => {
806-
// match arg1 => {
807-
// ref tmp1 => args_array } } }
793+
// match (&$arg0, &$arg1, …) {
794+
// _args => [ArgumentV1::new(_args.$i, …), ArgumentV1::new(_args.$j, …), …]
795+
// }
808796
//
809-
// But the nested match expression is proved to perform not as well
810-
// as series of let's; the first approach does.
811-
let args_match = {
812-
let pat = self.ecx.pat_ident(self.macsp, Ident::new(sym::_args, self.macsp));
813-
let arm = self.ecx.arm(self.macsp, pat, args_array);
814-
let head = self.ecx.expr(self.macsp, ast::ExprKind::Tup(heads));
815-
self.ecx.expr_match(self.macsp, head, vec![arm])
816-
};
797+
// for the sequence of indices $i, $j, … governed by fmt_arg_index_and_ty.
798+
for (arg_index, arg_ty) in fmt_arg_index_and_ty {
799+
let e = &mut original_args[arg_index];
800+
let span = e.span;
801+
let arg = if nicely_ordered {
802+
let expansion_span = e.span.with_ctxt(self.macsp.ctxt());
803+
// The indices are strictly ordered so e has not been taken yet.
804+
self.ecx.expr_addr_of(expansion_span, P(e.take()))
805+
} else {
806+
let def_site = self.ecx.with_def_site_ctxt(span);
807+
let args_tuple = self.ecx.expr_ident(def_site, Ident::new(sym::_args, def_site));
808+
let member = Ident::new(sym::integer(arg_index), def_site);
809+
self.ecx.expr(def_site, ast::ExprKind::Field(args_tuple, member))
810+
};
811+
fmt_args.push(Context::format_arg(self.ecx, self.macsp, span, arg_ty, arg));
812+
}
817813

818-
let args_slice = self.ecx.expr_addr_of(self.macsp, args_match);
814+
let args_array = self.ecx.expr_vec(self.macsp, fmt_args);
815+
let args_slice = self.ecx.expr_addr_of(
816+
self.macsp,
817+
if nicely_ordered {
818+
args_array
819+
} else {
820+
// In the !nicely_ordered case, none of the exprs were moved
821+
// away in the previous loop.
822+
//
823+
// This uses the arg span for `&arg` so that borrowck errors
824+
// point to the specific expression passed to the macro (the
825+
// span is otherwise unavailable in the MIR used by borrowck).
826+
let heads = original_args
827+
.into_iter()
828+
.map(|e| self.ecx.expr_addr_of(e.span.with_ctxt(self.macsp.ctxt()), e))
829+
.collect();
830+
831+
let pat = self.ecx.pat_ident(self.macsp, Ident::new(sym::_args, self.macsp));
832+
let arm = self.ecx.arm(self.macsp, pat, args_array);
833+
let head = self.ecx.expr(self.macsp, ast::ExprKind::Tup(heads));
834+
self.ecx.expr_match(self.macsp, head, vec![arm])
835+
},
836+
);
819837

820838
// Now create the fmt::Arguments struct with all our locals we created.
821839
let (fn_name, fn_args) = if self.all_pieces_simple {
@@ -848,11 +866,9 @@ impl<'a, 'b> Context<'a, 'b> {
848866
macsp: Span,
849867
mut sp: Span,
850868
ty: &ArgumentType,
851-
arg_index: usize,
869+
arg: P<ast::Expr>,
852870
) -> P<ast::Expr> {
853871
sp = ecx.with_def_site_ctxt(sp);
854-
let arg = ecx.expr_ident(sp, Ident::new(sym::_args, sp));
855-
let arg = ecx.expr(sp, ast::ExprKind::Field(arg, Ident::new(sym::integer(arg_index), sp)));
856872
let trait_ = match *ty {
857873
Placeholder(trait_) if trait_ == "<invalid>" => return DummyResult::raw_expr(sp, true),
858874
Placeholder(trait_) => trait_,

‎compiler/rustc_builtin_macros/src/lib.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@
22
//! injecting code into the crate before it is lowered to HIR.
33
44
#![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")]
5+
#![feature(array_windows)]
56
#![feature(box_patterns)]
67
#![feature(bool_to_option)]
78
#![feature(crate_visibility_modifier)]
89
#![feature(decl_macro)]
10+
#![feature(is_sorted)]
911
#![feature(nll)]
1012
#![feature(proc_macro_internals)]
1113
#![feature(proc_macro_quote)]

‎src/test/pretty/dollar-crate.pp‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,5 @@
99
// pp-exact:dollar-crate.pp
1010

1111
fn main() {
12-
{
13-
::std::io::_print(::core::fmt::Arguments::new_v1(&["rust\n"],
14-
&match () {
15-
_args => [],
16-
}));
17-
};
12+
{ ::std::io::_print(::core::fmt::Arguments::new_v1(&["rust\n"], &[])); };
1813
}

‎src/test/pretty/issue-4264.pp‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,16 +41,7 @@
4141
[&str; 1])
4242
as
4343
&[&str; 1]),
44-
(&(match (()
45-
as
46-
())
47-
{
48-
_args
49-
=>
50-
([]
51-
as
52-
[ArgumentV1; 0]),
53-
}
44+
(&([]
5445
as
5546
[ArgumentV1; 0])
5647
as

‎src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.closure.txt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@
2929
29| 1| some_string = Some(String::from("the string content"));
3030
30| 1| let
3131
31| 1| a
32-
32| 1| =
33-
33| 1| ||
32+
32| | =
33+
33| | ||
3434
34| 0| {
3535
35| 0| let mut countdown = 0;
3636
36| 0| if is_false {
@@ -173,7 +173,7 @@
173173
169| | ;
174174
170| |
175175
171| 1| let short_used_not_covered_closure_line_break_no_block_embedded_branch =
176-
172| 1| | _unused_arg: u8 |
176+
172| | | _unused_arg: u8 |
177177
173| 0| println!(
178178
174| 0| "not called: {}",
179179
175| 0| if is_true { "check" } else { "me" }

‎src/test/ui/attributes/key-value-expansion.stderr‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,8 @@ LL | bug!();
1818
error: unexpected token: `{
1919
let res =
2020
::alloc::fmt::format(::core::fmt::Arguments::new_v1(&[""],
21-
&match (&"u8",) {
22-
_args =>
23-
[::core::fmt::ArgumentV1::new(_args.0,
24-
::core::fmt::Display::fmt)],
25-
}));
21+
&[::core::fmt::ArgumentV1::new(&"u8",
22+
::core::fmt::Display::fmt)]));
2623
res
2724
}.as_str()`
2825
--> $DIR/key-value-expansion.rs:48:23

‎src/test/ui/closures/print/closure-print-generic-trim-off-verbose-2.stderr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ LL | let c1 : () = c;
99
| expected due to this
1010
|
1111
= note: expected unit type `()`
12-
found closure `[mod1::f<T>::{closure#0} closure_substs=(unavailable) substs=[T, _#22t, extern "rust-call" fn(()), _#23t]]`
12+
found closure `[mod1::f<T>::{closure#0} closure_substs=(unavailable) substs=[T, _#19t, extern "rust-call" fn(()), _#20t]]`
1313
help: use parentheses to call this closure
1414
|
1515
LL | let c1 : () = c();

‎src/test/ui/closures/print/closure-print-generic-verbose-2.stderr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ LL | let c1 : () = c;
99
| expected due to this
1010
|
1111
= note: expected unit type `()`
12-
found closure `[f<T>::{closure#0} closure_substs=(unavailable) substs=[T, _#22t, extern "rust-call" fn(()), _#23t]]`
12+
found closure `[f<T>::{closure#0} closure_substs=(unavailable) substs=[T, _#19t, extern "rust-call" fn(()), _#20t]]`
1313
help: use parentheses to call this closure
1414
|
1515
LL | let c1 : () = c();

‎src/test/ui/issues/issue-69455.stderr‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,16 @@
1-
error[E0284]: type annotations needed: cannot satisfy `<u64 as Test<_>>::Output == _`
2-
--> $DIR/issue-69455.rs:29:26
1+
error[E0282]: type annotations needed
2+
--> $DIR/issue-69455.rs:29:5
33
|
4+
LL | type Output;
5+
| ------------ `<Self as Test<Rhs>>::Output` defined here
6+
...
47
LL | println!("{}", 23u64.test(xs.iter().sum()));
5-
| ^^^^ cannot satisfy `<u64 as Test<_>>::Output == _`
8+
| ^^^^^^^^^^^^^^^---------------------------^
9+
| | |
10+
| | this method call resolves to `<Self as Test<Rhs>>::Output`
11+
| cannot infer type for type parameter `T` declared on the associated function `new`
12+
|
13+
= note: this error originates in the macro `$crate::format_args_nl` (in Nightly builds, run with -Z macro-backtrace for more info)
614

715
error[E0283]: type annotations needed
816
--> $DIR/issue-69455.rs:29:26
@@ -25,5 +33,5 @@ LL | println!("{}", 23u64.test(xs.iter().sum::<S>()));
2533

2634
error: aborting due to 2 previous errors
2735

28-
Some errors have detailed explanations: E0283, E0284.
29-
For more information about an error, try `rustc --explain E0283`.
36+
Some errors have detailed explanations: E0282, E0283.
37+
For more information about an error, try `rustc --explain E0282`.

0 commit comments

Comments
 (0)