Skip to content

Commit 67365d6

Browse files
borsbors
committed
Auto merge of #89139 - camsteffen:write-perf, r=Mark-Simulacrum
Use ZST for fmt unsafety as suggested here - #83302 (comment).
2 parents 30278d3 + 2efa9d7 commit 67365d6

File tree

9 files changed

+111
-103
lines changed

9 files changed

+111
-103
lines changed

Diff for: ‎compiler/rustc_builtin_macros/src/format.rs

+14-18
Original file line numberDiff line numberDiff line change
@@ -845,8 +845,7 @@ impl<'a, 'b> Context<'a, 'b> {
845845
self.ecx.expr_match(self.macsp, head, vec![arm])
846846
};
847847

848-
let ident = Ident::from_str_and_span("args", self.macsp);
849-
let args_slice = self.ecx.expr_ident(self.macsp, ident);
848+
let args_slice = self.ecx.expr_addr_of(self.macsp, args_match);
850849

851850
// Now create the fmt::Arguments struct with all our locals we created.
852851
let (fn_name, fn_args) = if self.all_pieces_simple {
@@ -856,25 +855,22 @@ impl<'a, 'b> Context<'a, 'b> {
856855
// nonstandard placeholders, if there are any.
857856
let fmt = self.ecx.expr_vec_slice(self.macsp, self.pieces);
858857

859-
("new_v1_formatted", vec![pieces, args_slice, fmt])
858+
let path = self.ecx.std_path(&[sym::fmt, sym::UnsafeArg, sym::new]);
859+
let unsafe_arg = self.ecx.expr_call_global(self.macsp, path, Vec::new());
860+
let unsafe_expr = self.ecx.expr_block(P(ast::Block {
861+
stmts: vec![self.ecx.stmt_expr(unsafe_arg)],
862+
id: ast::DUMMY_NODE_ID,
863+
rules: BlockCheckMode::Unsafe(UnsafeSource::CompilerGenerated),
864+
span: self.macsp,
865+
tokens: None,
866+
could_be_bare_literal: false,
867+
}));
868+
869+
("new_v1_formatted", vec![pieces, args_slice, fmt, unsafe_expr])
860870
};
861871

862872
let path = self.ecx.std_path(&[sym::fmt, sym::Arguments, Symbol::intern(fn_name)]);
863-
let arguments = self.ecx.expr_call_global(self.macsp, path, fn_args);
864-
let body = self.ecx.expr_block(P(ast::Block {
865-
stmts: vec![self.ecx.stmt_expr(arguments)],
866-
id: ast::DUMMY_NODE_ID,
867-
rules: BlockCheckMode::Unsafe(UnsafeSource::CompilerGenerated),
868-
span: self.macsp,
869-
tokens: None,
870-
could_be_bare_literal: false,
871-
}));
872-
873-
let ident = Ident::from_str_and_span("args", self.macsp);
874-
let binding_mode = ast::BindingMode::ByRef(ast::Mutability::Not);
875-
let pat = self.ecx.pat_ident_binding_mode(self.macsp, ident, binding_mode);
876-
let arm = self.ecx.arm(self.macsp, pat, body);
877-
self.ecx.expr_match(self.macsp, args_match, vec![arm])
873+
self.ecx.expr_call_global(self.macsp, path, fn_args)
878874
}
879875

880876
fn format_arg(

Diff for: ‎compiler/rustc_span/src/symbol.rs

+1
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,7 @@ symbols! {
253253
TyCtxt,
254254
TyKind,
255255
Unknown,
256+
UnsafeArg,
256257
Vec,
257258
Yield,
258259
_DECLS,

Diff for: ‎library/core/src/fmt/mod.rs

+44-9
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,26 @@ pub struct ArgumentV1<'a> {
265265
formatter: fn(&Opaque, &mut Formatter<'_>) -> Result,
266266
}
267267

268+
/// This struct represents the unsafety of constructing an `Arguments`.
269+
/// It exists, rather than an unsafe function, in order to simplify the expansion
270+
/// of `format_args!(..)` and reduce the scope of the `unsafe` block.
271+
#[allow(missing_debug_implementations)]
272+
#[doc(hidden)]
273+
#[non_exhaustive]
274+
#[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")]
275+
pub struct UnsafeArg;
276+
277+
impl UnsafeArg {
278+
/// See documentation where `UnsafeArg` is required to know when it is safe to
279+
/// create and use `UnsafeArg`.
280+
#[doc(hidden)]
281+
#[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")]
282+
#[inline(always)]
283+
pub unsafe fn new() -> Self {
284+
Self
285+
}
286+
}
287+
268288
// This guarantees a single stable value for the function pointer associated with
269289
// indices/counts in the formatting infrastructure.
270290
//
@@ -337,22 +357,37 @@ impl<'a> Arguments<'a> {
337357
#[inline]
338358
#[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")]
339359
#[rustc_const_unstable(feature = "const_fmt_arguments_new", issue = "none")]
340-
pub const unsafe fn new_v1(
341-
pieces: &'a [&'static str],
342-
args: &'a [ArgumentV1<'a>],
343-
) -> Arguments<'a> {
360+
pub const fn new_v1(pieces: &'a [&'static str], args: &'a [ArgumentV1<'a>]) -> Arguments<'a> {
344361
if pieces.len() < args.len() || pieces.len() > args.len() + 1 {
345362
panic!("invalid args");
346363
}
347364
Arguments { pieces, fmt: None, args }
348365
}
349366

350367
/// This function is used to specify nonstandard formatting parameters.
351-
/// The `pieces` array must be at least as long as `fmt` to construct
352-
/// a valid Arguments structure. Also, any `Count` within `fmt` that is
353-
/// `CountIsParam` or `CountIsNextParam` has to point to an argument
354-
/// created with `argumentusize`. However, failing to do so doesn't cause
355-
/// unsafety, but will ignore invalid .
368+
///
369+
/// An `UnsafeArg` is required because the following invariants must be held
370+
/// in order for this function to be safe:
371+
/// 1. The `pieces` slice must be at least as long as `fmt`.
372+
/// 2. Every [`rt::v1::Argument::position`] value within `fmt` must be a
373+
/// valid index of `args`.
374+
/// 3. Every [`Count::Param`] within `fmt` must contain a valid index of
375+
/// `args`.
376+
#[cfg(not(bootstrap))]
377+
#[doc(hidden)]
378+
#[inline]
379+
#[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")]
380+
#[rustc_const_unstable(feature = "const_fmt_arguments_new", issue = "none")]
381+
pub const fn new_v1_formatted(
382+
pieces: &'a [&'static str],
383+
args: &'a [ArgumentV1<'a>],
384+
fmt: &'a [rt::v1::Argument],
385+
_unsafe_arg: UnsafeArg,
386+
) -> Arguments<'a> {
387+
Arguments { pieces, fmt: Some(fmt), args }
388+
}
389+
390+
#[cfg(bootstrap)]
356391
#[doc(hidden)]
357392
#[inline]
358393
#[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")]

Diff for: ‎library/core/src/panicking.rs

+1-4
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,7 @@ pub fn panic(expr: &'static str) -> ! {
4747
// truncation and padding (even though none is used here). Using
4848
// Arguments::new_v1 may allow the compiler to omit Formatter::pad from the
4949
// output binary, saving up to a few kilobytes.
50-
panic_fmt(
51-
// SAFETY: Arguments::new_v1 is safe with exactly one str and zero args
52-
unsafe { fmt::Arguments::new_v1(&[expr], &[]) },
53-
);
50+
panic_fmt(fmt::Arguments::new_v1(&[expr], &[]));
5451
}
5552

5653
#[inline]

Diff for: ‎src/test/pretty/dollar-crate.pp

+4-6
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,9 @@
1010

1111
fn main() {
1212
{
13-
::std::io::_print(match match () { () => [], } {
14-
ref args => unsafe {
15-
::core::fmt::Arguments::new_v1(&["rust\n"],
16-
args)
17-
}
18-
});
13+
::std::io::_print(::core::fmt::Arguments::new_v1(&["rust\n"],
14+
&match () {
15+
() => [],
16+
}));
1917
};
2018
}

Diff for: ‎src/test/pretty/issue-4264.pp

+23-33
Original file line numberDiff line numberDiff line change
@@ -32,39 +32,29 @@
3232
({
3333
let res =
3434
((::alloc::fmt::format as
35-
for<'r> fn(Arguments<'r>) -> String {format})((match (match (()
36-
as
37-
())
38-
{
39-
()
40-
=>
41-
([]
42-
as
43-
[ArgumentV1; 0]),
44-
}
45-
as
46-
[ArgumentV1; 0])
47-
{
48-
ref args
49-
=>
50-
unsafe
51-
{
52-
((::core::fmt::Arguments::new_v1
53-
as
54-
unsafe fn(&[&'static str], &[ArgumentV1]) -> Arguments {Arguments::new_v1})((&([("test"
55-
as
56-
&str)]
57-
as
58-
[&str; 1])
59-
as
60-
&[&str; 1]),
61-
(args
62-
as
63-
&[ArgumentV1; 0]))
64-
as
65-
Arguments)
66-
}
67-
}
35+
for<'r> fn(Arguments<'r>) -> String {format})(((::core::fmt::Arguments::new_v1
36+
as
37+
fn(&[&'static str], &[ArgumentV1]) -> Arguments {Arguments::new_v1})((&([("test"
38+
as
39+
&str)]
40+
as
41+
[&str; 1])
42+
as
43+
&[&str; 1]),
44+
(&(match (()
45+
as
46+
())
47+
{
48+
()
49+
=>
50+
([]
51+
as
52+
[ArgumentV1; 0]),
53+
}
54+
as
55+
[ArgumentV1; 0])
56+
as
57+
&[ArgumentV1; 0]))
6858
as
6959
Arguments))
7060
as String);

Diff for: ‎src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt

+3-3
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,15 @@
3131
24| 1| println!("{:?}", Foo(1));
3232
25| 1|
3333
26| 1| assert_ne!(Foo(0), Foo(5), "{}", if is_true { "true message" } else { "false message" });
34-
^0 ^0 ^0 ^0
34+
^0 ^0 ^0
3535
27| 1| assert_ne!(
3636
28| | Foo(0)
3737
29| | ,
3838
30| | Foo(5)
3939
31| | ,
4040
32| 0| "{}"
41-
33| | ,
42-
34| | if
41+
33| 0| ,
42+
34| 0| if
4343
35| 0| is_true
4444
36| | {
4545
37| 0| "true message"

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

+6-10
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,12 @@ LL | bug!();
1717

1818
error: unexpected token: `{
1919
let res =
20-
::alloc::fmt::format(match match (&"u8",) {
21-
(arg0,) =>
22-
[::core::fmt::ArgumentV1::new(arg0,
23-
::core::fmt::Display::fmt)],
24-
} {
25-
ref args => unsafe {
26-
::core::fmt::Arguments::new_v1(&[""],
27-
args)
28-
}
29-
});
20+
::alloc::fmt::format(::core::fmt::Arguments::new_v1(&[""],
21+
&match (&"u8",) {
22+
(arg0,) =>
23+
[::core::fmt::ArgumentV1::new(arg0,
24+
::core::fmt::Display::fmt)],
25+
}));
3026
res
3127
}.as_str()`
3228
--> $DIR/key-value-expansion.rs:48:23

Diff for: ‎src/tools/clippy/clippy_utils/src/higher.rs

+15-20
Original file line numberDiff line numberDiff line change
@@ -523,28 +523,12 @@ impl FormatArgsExpn<'tcx> {
523523
if let ExpnKind::Macro(_, name) = expr.span.ctxt().outer_expn_data().kind;
524524
let name = name.as_str();
525525
if name.ends_with("format_args") || name.ends_with("format_args_nl");
526-
527-
if let ExprKind::Match(inner_match, [arm], _) = expr.kind;
528-
529-
// `match match`, if you will
530-
if let ExprKind::Match(args, [inner_arm], _) = inner_match.kind;
531-
if let ExprKind::Tup(value_args) = args.kind;
532-
if let Some(value_args) = value_args
533-
.iter()
534-
.map(|e| match e.kind {
535-
ExprKind::AddrOf(_, _, e) => Some(e),
536-
_ => None,
537-
})
538-
.collect();
539-
if let ExprKind::Array(args) = inner_arm.body.kind;
540-
541-
if let ExprKind::Block(Block { stmts: [], expr: Some(expr), .. }, _) = arm.body.kind;
542-
if let ExprKind::Call(_, call_args) = expr.kind;
543-
if let Some((strs_ref, fmt_expr)) = match call_args {
526+
if let ExprKind::Call(_, args) = expr.kind;
527+
if let Some((strs_ref, args, fmt_expr)) = match args {
544528
// Arguments::new_v1
545-
[strs_ref, _] => Some((strs_ref, None)),
529+
[strs_ref, args] => Some((strs_ref, args, None)),
546530
// Arguments::new_v1_formatted
547-
[strs_ref, _, fmt_expr] => Some((strs_ref, Some(fmt_expr))),
531+
[strs_ref, args, fmt_expr, _unsafe_arg] => Some((strs_ref, args, Some(fmt_expr))),
548532
_ => None,
549533
};
550534
if let ExprKind::AddrOf(BorrowKind::Ref, _, strs_arr) = strs_ref.kind;
@@ -560,6 +544,17 @@ impl FormatArgsExpn<'tcx> {
560544
None
561545
})
562546
.collect();
547+
if let ExprKind::AddrOf(BorrowKind::Ref, _, args) = args.kind;
548+
if let ExprKind::Match(args, [arm], _) = args.kind;
549+
if let ExprKind::Tup(value_args) = args.kind;
550+
if let Some(value_args) = value_args
551+
.iter()
552+
.map(|e| match e.kind {
553+
ExprKind::AddrOf(_, _, e) => Some(e),
554+
_ => None,
555+
})
556+
.collect();
557+
if let ExprKind::Array(args) = arm.body.kind;
563558
then {
564559
Some(FormatArgsExpn {
565560
format_string_span: strs_ref.span,

0 commit comments

Comments
 (0)