Skip to content

Commit

Permalink
Auto merge of rust-lang#90485 - camsteffen:fmt-args-less-bind, r=m-ou-se
Browse files Browse the repository at this point in the history
Don't destructure args tuple in format_args!

This allows Clippy to parse the HIR more simply since `arg0` is changed to `_args.0`. (cc rust-lang/rust-clippy#7843). From rustc's perspective, I think this is something between a lateral move and a tiny improvement since there are fewer bindings.

r? `@m-ou-se`
  • Loading branch information
bors committed Nov 9, 2021
2 parents c57704f + 8e21f3a commit 60952bc
Show file tree
Hide file tree
Showing 8 changed files with 21 additions and 37 deletions.
31 changes: 10 additions & 21 deletions compiler/rustc_builtin_macros/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -760,16 +760,11 @@ impl<'a, 'b> Context<'a, 'b> {
/// Actually builds the expression which the format_args! block will be
/// expanded to.
fn into_expr(self) -> P<ast::Expr> {
let mut locals =
Vec::with_capacity((0..self.args.len()).map(|i| self.arg_unique_types[i].len()).sum());
let mut counts = Vec::with_capacity(self.count_args.len());
let mut pats = Vec::with_capacity(self.args.len());
let mut args = Vec::with_capacity(
self.arg_unique_types.iter().map(|v| v.len()).sum::<usize>() + self.count_args.len(),
);
let mut heads = Vec::with_capacity(self.args.len());

let names_pos: Vec<_> = (0..self.args.len())
.map(|i| Ident::from_str_and_span(&format!("arg{}", i), self.macsp))
.collect();

// First, build up the static array which will become our precompiled
// format "string"
let pieces = self.ecx.expr_vec_slice(self.fmtsp, self.str_pieces);
Expand All @@ -787,11 +782,8 @@ impl<'a, 'b> Context<'a, 'b> {
// of each variable because we don't want to move out of the arguments
// passed to this function.
for (i, e) in self.args.into_iter().enumerate() {
let name = names_pos[i];
let span = self.ecx.with_def_site_ctxt(e.span);
pats.push(self.ecx.pat_ident(span, name));
for arg_ty in self.arg_unique_types[i].iter() {
locals.push(Context::format_arg(self.ecx, self.macsp, e.span, arg_ty, name));
args.push(Context::format_arg(self.ecx, self.macsp, e.span, arg_ty, i));
}
heads.push(self.ecx.expr_addr_of(e.span, e));
}
Expand All @@ -800,15 +792,11 @@ impl<'a, 'b> Context<'a, 'b> {
Exact(i) => i,
_ => panic!("should never happen"),
};
let name = names_pos[index];
let span = spans_pos[index];
counts.push(Context::format_arg(self.ecx, self.macsp, span, &Count, name));
args.push(Context::format_arg(self.ecx, self.macsp, span, &Count, index));
}

// Now create a vector containing all the arguments
let args = locals.into_iter().chain(counts.into_iter());

let args_array = self.ecx.expr_vec(self.macsp, args.collect());
let args_array = self.ecx.expr_vec(self.macsp, args);

// Constructs an AST equivalent to:
//
Expand Down Expand Up @@ -838,7 +826,7 @@ impl<'a, 'b> Context<'a, 'b> {
// But the nested match expression is proved to perform not as well
// as series of let's; the first approach does.
let args_match = {
let pat = self.ecx.pat_tuple(self.macsp, pats);
let pat = self.ecx.pat_ident(self.macsp, Ident::new(sym::_args, self.macsp));
let arm = self.ecx.arm(self.macsp, pat, args_array);
let head = self.ecx.expr(self.macsp, ast::ExprKind::Tup(heads));
self.ecx.expr_match(self.macsp, head, vec![arm])
Expand Down Expand Up @@ -877,10 +865,11 @@ impl<'a, 'b> Context<'a, 'b> {
macsp: Span,
mut sp: Span,
ty: &ArgumentType,
arg: Ident,
arg_index: usize,
) -> P<ast::Expr> {
sp = ecx.with_def_site_ctxt(sp);
let arg = ecx.expr_ident(sp, arg);
let arg = ecx.expr_ident(sp, Ident::new(sym::_args, sp));
let arg = ecx.expr(sp, ast::ExprKind::Field(arg, Ident::new(sym::integer(arg_index), sp)));
let trait_ = match *ty {
Placeholder(trait_) if trait_ == "<invalid>" => return DummyResult::raw_expr(sp, true),
Placeholder(trait_) => trait_,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ symbols! {
__S,
__next,
__try_var,
_args,
_d,
_e,
_task_context,
Expand Down
2 changes: 1 addition & 1 deletion src/test/pretty/dollar-crate.pp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
{
::std::io::_print(::core::fmt::Arguments::new_v1(&["rust\n"],
&match () {
() => [],
_args => [],
}));
};
}
2 changes: 1 addition & 1 deletion src/test/pretty/issue-4264.pp
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
as
())
{
()
_args
=>
([]
as
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/attributes/key-value-expansion.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ error: unexpected token: `{
let res =
::alloc::fmt::format(::core::fmt::Arguments::new_v1(&[""],
&match (&"u8",) {
(arg0,) =>
[::core::fmt::ArgumentV1::new(arg0,
_args =>
[::core::fmt::ArgumentV1::new(_args.0,
::core::fmt::Display::fmt)],
}));
res
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ LL | let c1 : () = c;
| expected due to this
|
= note: expected unit type `()`
found closure `[mod1::f<T>::{closure#0} closure_substs=(unavailable) substs=[T, _#25t, extern "rust-call" fn(()), _#26t]]`
found closure `[mod1::f<T>::{closure#0} closure_substs=(unavailable) substs=[T, _#22t, extern "rust-call" fn(()), _#23t]]`
help: use parentheses to call this closure
|
LL | let c1 : () = c();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ LL | let c1 : () = c;
| expected due to this
|
= note: expected unit type `()`
found closure `[f<T>::{closure#0} closure_substs=(unavailable) substs=[T, _#25t, extern "rust-call" fn(()), _#26t]]`
found closure `[f<T>::{closure#0} closure_substs=(unavailable) substs=[T, _#22t, extern "rust-call" fn(()), _#23t]]`
help: use parentheses to call this closure
|
LL | let c1 : () = c();
Expand Down
14 changes: 4 additions & 10 deletions src/tools/clippy/clippy_utils/src/higher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
#![deny(clippy::missing_docs_in_private_items)]

use crate::ty::is_type_diagnostic_item;
use crate::{is_expn_of, last_path_segment, match_def_path, path_to_local_id, paths};
use crate::{is_expn_of, last_path_segment, match_def_path, paths};
use if_chain::if_chain;
use rustc_ast::ast::{self, LitKind};
use rustc_hir as hir;
use rustc_hir::{
Arm, Block, BorrowKind, Expr, ExprKind, HirId, LoopSource, MatchSource, Node, Pat, PatKind, QPath, StmtKind, UnOp,
Arm, Block, BorrowKind, Expr, ExprKind, HirId, LoopSource, MatchSource, Node, Pat, QPath, StmtKind, UnOp,
};
use rustc_lint::LateContext;
use rustc_span::{sym, symbol, ExpnKind, Span, Symbol};
Expand Down Expand Up @@ -513,8 +513,6 @@ pub struct FormatArgsExpn<'tcx> {
pub format_string_parts: &'tcx [Expr<'tcx>],
/// Symbols corresponding to [`Self::format_string_parts`]
pub format_string_symbols: Vec<Symbol>,
/// Match arm patterns, the `arg0`, etc. from the next field `args`
pub arg_names: &'tcx [Pat<'tcx>],
/// Expressions like `ArgumentV1::new(arg0, Debug::fmt)`
pub args: &'tcx [Expr<'tcx>],
/// The final argument passed to `Arguments::new_v1_formatted`, if applicable
Expand Down Expand Up @@ -559,15 +557,13 @@ impl FormatArgsExpn<'tcx> {
_ => None,
})
.collect();
if let PatKind::Tuple(arg_names, None) = arm.pat.kind;
if let ExprKind::Array(args) = arm.body.kind;
then {
Some(FormatArgsExpn {
format_string_span: strs_ref.span,
value_args,
format_string_parts,
format_string_symbols,
arg_names,
args,
fmt_expr,
})
Expand All @@ -594,10 +590,8 @@ impl FormatArgsExpn<'tcx> {
if let Ok(i) = usize::try_from(position);
let arg = &self.args[i];
if let ExprKind::Call(_, [arg_name, _]) = arg.kind;
if let Some(j) = self
.arg_names
.iter()
.position(|pat| path_to_local_id(arg_name, pat.hir_id));
if let ExprKind::Field(_, j) = arg_name.kind;
if let Ok(j) = j.name.as_str().parse::<usize>();
then {
Some(FormatArgsArg { value: self.value_args[j], arg, fmt: Some(fmt) })
} else {
Expand Down

0 comments on commit 60952bc

Please sign in to comment.