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

Jump to bottom

Don't destructure args tuple in format_args! #90485

Merged
merged 3 commits into from Nov 9, 2021
Merged

Don't destructure args tuple in format_args! #90485

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
a9a24d5
Don't destructure args tuple in format_args!
camsteffen Nov 1, 2021
9f6a58e
Factor out some Vecs
camsteffen Nov 1, 2021
8e21f3a
Fix Clippy with changed format_args!
camsteffen Nov 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 10 additions & 21 deletions compiler/rustc_builtin_macros/src/format.rs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion src/test/ui/closures/print/closure-print-generic-trim-off-verbose-2.stderr
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion src/test/ui/closures/print/closure-print-generic-verbose-2.stderr
View file
Open in desktop
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
View file
Open in desktop
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