Skip to content

Commit

Permalink
Create core::fmt::ArgumentV1 with generics instead of fn pointer
Browse files Browse the repository at this point in the history
  • Loading branch information
@nbdd0121
nbdd0121 committed Jan 29, 2022
1 parent bfe1564 commit a832f5f
Show file tree
Hide file tree
Showing 12 changed files with 98 additions and 72 deletions.
18 changes: 14 additions & 4 deletions compiler/rustc_builtin_macros/src/format.rs
View file
Expand Up @@ -877,11 +877,21 @@ impl<'a, 'b> Context<'a, 'b> {
return ecx.expr_call_global(macsp, path, vec![arg]);
}
};
let new_fn_name = match trait_ {
"Display" => "new_display",
"Debug" => "new_debug",
"LowerExp" => "new_lower_exp",
"UpperExp" => "new_upper_exp",
"Octal" => "new_octal",
"Pointer" => "new_pointer",
"Binary" => "new_binary",
"LowerHex" => "new_lower_hex",
"UpperHex" => "new_upper_hex",
_ => unreachable!(),
};

let path = ecx.std_path(&[sym::fmt, Symbol::intern(trait_), sym::fmt]);
let format_fn = ecx.path_global(sp, path);
let path = ecx.std_path(&[sym::fmt, sym::ArgumentV1, sym::new]);
ecx.expr_call_global(macsp, path, vec![arg, ecx.expr_path(format_fn)])
let path = ecx.std_path(&[sym::fmt, sym::ArgumentV1, Symbol::intern(new_fn_name)]);
ecx.expr_call_global(sp, path, vec![arg])
}
}

Expand Down
62 changes: 21 additions & 41 deletions compiler/rustc_mir_transform/src/function_item_references.rs
View file
Expand Up @@ -42,54 +42,28 @@ impl<'tcx> Visitor<'tcx> for FunctionItemRefChecker<'_, 'tcx> {
} = &terminator.kind
{
let source_info = *self.body.source_info(location);
// Only handle function calls outside macros
if !source_info.span.from_expansion() {
let func_ty = func.ty(self.body, self.tcx);
if let ty::FnDef(def_id, substs_ref) = *func_ty.kind() {
// Handle calls to `transmute`
if self.tcx.is_diagnostic_item(sym::transmute, def_id) {
let arg_ty = args[0].ty(self.body, self.tcx);
for generic_inner_ty in arg_ty.walk() {
if let GenericArgKind::Type(inner_ty) = generic_inner_ty.unpack() {
if let Some((fn_id, fn_substs)) =
FunctionItemRefChecker::is_fn_ref(inner_ty)
{
let span = self.nth_arg_span(&args, 0);
self.emit_lint(fn_id, fn_substs, source_info, span);
}
let func_ty = func.ty(self.body, self.tcx);
if let ty::FnDef(def_id, substs_ref) = *func_ty.kind() {
// Handle calls to `transmute`
if self.tcx.is_diagnostic_item(sym::transmute, def_id) {
let arg_ty = args[0].ty(self.body, self.tcx);
for generic_inner_ty in arg_ty.walk() {
if let GenericArgKind::Type(inner_ty) = generic_inner_ty.unpack() {
if let Some((fn_id, fn_substs)) =
FunctionItemRefChecker::is_fn_ref(inner_ty)
{
let span = self.nth_arg_span(&args, 0);
self.emit_lint(fn_id, fn_substs, source_info, span);
}
}
} else {
self.check_bound_args(def_id, substs_ref, &args, source_info);
}
} else {
self.check_bound_args(def_id, substs_ref, &args, source_info);
}
}
}
self.super_terminator(terminator, location);
}

/// Emits a lint for function references formatted with `fmt::Pointer::fmt` by macros. These
/// cases are handled as operands instead of call terminators to avoid any dependence on
/// unstable, internal formatting details like whether `fmt` is called directly or not.
fn visit_operand(&mut self, operand: &Operand<'tcx>, location: Location) {
let source_info = *self.body.source_info(location);
if source_info.span.from_expansion() {
let op_ty = operand.ty(self.body, self.tcx);
if let ty::FnDef(def_id, substs_ref) = *op_ty.kind() {
if self.tcx.is_diagnostic_item(sym::pointer_trait_fmt, def_id) {
let param_ty = substs_ref.type_at(0);
if let Some((fn_id, fn_substs)) = FunctionItemRefChecker::is_fn_ref(param_ty) {
// The operand's ctxt wouldn't display the lint since it's inside a macro so
// we have to use the callsite's ctxt.
let callsite_ctxt = source_info.span.source_callsite().ctxt();
let span = source_info.span.with_ctxt(callsite_ctxt);
self.emit_lint(fn_id, fn_substs, source_info, span);
}
}
}
}
self.super_operand(operand, location);
}
}

impl<'tcx> FunctionItemRefChecker<'_, 'tcx> {
Expand Down Expand Up @@ -119,7 +93,13 @@ impl<'tcx> FunctionItemRefChecker<'_, 'tcx> {
if let Some((fn_id, fn_substs)) =
FunctionItemRefChecker::is_fn_ref(subst_ty)
{
let span = self.nth_arg_span(args, arg_num);
let mut span = self.nth_arg_span(args, arg_num);
if span.from_expansion() {
// The operand's ctxt wouldn't display the lint since it's inside a macro so
// we have to use the callsite's ctxt.
let callsite_ctxt = span.source_callsite().ctxt();
span = span.with_ctxt(callsite_ctxt);
}
self.emit_lint(fn_id, fn_substs, source_info, span);
}
}
Expand Down
22 changes: 22 additions & 0 deletions library/core/src/fmt/mod.rs
View file
Expand Up @@ -308,9 +308,21 @@ static USIZE_MARKER: fn(&usize, &mut Formatter<'_>) -> Result = |ptr, _| {
loop {}
};

macro_rules! arg_new {
($f: ident, $t: ident) => {
#[doc(hidden)]
#[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")]
#[inline]
pub fn $f<'b, T: $t>(x: &'b T) -> ArgumentV1<'_> {
Self::new(x, $t::fmt)
}
};
}

impl<'a> ArgumentV1<'a> {
#[doc(hidden)]
#[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")]
#[inline]
pub fn new<'b, T>(x: &'b T, f: fn(&T, &mut Formatter<'_>) -> Result) -> ArgumentV1<'b> {
// SAFETY: `mem::transmute(x)` is safe because
// 1. `&'b T` keeps the lifetime it originated with `'b`
Expand All @@ -323,6 +335,16 @@ impl<'a> ArgumentV1<'a> {
unsafe { ArgumentV1 { formatter: mem::transmute(f), value: mem::transmute(x) } }
}

arg_new!(new_display, Display);
arg_new!(new_debug, Debug);
arg_new!(new_octal, Octal);
arg_new!(new_lower_hex, LowerHex);
arg_new!(new_upper_hex, UpperHex);
arg_new!(new_pointer, Pointer);
arg_new!(new_binary, Binary);
arg_new!(new_lower_exp, LowerExp);
arg_new!(new_upper_exp, UpperExp);

#[doc(hidden)]
#[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")]
pub fn from_usize(x: &usize) -> ArgumentV1<'_> {
Expand Down
8 changes: 4 additions & 4 deletions src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.closure.txt
View file
Expand Up @@ -29,8 +29,8 @@
29| 1| some_string = Some(String::from("the string content"));
30| 1| let
31| 1| a
32| | =
33| | ||
32| 1| =
33| 1| ||
34| 0| {
35| 0| let mut countdown = 0;
36| 0| if is_false {
Expand Down Expand Up @@ -116,8 +116,8 @@
116| 1|
117| 1| let
118| 1| _unused_closure
119| 1| =
120| 1| |
119| | =
120| | |
121| | mut countdown
122| | |
123| 0| {
Expand Down
8 changes: 4 additions & 4 deletions src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt
View file
Expand Up @@ -19,12 +19,12 @@
18| 2| println!("used_only_from_bin_crate_generic_function with {:?}", arg);
19| 2|}
------------------
| used_crate::used_only_from_bin_crate_generic_function::<&alloc::vec::Vec<i32>>:
| used_crate::used_only_from_bin_crate_generic_function::<&str>:
| 17| 1|pub fn used_only_from_bin_crate_generic_function<T: Debug>(arg: T) {
| 18| 1| println!("used_only_from_bin_crate_generic_function with {:?}", arg);
| 19| 1|}
------------------
| used_crate::used_only_from_bin_crate_generic_function::<&str>:
| used_crate::used_only_from_bin_crate_generic_function::<&alloc::vec::Vec<i32>>:
| 17| 1|pub fn used_only_from_bin_crate_generic_function<T: Debug>(arg: T) {
| 18| 1| println!("used_only_from_bin_crate_generic_function with {:?}", arg);
| 19| 1|}
Expand All @@ -36,12 +36,12 @@
22| 2| println!("used_only_from_this_lib_crate_generic_function with {:?}", arg);
23| 2|}
------------------
| used_crate::used_only_from_this_lib_crate_generic_function::<alloc::vec::Vec<i32>>:
| used_crate::used_only_from_this_lib_crate_generic_function::<&str>:
| 21| 1|pub fn used_only_from_this_lib_crate_generic_function<T: Debug>(arg: T) {
| 22| 1| println!("used_only_from_this_lib_crate_generic_function with {:?}", arg);
| 23| 1|}
------------------
| used_crate::used_only_from_this_lib_crate_generic_function::<&str>:
| used_crate::used_only_from_this_lib_crate_generic_function::<alloc::vec::Vec<i32>>:
| 21| 1|pub fn used_only_from_this_lib_crate_generic_function<T: Debug>(arg: T) {
| 22| 1| println!("used_only_from_this_lib_crate_generic_function with {:?}", arg);
| 23| 1|}
Expand Down
8 changes: 4 additions & 4 deletions src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_inline_crate.txt
View file
Expand Up @@ -42,12 +42,12 @@
40| 2| println!("used_only_from_bin_crate_generic_function with {:?}", arg);
41| 2|}
------------------
| used_inline_crate::used_only_from_bin_crate_generic_function::<&str>:
| used_inline_crate::used_only_from_bin_crate_generic_function::<&alloc::vec::Vec<i32>>:
| 39| 1|pub fn used_only_from_bin_crate_generic_function<T: Debug>(arg: T) {
| 40| 1| println!("used_only_from_bin_crate_generic_function with {:?}", arg);
| 41| 1|}
------------------
| used_inline_crate::used_only_from_bin_crate_generic_function::<&alloc::vec::Vec<i32>>:
| used_inline_crate::used_only_from_bin_crate_generic_function::<&str>:
| 39| 1|pub fn used_only_from_bin_crate_generic_function<T: Debug>(arg: T) {
| 40| 1| println!("used_only_from_bin_crate_generic_function with {:?}", arg);
| 41| 1|}
Expand All @@ -61,12 +61,12 @@
46| 4| println!("used_only_from_this_lib_crate_generic_function with {:?}", arg);
47| 4|}
------------------
| used_inline_crate::used_only_from_this_lib_crate_generic_function::<&str>:
| used_inline_crate::used_only_from_this_lib_crate_generic_function::<alloc::vec::Vec<i32>>:
| 45| 2|pub fn used_only_from_this_lib_crate_generic_function<T: Debug>(arg: T) {
| 46| 2| println!("used_only_from_this_lib_crate_generic_function with {:?}", arg);
| 47| 2|}
------------------
| used_inline_crate::used_only_from_this_lib_crate_generic_function::<alloc::vec::Vec<i32>>:
| used_inline_crate::used_only_from_this_lib_crate_generic_function::<&str>:
| 45| 2|pub fn used_only_from_this_lib_crate_generic_function<T: Debug>(arg: T) {
| 46| 2| println!("used_only_from_this_lib_crate_generic_function with {:?}", arg);
| 47| 2|}
Expand Down
3 changes: 1 addition & 2 deletions src/test/ui/attributes/key-value-expansion.stderr
View file
Expand Up @@ -18,8 +18,7 @@ LL | bug!();
error: unexpected token: `{
let res =
::alloc::fmt::format(::core::fmt::Arguments::new_v1(&[""],
&[::core::fmt::ArgumentV1::new(&"u8",
::core::fmt::Display::fmt)]));
&[::core::fmt::ArgumentV1::new_display(&"u8")]));
res
}.as_str()`
--> $DIR/key-value-expansion.rs:48:23
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/closures/print/closure-print-generic-trim-off-verbose-2.stderr
View file
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, _#19t, extern "rust-call" fn(()), _#20t]]`
found closure `[mod1::f<T>::{closure#0} closure_substs=(unavailable) substs=[T, _#16t, extern "rust-call" fn(()), _#15t]]`
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
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, _#19t, extern "rust-call" fn(()), _#20t]]`
found closure `[f<T>::{closure#0} closure_substs=(unavailable) substs=[T, _#16t, extern "rust-call" fn(()), _#15t]]`
help: use parentheses to call this closure
|
LL | let c1 : () = c();
Expand Down
5 changes: 5 additions & 0 deletions src/test/ui/fmt/ifmt-unimpl.stderr
View file
Expand Up @@ -5,6 +5,11 @@ LL | format!("{:X}", "3");
| ^^^ the trait `UpperHex` is not implemented for `str`
|
= note: required because of the requirements on the impl of `UpperHex` for `&str`
note: required by a bound in `ArgumentV1::<'a>::new_upper_hex`
--> $SRC_DIR/core/src/fmt/mod.rs:LL:COL
|
LL | arg_new!(new_upper_hex, UpperHex);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `ArgumentV1::<'a>::new_upper_hex`
= note: this error originates in the macro `$crate::__export::format_args` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to previous error
Expand Down
10 changes: 5 additions & 5 deletions src/test/ui/issues/issue-69455.stderr
View file
@@ -1,14 +1,14 @@
error[E0282]: type annotations needed
--> $DIR/issue-69455.rs:29:5
--> $DIR/issue-69455.rs:29:20
|
LL | type Output;
| ------------ `<Self as Test<Rhs>>::Output` defined here
...
LL | println!("{}", 23u64.test(xs.iter().sum()));
| ^^^^^^^^^^^^^^^---------------------------^
| | |
| | this method call resolves to `<Self as Test<Rhs>>::Output`
| cannot infer type for type parameter `T` declared on the associated function `new`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| this method call resolves to `<Self as Test<Rhs>>::Output`
| cannot infer type for type parameter `T` declared on the associated function `new_display`
|
= note: this error originates in the macro `$crate::format_args_nl` (in Nightly builds, run with -Z macro-backtrace for more info)

Expand Down
22 changes: 16 additions & 6 deletions src/tools/clippy/clippy_utils/src/macros.rs
View file
Expand Up @@ -339,15 +339,13 @@ impl<'tcx> FormatArgsExpn<'tcx> {
expr_visitor_no_bodies(|e| {
// if we're still inside of the macro definition...
if e.span.ctxt() == expr.span.ctxt() {
// ArgumnetV1::new(<value>, <format_trait>::fmt)
// ArgumnetV1::new_<format_trait>(<value>)
if_chain! {
if let ExprKind::Call(callee, [val, fmt_path]) = e.kind;
if let ExprKind::Call(callee, [val]) = e.kind;
if let ExprKind::Path(QPath::TypeRelative(ty, seg)) = callee.kind;
if seg.ident.name == sym::new;
if let hir::TyKind::Path(QPath::Resolved(_, path)) = ty.kind;
if path.segments.last().unwrap().ident.name == sym::ArgumentV1;
if let ExprKind::Path(QPath::Resolved(_, path)) = fmt_path.kind;
if let [.., fmt_trait, _fmt] = path.segments;
if seg.ident.name.as_str().starts_with("new_");
then {
let val_idx = if_chain! {
if val.span.ctxt() == expr.span.ctxt();
Expand All @@ -361,7 +359,19 @@ impl<'tcx> FormatArgsExpn<'tcx> {
formatters.len()
}
};
formatters.push((val_idx, fmt_trait.ident.name));
let fmt_trait = match seg.ident.name.as_str() {
"new_display" => "Display",
"new_debug" => "Debug",
"new_lower_exp" => "LowerExp",
"new_upper_exp" => "UpperExp",
"new_octal" => "Octal",
"new_pointer" => "Pointer",
"new_binary" => "Binary",
"new_lower_hex" => "LowerHex",
"new_upper_hex" => "UpperHex",
_ => unreachable!(),
};
formatters.push((val_idx, Symbol::intern(fmt_trait)));
}
}
if let ExprKind::Struct(QPath::Resolved(_, path), ..) = e.kind {
Expand Down

0 comments on commit a832f5f

Please sign in to comment.