Skip to content

Commit b2d405e

Browse files
camsteffencamsteffen
committed
Revert "Use ZST for fmt unsafety"
This reverts commit 09b37d7.
1 parent 62e25f7 commit b2d405e

File tree

8 files changed

+100
-108
lines changed

8 files changed

+100
-108
lines changed

‎compiler/rustc_builtin_macros/src/format.rs

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -844,7 +844,8 @@ impl<'a, 'b> Context<'a, 'b> {
844844
self.ecx.expr_match(self.macsp, head, vec![arm])
845845
};
846846

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

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

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

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

875879
fn format_arg(

‎compiler/rustc_span/src/symbol.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,6 @@ symbols! {
260260
TyCtxt,
261261
TyKind,
262262
Unknown,
263-
UnsafeArg,
264263
Vec,
265264
VecDeque,
266265
Yield,

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

Lines changed: 9 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -265,26 +265,6 @@ 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-
288268
// This guarantees a single stable value for the function pointer associated with
289269
// indices/counts in the formatting infrastructure.
290270
//
@@ -357,37 +337,22 @@ impl<'a> Arguments<'a> {
357337
#[inline]
358338
#[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")]
359339
#[rustc_const_unstable(feature = "const_fmt_arguments_new", issue = "none")]
360-
pub const fn new_v1(pieces: &'a [&'static str], args: &'a [ArgumentV1<'a>]) -> Arguments<'a> {
340+
pub const unsafe fn new_v1(
341+
pieces: &'a [&'static str],
342+
args: &'a [ArgumentV1<'a>],
343+
) -> Arguments<'a> {
361344
if pieces.len() < args.len() || pieces.len() > args.len() + 1 {
362345
panic!("invalid args");
363346
}
364347
Arguments { pieces, fmt: None, args }
365348
}
366349

367350
/// This function is used to specify nonstandard formatting parameters.
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)]
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 .
391356
#[doc(hidden)]
392357
#[inline]
393358
#[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")]

‎library/core/src/panicking.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,10 @@ 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(fmt::Arguments::new_v1(&[expr], &[]));
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+
);
5154
}
5255

5356
#[inline]

‎src/test/pretty/dollar-crate.pp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,11 @@
1010

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

‎src/test/pretty/issue-4264.pp

Lines changed: 33 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -32,29 +32,39 @@
3232
({
3333
let res =
3434
((::alloc::fmt::format as
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]))
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+
}
5868
as
5969
Arguments))
6070
as String);

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

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,16 @@ LL | bug!();
1717

1818
error: unexpected token: `{
1919
let res =
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-
}));
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+
});
2630
res
2731
}.as_str()`
2832
--> $DIR/key-value-expansion.rs:48:23

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

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -523,12 +523,28 @@ 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-
if let ExprKind::Call(_, args) = expr.kind;
527-
if let Some((strs_ref, args, fmt_expr)) = match args {
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 {
528544
// Arguments::new_v1
529-
[strs_ref, args] => Some((strs_ref, args, None)),
545+
[strs_ref, _] => Some((strs_ref, None)),
530546
// Arguments::new_v1_formatted
531-
[strs_ref, args, fmt_expr, _unsafe_arg] => Some((strs_ref, args, Some(fmt_expr))),
547+
[strs_ref, _, fmt_expr] => Some((strs_ref, Some(fmt_expr))),
532548
_ => None,
533549
};
534550
if let ExprKind::AddrOf(BorrowKind::Ref, _, strs_arr) = strs_ref.kind;
@@ -544,17 +560,6 @@ impl FormatArgsExpn<'tcx> {
544560
None
545561
})
546562
.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;
558563
then {
559564
Some(FormatArgsExpn {
560565
format_string_span: strs_ref.span,

0 commit comments

Comments
 (0)