about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2021-09-23 02:10:26 +0000
committerbors <bors@rust-lang.org>2021-09-23 02:10:26 +0000
commit67365d64bcdfeae1334bf2ff49587c27d1c973f0 (patch)
treee675fccc48356eac3a83e78ae3894491a6c0ded5
parent30278d3cf92b581550933546370443a5d5700002 (diff)
parent2efa9d796981163031a7478734fee561dc3a6da0 (diff)
downloadrust-67365d64bcdfeae1334bf2ff49587c27d1c973f0.tar.gz
rust-67365d64bcdfeae1334bf2ff49587c27d1c973f0.zip
Auto merge of #89139 - camsteffen:write-perf, r=Mark-Simulacrum
Use ZST for fmt unsafety

as suggested here - https://github.com/rust-lang/rust/pull/83302#issuecomment-923529151.
-rw-r--r--compiler/rustc_builtin_macros/src/format.rs32
-rw-r--r--compiler/rustc_span/src/symbol.rs1
-rw-r--r--library/core/src/fmt/mod.rs53
-rw-r--r--library/core/src/panicking.rs5
-rw-r--r--src/test/pretty/dollar-crate.pp10
-rw-r--r--src/test/pretty/issue-4264.pp56
-rw-r--r--src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt6
-rw-r--r--src/test/ui/attributes/key-value-expansion.stderr16
-rw-r--r--src/tools/clippy/clippy_utils/src/higher.rs35
9 files changed, 111 insertions, 103 deletions
diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs
index c7626dec4d7..9b9adc2d7f3 100644
--- a/compiler/rustc_builtin_macros/src/format.rs
+++ b/compiler/rustc_builtin_macros/src/format.rs
@@ -845,8 +845,7 @@ impl<'a, 'b> Context<'a, 'b> {
             self.ecx.expr_match(self.macsp, head, vec![arm])
         };
 
-        let ident = Ident::from_str_and_span("args", self.macsp);
-        let args_slice = self.ecx.expr_ident(self.macsp, ident);
+        let args_slice = self.ecx.expr_addr_of(self.macsp, args_match);
 
         // Now create the fmt::Arguments struct with all our locals we created.
         let (fn_name, fn_args) = if self.all_pieces_simple {
@@ -856,25 +855,22 @@ impl<'a, 'b> Context<'a, 'b> {
             // nonstandard placeholders, if there are any.
             let fmt = self.ecx.expr_vec_slice(self.macsp, self.pieces);
 
-            ("new_v1_formatted", vec![pieces, args_slice, fmt])
+            let path = self.ecx.std_path(&[sym::fmt, sym::UnsafeArg, sym::new]);
+            let unsafe_arg = self.ecx.expr_call_global(self.macsp, path, Vec::new());
+            let unsafe_expr = self.ecx.expr_block(P(ast::Block {
+                stmts: vec![self.ecx.stmt_expr(unsafe_arg)],
+                id: ast::DUMMY_NODE_ID,
+                rules: BlockCheckMode::Unsafe(UnsafeSource::CompilerGenerated),
+                span: self.macsp,
+                tokens: None,
+                could_be_bare_literal: false,
+            }));
+
+            ("new_v1_formatted", vec![pieces, args_slice, fmt, unsafe_expr])
         };
 
         let path = self.ecx.std_path(&[sym::fmt, sym::Arguments, Symbol::intern(fn_name)]);
-        let arguments = self.ecx.expr_call_global(self.macsp, path, fn_args);
-        let body = self.ecx.expr_block(P(ast::Block {
-            stmts: vec![self.ecx.stmt_expr(arguments)],
-            id: ast::DUMMY_NODE_ID,
-            rules: BlockCheckMode::Unsafe(UnsafeSource::CompilerGenerated),
-            span: self.macsp,
-            tokens: None,
-            could_be_bare_literal: false,
-        }));
-
-        let ident = Ident::from_str_and_span("args", self.macsp);
-        let binding_mode = ast::BindingMode::ByRef(ast::Mutability::Not);
-        let pat = self.ecx.pat_ident_binding_mode(self.macsp, ident, binding_mode);
-        let arm = self.ecx.arm(self.macsp, pat, body);
-        self.ecx.expr_match(self.macsp, args_match, vec![arm])
+        self.ecx.expr_call_global(self.macsp, path, fn_args)
     }
 
     fn format_arg(
diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs
index 7c2a09e0a32..11ca6c7d81d 100644
--- a/compiler/rustc_span/src/symbol.rs
+++ b/compiler/rustc_span/src/symbol.rs
@@ -253,6 +253,7 @@ symbols! {
         TyCtxt,
         TyKind,
         Unknown,
+        UnsafeArg,
         Vec,
         Yield,
         _DECLS,
diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs
index 166a8e3f28a..8fa941c42fc 100644
--- a/library/core/src/fmt/mod.rs
+++ b/library/core/src/fmt/mod.rs
@@ -265,6 +265,26 @@ pub struct ArgumentV1<'a> {
     formatter: fn(&Opaque, &mut Formatter<'_>) -> Result,
 }
 
+/// This struct represents the unsafety of constructing an `Arguments`.
+/// It exists, rather than an unsafe function, in order to simplify the expansion
+/// of `format_args!(..)` and reduce the scope of the `unsafe` block.
+#[allow(missing_debug_implementations)]
+#[doc(hidden)]
+#[non_exhaustive]
+#[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")]
+pub struct UnsafeArg;
+
+impl UnsafeArg {
+    /// See documentation where `UnsafeArg` is required to know when it is safe to
+    /// create and use `UnsafeArg`.
+    #[doc(hidden)]
+    #[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")]
+    #[inline(always)]
+    pub unsafe fn new() -> Self {
+        Self
+    }
+}
+
 // This guarantees a single stable value for the function pointer associated with
 // indices/counts in the formatting infrastructure.
 //
@@ -337,10 +357,7 @@ impl<'a> Arguments<'a> {
     #[inline]
     #[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")]
     #[rustc_const_unstable(feature = "const_fmt_arguments_new", issue = "none")]
-    pub const unsafe fn new_v1(
-        pieces: &'a [&'static str],
-        args: &'a [ArgumentV1<'a>],
-    ) -> Arguments<'a> {
+    pub const fn new_v1(pieces: &'a [&'static str], args: &'a [ArgumentV1<'a>]) -> Arguments<'a> {
         if pieces.len() < args.len() || pieces.len() > args.len() + 1 {
             panic!("invalid args");
         }
@@ -348,11 +365,29 @@ impl<'a> Arguments<'a> {
     }
 
     /// This function is used to specify nonstandard formatting parameters.
-    /// The `pieces` array must be at least as long as `fmt` to construct
-    /// a valid Arguments structure. Also, any `Count` within `fmt` that is
-    /// `CountIsParam` or `CountIsNextParam` has to point to an argument
-    /// created with `argumentusize`. However, failing to do so doesn't cause
-    /// unsafety, but will ignore invalid .
+    ///
+    /// An `UnsafeArg` is required because the following invariants must be held
+    /// in order for this function to be safe:
+    /// 1. The `pieces` slice must be at least as long as `fmt`.
+    /// 2. Every [`rt::v1::Argument::position`] value within `fmt` must be a
+    ///    valid index of `args`.
+    /// 3. Every [`Count::Param`] within `fmt` must contain a valid index of
+    ///    `args`.
+    #[cfg(not(bootstrap))]
+    #[doc(hidden)]
+    #[inline]
+    #[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")]
+    #[rustc_const_unstable(feature = "const_fmt_arguments_new", issue = "none")]
+    pub const fn new_v1_formatted(
+        pieces: &'a [&'static str],
+        args: &'a [ArgumentV1<'a>],
+        fmt: &'a [rt::v1::Argument],
+        _unsafe_arg: UnsafeArg,
+    ) -> Arguments<'a> {
+        Arguments { pieces, fmt: Some(fmt), args }
+    }
+
+    #[cfg(bootstrap)]
     #[doc(hidden)]
     #[inline]
     #[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")]
diff --git a/library/core/src/panicking.rs b/library/core/src/panicking.rs
index a6aa4bf43c8..6d3ec6ae861 100644
--- a/library/core/src/panicking.rs
+++ b/library/core/src/panicking.rs
@@ -47,10 +47,7 @@ pub fn panic(expr: &'static str) -> ! {
     // truncation and padding (even though none is used here). Using
     // Arguments::new_v1 may allow the compiler to omit Formatter::pad from the
     // output binary, saving up to a few kilobytes.
-    panic_fmt(
-        // SAFETY: Arguments::new_v1 is safe with exactly one str and zero args
-        unsafe { fmt::Arguments::new_v1(&[expr], &[]) },
-    );
+    panic_fmt(fmt::Arguments::new_v1(&[expr], &[]));
 }
 
 #[inline]
diff --git a/src/test/pretty/dollar-crate.pp b/src/test/pretty/dollar-crate.pp
index 4eccba06b13..f4be3c1c63a 100644
--- a/src/test/pretty/dollar-crate.pp
+++ b/src/test/pretty/dollar-crate.pp
@@ -10,11 +10,9 @@ extern crate std;
 
 fn main() {
     {
-        ::std::io::_print(match match () { () => [], } {
-                              ref args => unsafe {
-                                  ::core::fmt::Arguments::new_v1(&["rust\n"],
-                                                                 args)
-                              }
-                          });
+        ::std::io::_print(::core::fmt::Arguments::new_v1(&["rust\n"],
+                                                         &match () {
+                                                              () => [],
+                                                          }));
     };
 }
diff --git a/src/test/pretty/issue-4264.pp b/src/test/pretty/issue-4264.pp
index a21ea520121..199aee05622 100644
--- a/src/test/pretty/issue-4264.pp
+++ b/src/test/pretty/issue-4264.pp
@@ -32,39 +32,29 @@ pub fn bar() ({
                   ({
                        let res =
                            ((::alloc::fmt::format as
-                                for<'r> fn(Arguments<'r>) -> String {format})((match (match (()
-                                                                                                as
-                                                                                                ())
-                                                                                          {
-                                                                                          ()
-                                                                                          =>
-                                                                                          ([]
-                                                                                              as
-                                                                                              [ArgumentV1; 0]),
-                                                                                      }
-                                                                                         as
-                                                                                         [ArgumentV1; 0])
-                                                                                   {
-                                                                                   ref args
-                                                                                   =>
-                                                                                   unsafe
-                                                                                   {
-                                                                                       ((::core::fmt::Arguments::new_v1
-                                                                                            as
-                                                                                            unsafe fn(&[&'static str], &[ArgumentV1]) -> Arguments {Arguments::new_v1})((&([("test"
-                                                                                                                                                                                as
-                                                                                                                                                                                &str)]
-                                                                                                                                                                              as
-                                                                                                                                                                              [&str; 1])
-                                                                                                                                                                            as
-                                                                                                                                                                            &[&str; 1]),
-                                                                                                                                                                        (args
-                                                                                                                                                                            as
-                                                                                                                                                                            &[ArgumentV1; 0]))
-                                                                                           as
-                                                                                           Arguments)
-                                                                                   }
-                                                                               }
+                                for<'r> fn(Arguments<'r>) -> String {format})(((::core::fmt::Arguments::new_v1
+                                                                                   as
+                                                                                   fn(&[&'static str], &[ArgumentV1]) -> Arguments {Arguments::new_v1})((&([("test"
+                                                                                                                                                                as
+                                                                                                                                                                &str)]
+                                                                                                                                                              as
+                                                                                                                                                              [&str; 1])
+                                                                                                                                                            as
+                                                                                                                                                            &[&str; 1]),
+                                                                                                                                                        (&(match (()
+                                                                                                                                                                     as
+                                                                                                                                                                     ())
+                                                                                                                                                               {
+                                                                                                                                                               ()
+                                                                                                                                                               =>
+                                                                                                                                                               ([]
+                                                                                                                                                                   as
+                                                                                                                                                                   [ArgumentV1; 0]),
+                                                                                                                                                           }
+                                                                                                                                                              as
+                                                                                                                                                              [ArgumentV1; 0])
+                                                                                                                                                            as
+                                                                                                                                                            &[ArgumentV1; 0]))
                                                                                   as
                                                                                   Arguments))
                                as String);
diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt
index b35f3f54de9..f24f7c69404 100644
--- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt
+++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt
@@ -31,15 +31,15 @@
    24|      1|    println!("{:?}", Foo(1));
    25|      1|
    26|      1|    assert_ne!(Foo(0), Foo(5), "{}", if is_true { "true message" } else { "false message" });
-                                             ^0       ^0        ^0                      ^0
+                                             ^0                 ^0                      ^0
    27|      1|    assert_ne!(
    28|       |        Foo(0)
    29|       |        ,
    30|       |        Foo(5)
    31|       |        ,
    32|      0|        "{}"
-   33|       |        ,
-   34|       |        if
+   33|      0|        ,
+   34|      0|        if
    35|      0|        is_true
    36|       |        {
    37|      0|            "true message"
diff --git a/src/test/ui/attributes/key-value-expansion.stderr b/src/test/ui/attributes/key-value-expansion.stderr
index 03ca515265c..31e93ef54f2 100644
--- a/src/test/ui/attributes/key-value-expansion.stderr
+++ b/src/test/ui/attributes/key-value-expansion.stderr
@@ -17,16 +17,12 @@ LL | bug!();
 
 error: unexpected token: `{
     let res =
-        ::alloc::fmt::format(match match (&"u8",) {
-                                       (arg0,) =>
-                                       [::core::fmt::ArgumentV1::new(arg0,
-                                                                     ::core::fmt::Display::fmt)],
-                                   } {
-                                 ref args => unsafe {
-                                     ::core::fmt::Arguments::new_v1(&[""],
-                                                                    args)
-                                 }
-                             });
+        ::alloc::fmt::format(::core::fmt::Arguments::new_v1(&[""],
+                                                            &match (&"u8",) {
+                                                                 (arg0,) =>
+                                                                 [::core::fmt::ArgumentV1::new(arg0,
+                                                                                               ::core::fmt::Display::fmt)],
+                                                             }));
     res
 }.as_str()`
   --> $DIR/key-value-expansion.rs:48:23
diff --git a/src/tools/clippy/clippy_utils/src/higher.rs b/src/tools/clippy/clippy_utils/src/higher.rs
index 94b3cd371bd..e6c06224999 100644
--- a/src/tools/clippy/clippy_utils/src/higher.rs
+++ b/src/tools/clippy/clippy_utils/src/higher.rs
@@ -523,28 +523,12 @@ impl FormatArgsExpn<'tcx> {
             if let ExpnKind::Macro(_, name) = expr.span.ctxt().outer_expn_data().kind;
             let name = name.as_str();
             if name.ends_with("format_args") || name.ends_with("format_args_nl");
-
-            if let ExprKind::Match(inner_match, [arm], _) = expr.kind;
-
-            // `match match`, if you will
-            if let ExprKind::Match(args, [inner_arm], _) = inner_match.kind;
-            if let ExprKind::Tup(value_args) = args.kind;
-            if let Some(value_args) = value_args
-                .iter()
-                .map(|e| match e.kind {
-                    ExprKind::AddrOf(_, _, e) => Some(e),
-                    _ => None,
-                })
-                .collect();
-            if let ExprKind::Array(args) = inner_arm.body.kind;
-
-            if let ExprKind::Block(Block { stmts: [], expr: Some(expr), .. }, _) = arm.body.kind;
-            if let ExprKind::Call(_, call_args) = expr.kind;
-            if let Some((strs_ref, fmt_expr)) = match call_args {
+            if let ExprKind::Call(_, args) = expr.kind;
+            if let Some((strs_ref, args, fmt_expr)) = match args {
                 // Arguments::new_v1
-                [strs_ref, _] => Some((strs_ref, None)),
+                [strs_ref, args] => Some((strs_ref, args, None)),
                 // Arguments::new_v1_formatted
-                [strs_ref, _, fmt_expr] => Some((strs_ref, Some(fmt_expr))),
+                [strs_ref, args, fmt_expr, _unsafe_arg] => Some((strs_ref, args, Some(fmt_expr))),
                 _ => None,
             };
             if let ExprKind::AddrOf(BorrowKind::Ref, _, strs_arr) = strs_ref.kind;
@@ -560,6 +544,17 @@ impl FormatArgsExpn<'tcx> {
                     None
                 })
                 .collect();
+            if let ExprKind::AddrOf(BorrowKind::Ref, _, args) = args.kind;
+            if let ExprKind::Match(args, [arm], _) = args.kind;
+            if let ExprKind::Tup(value_args) = args.kind;
+            if let Some(value_args) = value_args
+                .iter()
+                .map(|e| match e.kind {
+                    ExprKind::AddrOf(_, _, e) => Some(e),
+                    _ => None,
+                })
+                .collect();
+            if let ExprKind::Array(args) = arm.body.kind;
             then {
                 Some(FormatArgsExpn {
                     format_string_span: strs_ref.span,