Skip to content

Commit e58e7b1

Browse files
borsbors
committed
Auto merge of #90891 - nbdd0121:format, r=Mark-Simulacrum
Create `core::fmt::ArgumentV1` with generics instead of fn pointer Split from (and prerequisite of) #90488, as this seems to have perf implication. `@rustbot` label: +T-libs
2 parents 08df8b8 + 0d4bb0b commit e58e7b1

File tree

13 files changed

+104
-78
lines changed

13 files changed

+104
-78
lines changed

‎compiler/rustc_builtin_macros/src/format.rs

+14-4
Original file line numberDiff line numberDiff line change
@@ -877,11 +877,21 @@ impl<'a, 'b> Context<'a, 'b> {
877877
return ecx.expr_call_global(macsp, path, vec![arg]);
878878
}
879879
};
880+
let new_fn_name = match trait_ {
881+
"Display" => "new_display",
882+
"Debug" => "new_debug",
883+
"LowerExp" => "new_lower_exp",
884+
"UpperExp" => "new_upper_exp",
885+
"Octal" => "new_octal",
886+
"Pointer" => "new_pointer",
887+
"Binary" => "new_binary",
888+
"LowerHex" => "new_lower_hex",
889+
"UpperHex" => "new_upper_hex",
890+
_ => unreachable!(),
891+
};
880892

881-
let path = ecx.std_path(&[sym::fmt, Symbol::intern(trait_), sym::fmt]);
882-
let format_fn = ecx.path_global(sp, path);
883-
let path = ecx.std_path(&[sym::fmt, sym::ArgumentV1, sym::new]);
884-
ecx.expr_call_global(macsp, path, vec![arg, ecx.expr_path(format_fn)])
893+
let path = ecx.std_path(&[sym::fmt, sym::ArgumentV1, Symbol::intern(new_fn_name)]);
894+
ecx.expr_call_global(sp, path, vec![arg])
885895
}
886896
}
887897

‎compiler/rustc_mir_transform/src/function_item_references.rs

+21-41
Original file line numberDiff line numberDiff line change
@@ -43,54 +43,28 @@ impl<'tcx> Visitor<'tcx> for FunctionItemRefChecker<'_, 'tcx> {
4343
} = &terminator.kind
4444
{
4545
let source_info = *self.body.source_info(location);
46-
// Only handle function calls outside macros
47-
if !source_info.span.from_expansion() {
48-
let func_ty = func.ty(self.body, self.tcx);
49-
if let ty::FnDef(def_id, substs_ref) = *func_ty.kind() {
50-
// Handle calls to `transmute`
51-
if self.tcx.is_diagnostic_item(sym::transmute, def_id) {
52-
let arg_ty = args[0].ty(self.body, self.tcx);
53-
for generic_inner_ty in arg_ty.walk() {
54-
if let GenericArgKind::Type(inner_ty) = generic_inner_ty.unpack() {
55-
if let Some((fn_id, fn_substs)) =
56-
FunctionItemRefChecker::is_fn_ref(inner_ty)
57-
{
58-
let span = self.nth_arg_span(&args, 0);
59-
self.emit_lint(fn_id, fn_substs, source_info, span);
60-
}
46+
let func_ty = func.ty(self.body, self.tcx);
47+
if let ty::FnDef(def_id, substs_ref) = *func_ty.kind() {
48+
// Handle calls to `transmute`
49+
if self.tcx.is_diagnostic_item(sym::transmute, def_id) {
50+
let arg_ty = args[0].ty(self.body, self.tcx);
51+
for generic_inner_ty in arg_ty.walk() {
52+
if let GenericArgKind::Type(inner_ty) = generic_inner_ty.unpack() {
53+
if let Some((fn_id, fn_substs)) =
54+
FunctionItemRefChecker::is_fn_ref(inner_ty)
55+
{
56+
let span = self.nth_arg_span(&args, 0);
57+
self.emit_lint(fn_id, fn_substs, source_info, span);
6158
}
6259
}
63-
} else {
64-
self.check_bound_args(def_id, substs_ref, &args, source_info);
6560
}
61+
} else {
62+
self.check_bound_args(def_id, substs_ref, &args, source_info);
6663
}
6764
}
6865
}
6966
self.super_terminator(terminator, location);
7067
}
71-
72-
/// Emits a lint for function references formatted with `fmt::Pointer::fmt` by macros. These
73-
/// cases are handled as operands instead of call terminators to avoid any dependence on
74-
/// unstable, internal formatting details like whether `fmt` is called directly or not.
75-
fn visit_operand(&mut self, operand: &Operand<'tcx>, location: Location) {
76-
let source_info = *self.body.source_info(location);
77-
if source_info.span.from_expansion() {
78-
let op_ty = operand.ty(self.body, self.tcx);
79-
if let ty::FnDef(def_id, substs_ref) = *op_ty.kind() {
80-
if self.tcx.is_diagnostic_item(sym::pointer_trait_fmt, def_id) {
81-
let param_ty = substs_ref.type_at(0);
82-
if let Some((fn_id, fn_substs)) = FunctionItemRefChecker::is_fn_ref(param_ty) {
83-
// The operand's ctxt wouldn't display the lint since it's inside a macro so
84-
// we have to use the callsite's ctxt.
85-
let callsite_ctxt = source_info.span.source_callsite().ctxt();
86-
let span = source_info.span.with_ctxt(callsite_ctxt);
87-
self.emit_lint(fn_id, fn_substs, source_info, span);
88-
}
89-
}
90-
}
91-
}
92-
self.super_operand(operand, location);
93-
}
9468
}
9569

9670
impl<'tcx> FunctionItemRefChecker<'_, 'tcx> {
@@ -120,7 +94,13 @@ impl<'tcx> FunctionItemRefChecker<'_, 'tcx> {
12094
if let Some((fn_id, fn_substs)) =
12195
FunctionItemRefChecker::is_fn_ref(subst_ty)
12296
{
123-
let span = self.nth_arg_span(args, arg_num);
97+
let mut span = self.nth_arg_span(args, arg_num);
98+
if span.from_expansion() {
99+
// The operand's ctxt wouldn't display the lint since it's inside a macro so
100+
// we have to use the callsite's ctxt.
101+
let callsite_ctxt = span.source_callsite().ctxt();
102+
span = span.with_ctxt(callsite_ctxt);
103+
}
124104
self.emit_lint(fn_id, fn_substs, source_info, span);
125105
}
126106
}

‎library/core/src/fmt/mod.rs

+22
Original file line numberDiff line numberDiff line change
@@ -308,9 +308,21 @@ static USIZE_MARKER: fn(&usize, &mut Formatter<'_>) -> Result = |ptr, _| {
308308
loop {}
309309
};
310310

311+
macro_rules! arg_new {
312+
($f: ident, $t: ident) => {
313+
#[doc(hidden)]
314+
#[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")]
315+
#[inline]
316+
pub fn $f<'b, T: $t>(x: &'b T) -> ArgumentV1<'_> {
317+
Self::new(x, $t::fmt)
318+
}
319+
};
320+
}
321+
311322
impl<'a> ArgumentV1<'a> {
312323
#[doc(hidden)]
313324
#[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")]
325+
#[inline]
314326
pub fn new<'b, T>(x: &'b T, f: fn(&T, &mut Formatter<'_>) -> Result) -> ArgumentV1<'b> {
315327
// SAFETY: `mem::transmute(x)` is safe because
316328
// 1. `&'b T` keeps the lifetime it originated with `'b`
@@ -323,6 +335,16 @@ impl<'a> ArgumentV1<'a> {
323335
unsafe { ArgumentV1 { formatter: mem::transmute(f), value: mem::transmute(x) } }
324336
}
325337

338+
arg_new!(new_display, Display);
339+
arg_new!(new_debug, Debug);
340+
arg_new!(new_octal, Octal);
341+
arg_new!(new_lower_hex, LowerHex);
342+
arg_new!(new_upper_hex, UpperHex);
343+
arg_new!(new_pointer, Pointer);
344+
arg_new!(new_binary, Binary);
345+
arg_new!(new_lower_exp, LowerExp);
346+
arg_new!(new_upper_exp, UpperExp);
347+
326348
#[doc(hidden)]
327349
#[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")]
328350
pub fn from_usize(x: &usize) -> ArgumentV1<'_> {

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

+4-4
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| | =
33-
33| | ||
32+
32| 1| =
33+
33| 1| ||
3434
34| 0| {
3535
35| 0| let mut countdown = 0;
3636
36| 0| if is_false {
@@ -116,8 +116,8 @@
116116
116| 1|
117117
117| 1| let
118118
118| 1| _unused_closure
119-
119| 1| =
120-
120| 1| |
119+
119| | =
120+
120| | |
121121
121| | mut countdown
122122
122| | |
123123
123| 0| {

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

+4-4
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,12 @@
1919
18| 2| println!("used_only_from_bin_crate_generic_function with {:?}", arg);
2020
19| 2|}
2121
------------------
22-
| used_crate::used_only_from_bin_crate_generic_function::<&alloc::vec::Vec<i32>>:
22+
| used_crate::used_only_from_bin_crate_generic_function::<&str>:
2323
| 17| 1|pub fn used_only_from_bin_crate_generic_function<T: Debug>(arg: T) {
2424
| 18| 1| println!("used_only_from_bin_crate_generic_function with {:?}", arg);
2525
| 19| 1|}
2626
------------------
27-
| used_crate::used_only_from_bin_crate_generic_function::<&str>:
27+
| used_crate::used_only_from_bin_crate_generic_function::<&alloc::vec::Vec<i32>>:
2828
| 17| 1|pub fn used_only_from_bin_crate_generic_function<T: Debug>(arg: T) {
2929
| 18| 1| println!("used_only_from_bin_crate_generic_function with {:?}", arg);
3030
| 19| 1|}
@@ -36,12 +36,12 @@
3636
22| 2| println!("used_only_from_this_lib_crate_generic_function with {:?}", arg);
3737
23| 2|}
3838
------------------
39-
| used_crate::used_only_from_this_lib_crate_generic_function::<alloc::vec::Vec<i32>>:
39+
| used_crate::used_only_from_this_lib_crate_generic_function::<&str>:
4040
| 21| 1|pub fn used_only_from_this_lib_crate_generic_function<T: Debug>(arg: T) {
4141
| 22| 1| println!("used_only_from_this_lib_crate_generic_function with {:?}", arg);
4242
| 23| 1|}
4343
------------------
44-
| used_crate::used_only_from_this_lib_crate_generic_function::<&str>:
44+
| used_crate::used_only_from_this_lib_crate_generic_function::<alloc::vec::Vec<i32>>:
4545
| 21| 1|pub fn used_only_from_this_lib_crate_generic_function<T: Debug>(arg: T) {
4646
| 22| 1| println!("used_only_from_this_lib_crate_generic_function with {:?}", arg);
4747
| 23| 1|}

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

+4-4
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,12 @@
4242
40| 2| println!("used_only_from_bin_crate_generic_function with {:?}", arg);
4343
41| 2|}
4444
------------------
45-
| used_inline_crate::used_only_from_bin_crate_generic_function::<&str>:
45+
| used_inline_crate::used_only_from_bin_crate_generic_function::<&alloc::vec::Vec<i32>>:
4646
| 39| 1|pub fn used_only_from_bin_crate_generic_function<T: Debug>(arg: T) {
4747
| 40| 1| println!("used_only_from_bin_crate_generic_function with {:?}", arg);
4848
| 41| 1|}
4949
------------------
50-
| used_inline_crate::used_only_from_bin_crate_generic_function::<&alloc::vec::Vec<i32>>:
50+
| used_inline_crate::used_only_from_bin_crate_generic_function::<&str>:
5151
| 39| 1|pub fn used_only_from_bin_crate_generic_function<T: Debug>(arg: T) {
5252
| 40| 1| println!("used_only_from_bin_crate_generic_function with {:?}", arg);
5353
| 41| 1|}
@@ -61,12 +61,12 @@
6161
46| 4| println!("used_only_from_this_lib_crate_generic_function with {:?}", arg);
6262
47| 4|}
6363
------------------
64-
| used_inline_crate::used_only_from_this_lib_crate_generic_function::<&str>:
64+
| used_inline_crate::used_only_from_this_lib_crate_generic_function::<alloc::vec::Vec<i32>>:
6565
| 45| 2|pub fn used_only_from_this_lib_crate_generic_function<T: Debug>(arg: T) {
6666
| 46| 2| println!("used_only_from_this_lib_crate_generic_function with {:?}", arg);
6767
| 47| 2|}
6868
------------------
69-
| used_inline_crate::used_only_from_this_lib_crate_generic_function::<alloc::vec::Vec<i32>>:
69+
| used_inline_crate::used_only_from_this_lib_crate_generic_function::<&str>:
7070
| 45| 2|pub fn used_only_from_this_lib_crate_generic_function<T: Debug>(arg: T) {
7171
| 46| 2| println!("used_only_from_this_lib_crate_generic_function with {:?}", arg);
7272
| 47| 2|}

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

+1-2
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@ LL | bug!();
1818
error: unexpected token: `{
1919
let res =
2020
::alloc::fmt::format(::core::fmt::Arguments::new_v1(&[""],
21-
&[::core::fmt::ArgumentV1::new(&"u8",
22-
::core::fmt::Display::fmt)]));
21+
&[::core::fmt::ArgumentV1::new_display(&"u8")]));
2322
res
2423
}.as_str()`
2524
--> $DIR/key-value-expansion.rs:48:23

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

+1-1
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, _#19t, extern "rust-call" fn(()), _#20t]]`
12+
found closure `[mod1::f<T>::{closure#0} closure_substs=(unavailable) substs=[T, _#16t, extern "rust-call" fn(()), _#15t]]`
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

+1-1
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, _#19t, extern "rust-call" fn(()), _#20t]]`
12+
found closure `[f<T>::{closure#0} closure_substs=(unavailable) substs=[T, _#16t, extern "rust-call" fn(()), _#15t]]`
1313
help: use parentheses to call this closure
1414
|
1515
LL | let c1 : () = c();

‎src/test/ui/fmt/ifmt-unimpl.stderr

+5
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@ LL | format!("{:X}", "3");
55
| ^^^ the trait `UpperHex` is not implemented for `str`
66
|
77
= note: required because of the requirements on the impl of `UpperHex` for `&str`
8+
note: required by a bound in `ArgumentV1::<'a>::new_upper_hex`
9+
--> $SRC_DIR/core/src/fmt/mod.rs:LL:COL
10+
|
11+
LL | arg_new!(new_upper_hex, UpperHex);
12+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `ArgumentV1::<'a>::new_upper_hex`
813
= note: this error originates in the macro `$crate::__export::format_args` (in Nightly builds, run with -Z macro-backtrace for more info)
914

1015
error: aborting due to previous error

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

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
error[E0282]: type annotations needed
2-
--> $DIR/issue-69455.rs:29:5
2+
--> $DIR/issue-69455.rs:29:20
33
|
44
LL | type Output;
55
| ------------ `<Self as Test<Rhs>>::Output` defined here
66
...
77
LL | println!("{}", 23u64.test(xs.iter().sum()));
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`
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_display`
1212
|
1313
= note: this error originates in the macro `$crate::format_args_nl` (in Nightly builds, run with -Z macro-backtrace for more info)
1414

‎src/tools/clippy/clippy_lints/src/index_refutable_slice.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use clippy_utils::higher::IfLet;
44
use clippy_utils::ty::is_copy;
55
use clippy_utils::{is_expn_of, is_lint_allowed, meets_msrv, msrvs, path_to_local};
66
use if_chain::if_chain;
7-
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
7+
use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
88
use rustc_errors::Applicability;
99
use rustc_hir as hir;
1010
use rustc_hir::intravisit::{self, Visitor};
@@ -92,9 +92,9 @@ impl<'tcx> LateLintPass<'tcx> for IndexRefutableSlice {
9292
extract_msrv_attr!(LateContext);
9393
}
9494

95-
fn find_slice_values(cx: &LateContext<'_>, pat: &hir::Pat<'_>) -> FxHashMap<hir::HirId, SliceLintInformation> {
95+
fn find_slice_values(cx: &LateContext<'_>, pat: &hir::Pat<'_>) -> FxIndexMap<hir::HirId, SliceLintInformation> {
9696
let mut removed_pat: FxHashSet<hir::HirId> = FxHashSet::default();
97-
let mut slices: FxHashMap<hir::HirId, SliceLintInformation> = FxHashMap::default();
97+
let mut slices: FxIndexMap<hir::HirId, SliceLintInformation> = FxIndexMap::default();
9898
pat.walk_always(|pat| {
9999
if let hir::PatKind::Binding(binding, value_hir_id, ident, sub_pat) = pat.kind {
100100
// We'll just ignore mut and ref mut for simplicity sake right now
@@ -208,10 +208,10 @@ impl SliceLintInformation {
208208

209209
fn filter_lintable_slices<'a, 'tcx>(
210210
cx: &'a LateContext<'tcx>,
211-
slice_lint_info: FxHashMap<hir::HirId, SliceLintInformation>,
211+
slice_lint_info: FxIndexMap<hir::HirId, SliceLintInformation>,
212212
max_suggested_slice: u64,
213213
scope: &'tcx hir::Expr<'tcx>,
214-
) -> FxHashMap<hir::HirId, SliceLintInformation> {
214+
) -> FxIndexMap<hir::HirId, SliceLintInformation> {
215215
let mut visitor = SliceIndexLintingVisitor {
216216
cx,
217217
slice_lint_info,
@@ -225,7 +225,7 @@ fn filter_lintable_slices<'a, 'tcx>(
225225

226226
struct SliceIndexLintingVisitor<'a, 'tcx> {
227227
cx: &'a LateContext<'tcx>,
228-
slice_lint_info: FxHashMap<hir::HirId, SliceLintInformation>,
228+
slice_lint_info: FxIndexMap<hir::HirId, SliceLintInformation>,
229229
max_suggested_slice: u64,
230230
}
231231

‎src/tools/clippy/clippy_utils/src/macros.rs

+16-6
Original file line numberDiff line numberDiff line change
@@ -339,15 +339,13 @@ impl<'tcx> FormatArgsExpn<'tcx> {
339339
expr_visitor_no_bodies(|e| {
340340
// if we're still inside of the macro definition...
341341
if e.span.ctxt() == expr.span.ctxt() {
342-
// ArgumnetV1::new(<value>, <format_trait>::fmt)
342+
// ArgumnetV1::new_<format_trait>(<value>)
343343
if_chain! {
344-
if let ExprKind::Call(callee, [val, fmt_path]) = e.kind;
344+
if let ExprKind::Call(callee, [val]) = e.kind;
345345
if let ExprKind::Path(QPath::TypeRelative(ty, seg)) = callee.kind;
346-
if seg.ident.name == sym::new;
347346
if let hir::TyKind::Path(QPath::Resolved(_, path)) = ty.kind;
348347
if path.segments.last().unwrap().ident.name == sym::ArgumentV1;
349-
if let ExprKind::Path(QPath::Resolved(_, path)) = fmt_path.kind;
350-
if let [.., fmt_trait, _fmt] = path.segments;
348+
if seg.ident.name.as_str().starts_with("new_");
351349
then {
352350
let val_idx = if_chain! {
353351
if val.span.ctxt() == expr.span.ctxt();
@@ -361,7 +359,19 @@ impl<'tcx> FormatArgsExpn<'tcx> {
361359
formatters.len()
362360
}
363361
};
364-
formatters.push((val_idx, fmt_trait.ident.name));
362+
let fmt_trait = match seg.ident.name.as_str() {
363+
"new_display" => "Display",
364+
"new_debug" => "Debug",
365+
"new_lower_exp" => "LowerExp",
366+
"new_upper_exp" => "UpperExp",
367+
"new_octal" => "Octal",
368+
"new_pointer" => "Pointer",
369+
"new_binary" => "Binary",
370+
"new_lower_hex" => "LowerHex",
371+
"new_upper_hex" => "UpperHex",
372+
_ => unreachable!(),
373+
};
374+
formatters.push((val_idx, Symbol::intern(fmt_trait)));
365375
}
366376
}
367377
if let ExprKind::Struct(QPath::Resolved(_, path), ..) = e.kind {

0 commit comments

Comments
 (0)