From 8e7a84b6c5784c569c723ce5d83b4729259b119a Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Mon, 4 Oct 2021 08:17:50 -0500 Subject: [PATCH 1/9] Revert "Add private arg to fmt::UnsafeArg" This reverts commit f5e4f78eb7e3bad90b8dbbd20c58426817a4ffbf. --- library/core/src/fmt/mod.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index d10563a40976d..9dea92136fcde 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -270,10 +270,9 @@ pub struct ArgumentV1<'a> { /// 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 { - _private: (), -} +pub struct UnsafeArg; impl UnsafeArg { /// See documentation where `UnsafeArg` is required to know when it is safe to @@ -282,7 +281,7 @@ impl UnsafeArg { #[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")] #[inline(always)] pub unsafe fn new() -> Self { - Self { _private: () } + Self } } From 62e25f7eda1a1ee1cde018a35f0a625054d87361 Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Mon, 4 Oct 2021 08:18:07 -0500 Subject: [PATCH 2/9] Revert "Fix test" This reverts commit 2efa9d796981163031a7478734fee561dc3a6da0. --- .../coverage-reports/expected_show_coverage.issue-84561.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 f24f7c6940473..b35f3f54de904 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| 0| , - 34| 0| if + 33| | , + 34| | if 35| 0| is_true 36| | { 37| 0| "true message" From b2d405e96b04616dbc88be4869e756025dcbbade Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Mon, 4 Oct 2021 08:18:17 -0500 Subject: [PATCH 3/9] Revert "Use ZST for fmt unsafety" This reverts commit 09b37d743328bd497939bd27135f82350f1b0bd7. --- compiler/rustc_builtin_macros/src/format.rs | 32 ++++++----- compiler/rustc_span/src/symbol.rs | 1 - library/core/src/fmt/mod.rs | 53 +++--------------- library/core/src/panicking.rs | 5 +- src/test/pretty/dollar-crate.pp | 10 ++-- src/test/pretty/issue-4264.pp | 56 +++++++++++-------- .../ui/attributes/key-value-expansion.stderr | 16 ++++-- src/tools/clippy/clippy_utils/src/higher.rs | 35 +++++++----- 8 files changed, 100 insertions(+), 108 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs index f0056cb79766a..950c86b3202a8 100644 --- a/compiler/rustc_builtin_macros/src/format.rs +++ b/compiler/rustc_builtin_macros/src/format.rs @@ -844,7 +844,8 @@ impl<'a, 'b> Context<'a, 'b> { self.ecx.expr_match(self.macsp, head, vec![arm]) }; - let args_slice = self.ecx.expr_addr_of(self.macsp, args_match); + let ident = Ident::from_str_and_span("args", self.macsp); + let args_slice = self.ecx.expr_ident(self.macsp, ident); // Now create the fmt::Arguments struct with all our locals we created. let (fn_name, fn_args) = if self.all_pieces_simple { @@ -854,22 +855,25 @@ impl<'a, 'b> Context<'a, 'b> { // nonstandard placeholders, if there are any. let fmt = self.ecx.expr_vec_slice(self.macsp, self.pieces); - 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]) + ("new_v1_formatted", vec![pieces, args_slice, fmt]) }; let path = self.ecx.std_path(&[sym::fmt, sym::Arguments, Symbol::intern(fn_name)]); - self.ecx.expr_call_global(self.macsp, path, fn_args) + 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]) } fn format_arg( diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 77baf7d73810e..5acf9a35c7b4e 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -260,7 +260,6 @@ symbols! { TyCtxt, TyKind, Unknown, - UnsafeArg, Vec, VecDeque, Yield, diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index 9dea92136fcde..a1bc79444b2d7 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -265,26 +265,6 @@ 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. // @@ -357,7 +337,10 @@ 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 fn new_v1(pieces: &'a [&'static str], args: &'a [ArgumentV1<'a>]) -> Arguments<'a> { + pub const unsafe 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"); } @@ -365,29 +348,11 @@ impl<'a> Arguments<'a> { } /// This function is used to specify nonstandard formatting parameters. - /// - /// 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)] + /// 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 . #[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 6d3ec6ae8612a..a6aa4bf43c865 100644 --- a/library/core/src/panicking.rs +++ b/library/core/src/panicking.rs @@ -47,7 +47,10 @@ 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(fmt::Arguments::new_v1(&[expr], &[])); + panic_fmt( + // SAFETY: Arguments::new_v1 is safe with exactly one str and zero args + unsafe { fmt::Arguments::new_v1(&[expr], &[]) }, + ); } #[inline] diff --git a/src/test/pretty/dollar-crate.pp b/src/test/pretty/dollar-crate.pp index f4be3c1c63a84..4eccba06b134f 100644 --- a/src/test/pretty/dollar-crate.pp +++ b/src/test/pretty/dollar-crate.pp @@ -10,9 +10,11 @@ fn main() { { - ::std::io::_print(::core::fmt::Arguments::new_v1(&["rust\n"], - &match () { - () => [], - })); + ::std::io::_print(match match () { () => [], } { + ref args => unsafe { + ::core::fmt::Arguments::new_v1(&["rust\n"], + args) + } + }); }; } diff --git a/src/test/pretty/issue-4264.pp b/src/test/pretty/issue-4264.pp index 199aee05622be..a21ea520121e3 100644 --- a/src/test/pretty/issue-4264.pp +++ b/src/test/pretty/issue-4264.pp @@ -32,29 +32,39 @@ ({ let res = ((::alloc::fmt::format as - 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])) + 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) + } + } as Arguments)) as String); diff --git a/src/test/ui/attributes/key-value-expansion.stderr b/src/test/ui/attributes/key-value-expansion.stderr index 31e93ef54f260..03ca515265cb0 100644 --- a/src/test/ui/attributes/key-value-expansion.stderr +++ b/src/test/ui/attributes/key-value-expansion.stderr @@ -17,12 +17,16 @@ LL | bug!(); error: unexpected token: `{ let res = - ::alloc::fmt::format(::core::fmt::Arguments::new_v1(&[""], - &match (&"u8",) { - (arg0,) => - [::core::fmt::ArgumentV1::new(arg0, - ::core::fmt::Display::fmt)], - })); + ::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) + } + }); 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 ba4d50bf74469..15e7f4904725c 100644 --- a/src/tools/clippy/clippy_utils/src/higher.rs +++ b/src/tools/clippy/clippy_utils/src/higher.rs @@ -523,12 +523,28 @@ 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::Call(_, args) = expr.kind; - if let Some((strs_ref, args, fmt_expr)) = match args { + + 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 { // Arguments::new_v1 - [strs_ref, args] => Some((strs_ref, args, None)), + [strs_ref, _] => Some((strs_ref, None)), // Arguments::new_v1_formatted - [strs_ref, args, fmt_expr, _unsafe_arg] => Some((strs_ref, args, Some(fmt_expr))), + [strs_ref, _, fmt_expr] => Some((strs_ref, Some(fmt_expr))), _ => None, }; if let ExprKind::AddrOf(BorrowKind::Ref, _, strs_arr) = strs_ref.kind; @@ -544,17 +560,6 @@ 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, From da2bf00dd0761781c5a8a5756d7e852b8f125b41 Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Mon, 4 Oct 2021 08:20:13 -0500 Subject: [PATCH 4/9] Revert "Fix a coverage-reports test" This reverts commit b1e07b82dcef6d31556a92cb75d825bce6032519. --- .../coverage-reports/expected_show_coverage.issue-84561.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 b35f3f54de904..f24f7c6940473 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" From ecfeaa096983beb3f1012ec59c28cdef54cbd9bd Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Mon, 4 Oct 2021 08:20:18 -0500 Subject: [PATCH 5/9] Revert "clippy: Fix format_args expansion parsing" This reverts commit d3097fc496d2fb87f988618a24f133026229120c. --- src/tools/clippy/clippy_utils/src/higher.rs | 35 +++++++++------------ 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/src/tools/clippy/clippy_utils/src/higher.rs b/src/tools/clippy/clippy_utils/src/higher.rs index 15e7f4904725c..582b108ddbb51 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] => 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, From 2959f066289985b83c0d3e54b09d05f52281aa25 Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Mon, 4 Oct 2021 08:20:21 -0500 Subject: [PATCH 6/9] Revert "Fix a debuginfo test" This reverts commit 51979ac1c710f334d3a13379b0279599b2f14fdc. --- src/test/debuginfo/rc_arc.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/test/debuginfo/rc_arc.rs b/src/test/debuginfo/rc_arc.rs index 144a746062daa..55cddf7c6c6b4 100644 --- a/src/test/debuginfo/rc_arc.rs +++ b/src/test/debuginfo/rc_arc.rs @@ -75,7 +75,5 @@ fn main() { let a1 = Arc::clone(&a); let w2 = Arc::downgrade(&a); - zzz(); // #break + print!(""); // #break } - -fn zzz() { () } From 97f0b3cb52d526d906422cb37f7e273c369f735d Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Mon, 4 Oct 2021 08:20:23 -0500 Subject: [PATCH 7/9] Revert "Add unnecessary unsafe test" This reverts commit 46377c48a400bc39110fb9a930c2c200ceac7cd2. --- .../unsafe-around-compiler-generated-unsafe.mir.stderr | 8 +------- .../ui/unsafe/unsafe-around-compiler-generated-unsafe.rs | 3 --- .../unsafe-around-compiler-generated-unsafe.thir.stderr | 8 +------- 3 files changed, 2 insertions(+), 17 deletions(-) diff --git a/src/test/ui/unsafe/unsafe-around-compiler-generated-unsafe.mir.stderr b/src/test/ui/unsafe/unsafe-around-compiler-generated-unsafe.mir.stderr index 62199e5a2ec0b..29bd84cd0db53 100644 --- a/src/test/ui/unsafe/unsafe-around-compiler-generated-unsafe.mir.stderr +++ b/src/test/ui/unsafe/unsafe-around-compiler-generated-unsafe.mir.stderr @@ -10,11 +10,5 @@ note: the lint level is defined here LL | #![deny(unused_unsafe)] | ^^^^^^^^^^^^^ -error: unnecessary `unsafe` block - --> $DIR/unsafe-around-compiler-generated-unsafe.rs:13:5 - | -LL | unsafe { println!("foo"); } - | ^^^^^^ unnecessary `unsafe` block - -error: aborting due to 2 previous errors +error: aborting due to previous error diff --git a/src/test/ui/unsafe/unsafe-around-compiler-generated-unsafe.rs b/src/test/ui/unsafe/unsafe-around-compiler-generated-unsafe.rs index c1a3276403985..e9c7efb9e8b80 100644 --- a/src/test/ui/unsafe/unsafe-around-compiler-generated-unsafe.rs +++ b/src/test/ui/unsafe/unsafe-around-compiler-generated-unsafe.rs @@ -8,7 +8,4 @@ fn main() { let _ = async { unsafe { async {}.await; } //~ ERROR unnecessary `unsafe` }; - - // `format_args!` expands with a compiler-generated unsafe block - unsafe { println!("foo"); } //~ ERROR unnecessary `unsafe` } diff --git a/src/test/ui/unsafe/unsafe-around-compiler-generated-unsafe.thir.stderr b/src/test/ui/unsafe/unsafe-around-compiler-generated-unsafe.thir.stderr index 62199e5a2ec0b..29bd84cd0db53 100644 --- a/src/test/ui/unsafe/unsafe-around-compiler-generated-unsafe.thir.stderr +++ b/src/test/ui/unsafe/unsafe-around-compiler-generated-unsafe.thir.stderr @@ -10,11 +10,5 @@ note: the lint level is defined here LL | #![deny(unused_unsafe)] | ^^^^^^^^^^^^^ -error: unnecessary `unsafe` block - --> $DIR/unsafe-around-compiler-generated-unsafe.rs:13:5 - | -LL | unsafe { println!("foo"); } - | ^^^^^^ unnecessary `unsafe` block - -error: aborting due to 2 previous errors +error: aborting due to previous error From c10e80abfb65dd875a370dfb944dd8b2d499781e Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Mon, 4 Oct 2021 08:20:25 -0500 Subject: [PATCH 8/9] Revert "Get piece unchecked in write" This reverts commit f4ef07c2a9adf6df5717c6d664fad0b8fea7a1bd. --- library/core/src/fmt/mod.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index a1bc79444b2d7..1317a5afae476 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -4,6 +4,7 @@ use crate::cell::{Cell, Ref, RefCell, RefMut, UnsafeCell}; use crate::char::EscapeDebugExtArgs; +use crate::iter; use crate::marker::PhantomData; use crate::mem; use crate::num::fmt as numfmt; @@ -1116,10 +1117,7 @@ pub fn write(output: &mut dyn Write, args: Arguments<'_>) -> Result { match args.fmt { None => { // We can use default formatting parameters for all arguments. - for (i, arg) in args.args.iter().enumerate() { - // SAFETY: args.args and args.pieces come from the same Arguments, - // which guarantees the indexes are always within bounds. - let piece = unsafe { args.pieces.get_unchecked(i) }; + for (arg, piece) in iter::zip(args.args, args.pieces) { if !piece.is_empty() { formatter.buf.write_str(*piece)?; } @@ -1130,10 +1128,7 @@ pub fn write(output: &mut dyn Write, args: Arguments<'_>) -> Result { Some(fmt) => { // Every spec has a corresponding argument that is preceded by // a string piece. - for (i, arg) in fmt.iter().enumerate() { - // SAFETY: fmt and args.pieces come from the same Arguments, - // which guarantees the indexes are always within bounds. - let piece = unsafe { args.pieces.get_unchecked(i) }; + for (arg, piece) in iter::zip(fmt, args.pieces) { if !piece.is_empty() { formatter.buf.write_str(*piece)?; } From 669f6ecb0f0776195bc899c99cc4aeb485e8e111 Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Mon, 4 Oct 2021 08:24:36 -0500 Subject: [PATCH 9/9] Revert "Make Arguments constructors unsafe" This reverts commit 975bc18481879b69603674266a5239ecb579f928. --- compiler/rustc_builtin_macros/src/format.rs | 31 +++------- library/core/src/fmt/mod.rs | 10 +--- library/core/src/lib.rs | 1 - library/core/src/macros/mod.rs | 1 - library/core/src/panicking.rs | 5 +- src/test/pretty/dollar-crate.pp | 10 ++-- src/test/pretty/issue-4264.pp | 56 ++++++++----------- .../ui/attributes/key-value-expansion.stderr | 16 ++---- 8 files changed, 43 insertions(+), 87 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs index 950c86b3202a8..7bf85a8719e59 100644 --- a/compiler/rustc_builtin_macros/src/format.rs +++ b/compiler/rustc_builtin_macros/src/format.rs @@ -3,8 +3,8 @@ use Position::*; use rustc_ast as ast; use rustc_ast::ptr::P; +use rustc_ast::token; use rustc_ast::tokenstream::TokenStream; -use rustc_ast::{token, BlockCheckMode, UnsafeSource}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::{pluralize, Applicability, DiagnosticBuilder}; use rustc_expand::base::{self, *}; @@ -837,15 +837,12 @@ impl<'a, 'b> Context<'a, 'b> { // // But the nested match expression is proved to perform not as well // as series of let's; the first approach does. - let args_match = { - let pat = self.ecx.pat_tuple(self.macsp, pats); - let arm = self.ecx.arm(self.macsp, pat, args_array); - let head = self.ecx.expr(self.macsp, ast::ExprKind::Tup(heads)); - self.ecx.expr_match(self.macsp, head, vec![arm]) - }; + let pat = self.ecx.pat_tuple(self.macsp, pats); + let arm = self.ecx.arm(self.macsp, pat, args_array); + let head = self.ecx.expr(self.macsp, ast::ExprKind::Tup(heads)); + let result = 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, result); // Now create the fmt::Arguments struct with all our locals we created. let (fn_name, fn_args) = if self.all_pieces_simple { @@ -859,21 +856,7 @@ impl<'a, 'b> Context<'a, 'b> { }; 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/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index 1317a5afae476..efec2151f08aa 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -338,13 +338,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> { - if pieces.len() < args.len() || pieces.len() > args.len() + 1 { - panic!("invalid args"); - } + pub const fn new_v1(pieces: &'a [&'static str], args: &'a [ArgumentV1<'a>]) -> Arguments<'a> { Arguments { pieces, fmt: None, args } } @@ -358,7 +352,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_formatted( + pub const fn new_v1_formatted( pieces: &'a [&'static str], args: &'a [ArgumentV1<'a>], fmt: &'a [rt::v1::Argument], diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index 45ef1fcde2a55..e75b73a7b7407 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -113,7 +113,6 @@ // // Language features: #![feature(abi_unadjusted)] -#![feature(allow_internal_unsafe)] #![feature(allow_internal_unstable)] #![feature(asm)] #![feature(associated_type_bounds)] diff --git a/library/core/src/macros/mod.rs b/library/core/src/macros/mod.rs index 035a748f78248..3d658e73ff56c 100644 --- a/library/core/src/macros/mod.rs +++ b/library/core/src/macros/mod.rs @@ -828,7 +828,6 @@ pub(crate) mod builtin { /// assert_eq!(s, format!("hello {}", "world")); /// ``` #[stable(feature = "rust1", since = "1.0.0")] - #[allow_internal_unsafe] #[allow_internal_unstable(fmt_internals)] #[rustc_builtin_macro] #[macro_export] diff --git a/library/core/src/panicking.rs b/library/core/src/panicking.rs index a6aa4bf43c865..6d3ec6ae8612a 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 4eccba06b134f..f4be3c1c63a84 100644 --- a/src/test/pretty/dollar-crate.pp +++ b/src/test/pretty/dollar-crate.pp @@ -10,11 +10,9 @@ 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 a21ea520121e3..199aee05622be 100644 --- a/src/test/pretty/issue-4264.pp +++ b/src/test/pretty/issue-4264.pp @@ -32,39 +32,29 @@ ({ 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/ui/attributes/key-value-expansion.stderr b/src/test/ui/attributes/key-value-expansion.stderr index 03ca515265cb0..31e93ef54f260 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