Skip to content

Perf experiment for "Get piece unchecked in write" #89521

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 9 commits into from
27 changes: 7 additions & 20 deletions compiler/rustc_builtin_macros/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, *};
Expand Down Expand Up @@ -837,14 +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 args_slice = self.ecx.expr_addr_of(self.macsp, args_match);
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 {
Expand All @@ -854,18 +852,7 @@ 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)]);
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,6 @@ symbols! {
TyCtxt,
TyKind,
Unknown,
UnsafeArg,
Vec,
VecDeque,
Yield,
Expand Down
63 changes: 8 additions & 55 deletions library/core/src/fmt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -265,27 +266,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)]
#[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")]
pub struct UnsafeArg {
_private: (),
}

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 { _private: () }
}
}

// This guarantees a single stable value for the function pointer associated with
// indices/counts in the formatting infrastructure.
//
Expand Down Expand Up @@ -359,22 +339,15 @@ impl<'a> Arguments<'a> {
#[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> {
if pieces.len() < args.len() || pieces.len() > args.len() + 1 {
panic!("invalid args");
}
Arguments { pieces, fmt: None, args }
}

/// 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))]
/// 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")]
Expand All @@ -383,20 +356,6 @@ impl<'a> Arguments<'a> {
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")]
#[rustc_const_unstable(feature = "const_fmt_arguments_new", issue = "none")]
pub const unsafe fn new_v1_formatted(
pieces: &'a [&'static str],
args: &'a [ArgumentV1<'a>],
fmt: &'a [rt::v1::Argument],
) -> Arguments<'a> {
Arguments { pieces, fmt: Some(fmt), args }
}
Expand Down Expand Up @@ -1152,10 +1111,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)?;
}
Expand All @@ -1166,10 +1122,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)?;
}
Expand Down
1 change: 0 additions & 1 deletion library/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@
//
// Language features:
#![feature(abi_unadjusted)]
#![feature(allow_internal_unsafe)]
#![feature(allow_internal_unstable)]
#![feature(asm)]
#![feature(associated_type_bounds)]
Expand Down
1 change: 0 additions & 1 deletion library/core/src/macros/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
4 changes: 1 addition & 3 deletions src/test/debuginfo/rc_arc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,5 @@ fn main() {
let a1 = Arc::clone(&a);
let w2 = Arc::downgrade(&a);

zzz(); // #break
print!(""); // #break
}

fn zzz() { () }
Original file line number Diff line number Diff line change
Expand Up @@ -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

3 changes: 0 additions & 3 deletions src/test/ui/unsafe/unsafe-around-compiler-generated-unsafe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`
}
Original file line number Diff line number Diff line change
Expand Up @@ -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

2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_utils/src/higher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ impl FormatArgsExpn<'tcx> {
// Arguments::new_v1
[strs_ref, args] => Some((strs_ref, args, None)),
// Arguments::new_v1_formatted
[strs_ref, args, fmt_expr, _unsafe_arg] => Some((strs_ref, args, 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;
Expand Down