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
This commit is contained in:
bors 2022-01-31 00:04:46 +00:00
commit e58e7b10e1
13 changed files with 104 additions and 78 deletions

View File

@ -877,11 +877,21 @@ fn format_arg(
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])
}
}

View File

@ -43,54 +43,28 @@ fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location
} = &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> {
@ -120,7 +94,13 @@ fn check_bound_args(
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);
}
}

View File

@ -308,9 +308,21 @@ pub unsafe fn new() -> Self {
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`
@ -323,6 +335,16 @@ pub fn new<'b, T>(x: &'b T, f: fn(&T, &mut Formatter<'_>) -> Result) -> Argument
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<'_> {

View File

@ -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 {
@ -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| {

View File

@ -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|}
@ -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|}

View File

@ -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|}
@ -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|}

View File

@ -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

View File

@ -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();

View File

@ -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();

View File

@ -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

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)

View File

@ -4,7 +4,7 @@
use clippy_utils::ty::is_copy;
use clippy_utils::{is_expn_of, is_lint_allowed, meets_msrv, msrvs, path_to_local};
use if_chain::if_chain;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::intravisit::{self, Visitor};
@ -92,9 +92,9 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
extract_msrv_attr!(LateContext);
}
fn find_slice_values(cx: &LateContext<'_>, pat: &hir::Pat<'_>) -> FxHashMap<hir::HirId, SliceLintInformation> {
fn find_slice_values(cx: &LateContext<'_>, pat: &hir::Pat<'_>) -> FxIndexMap<hir::HirId, SliceLintInformation> {
let mut removed_pat: FxHashSet<hir::HirId> = FxHashSet::default();
let mut slices: FxHashMap<hir::HirId, SliceLintInformation> = FxHashMap::default();
let mut slices: FxIndexMap<hir::HirId, SliceLintInformation> = FxIndexMap::default();
pat.walk_always(|pat| {
if let hir::PatKind::Binding(binding, value_hir_id, ident, sub_pat) = pat.kind {
// We'll just ignore mut and ref mut for simplicity sake right now
@ -208,10 +208,10 @@ fn new(ident: Ident, needs_ref: bool) -> Self {
fn filter_lintable_slices<'a, 'tcx>(
cx: &'a LateContext<'tcx>,
slice_lint_info: FxHashMap<hir::HirId, SliceLintInformation>,
slice_lint_info: FxIndexMap<hir::HirId, SliceLintInformation>,
max_suggested_slice: u64,
scope: &'tcx hir::Expr<'tcx>,
) -> FxHashMap<hir::HirId, SliceLintInformation> {
) -> FxIndexMap<hir::HirId, SliceLintInformation> {
let mut visitor = SliceIndexLintingVisitor {
cx,
slice_lint_info,
@ -225,7 +225,7 @@ fn filter_lintable_slices<'a, 'tcx>(
struct SliceIndexLintingVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
slice_lint_info: FxHashMap<hir::HirId, SliceLintInformation>,
slice_lint_info: FxIndexMap<hir::HirId, SliceLintInformation>,
max_suggested_slice: u64,
}

View File

@ -339,15 +339,13 @@ pub fn parse(cx: &LateContext<'_>, expr: &'tcx Expr<'tcx>) -> Option<Self> {
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();
@ -361,7 +359,19 @@ pub fn parse(cx: &LateContext<'_>, expr: &'tcx Expr<'tcx>) -> Option<Self> {
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 {