Skip to content

Commit a832f5f

Browse files
nbdd0121nbdd0121
committed
Create core::fmt::ArgumentV1 with generics instead of fn pointer
1 parent bfe1564 commit a832f5f

File tree

12 files changed

+98
-72
lines changed

12 files changed

+98
-72
lines changed

Diff for: ‎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

Diff for: ‎compiler/rustc_mir_transform/src/function_item_references.rs

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

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

Diff for: ‎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<'_> {

Diff for: ‎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| {

Diff for: ‎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|}

Diff for: ‎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|}

Diff for: ‎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

Diff for: ‎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();

Diff for: ‎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();

Diff for: ‎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

Diff for: ‎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

Diff for: ‎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)