Skip to content

Commit

Permalink
chore: cleanup format args (#1688)
Browse files Browse the repository at this point in the history
* chore: cleanup format args

Apply [`uninlined_format_args`](https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args) clippy lint -- makes code a bit more readable.

* chore: cleanup format args

Manually clean up [`uninlined_format_args`](https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args) clippy lint.  Some of these changes cause about 5% performance improvements because `format!("{}", &var)` does not need a reference and cannot be optimized by the compiler (but in thees cases the perf difference is less important)
  • Loading branch information
nyurik authored Feb 10, 2025
1 parent 31137e3 commit 7b0ee75
Show file tree
Hide file tree
Showing 14 changed files with 59 additions and 70 deletions.
10 changes: 4 additions & 6 deletions benches/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,17 +130,16 @@ fn main() {
&["undefined_from_scope", "undefined_from_isolate"][..],
),
] {
println!("Running {} ...", group_name);
println!("Running {group_name} ...");
for x in benches {
let code = format!(
"
function bench() {{ return {}(); }};
runs = {};
function bench() {{ return {x}(); }};
runs = {runs};
start = Date.now();
for (i = 0; i < runs; i++) bench();
Date.now() - start;
",
x, runs
);

let r = eval(scope, &code).unwrap();
Expand All @@ -150,8 +149,7 @@ fn main() {
let ns_per_run = total_ns / (runs as f64);
let mops_per_sec = (runs as f64) / (total_ms / 1000.0) / 1e6;
println!(
" {:.1} ns per run {:.1} million ops/sec → {}",
ns_per_run, mops_per_sec, x
" {ns_per_run:.1} ns per run {mops_per_sec:.1} million ops/sec → {x}"
);
}
}
Expand Down
55 changes: 25 additions & 30 deletions build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ fn main() {
"CARGO_ENCODED_RUSTFLAGS",
];
for env in envs {
println!("cargo:rerun-if-env-changed={}", env);
println!("cargo:rerun-if-env-changed={env}");
}

// Detect if trybuild tests are being compiled.
Expand Down Expand Up @@ -133,7 +133,7 @@ fn acquire_lock() -> LockFile {
let mut lockfile = LockFile::open(&lockfilepath)
.expect("Couldn't open lib download lockfile.");
lockfile.lock_with_pid().expect("Couldn't get lock");
println!("lockfile: {:?}", &lockfilepath);
println!("lockfile: {lockfilepath:?}");
lockfile
}

Expand Down Expand Up @@ -213,12 +213,12 @@ fn build_v8(is_asan: bool) {
gn_args.push("line_tables_only=false".into());
} else if let Some(clang_base_path) = find_compatible_system_clang() {
println!("clang_base_path (system): {}", clang_base_path.display());
gn_args.push(format!("clang_base_path={:?}", clang_base_path));
gn_args.push(format!("clang_base_path={clang_base_path:?}"));
gn_args.push("treat_warnings_as_errors=false".to_string());
} else {
println!("using Chromium's clang");
let clang_base_path = clang_download();
gn_args.push(format!("clang_base_path={:?}", clang_base_path));
gn_args.push(format!("clang_base_path={clang_base_path:?}"));

if target_os == "android" && target_arch == "aarch64" {
gn_args.push("treat_warnings_as_errors=false".to_string());
Expand Down Expand Up @@ -270,8 +270,8 @@ fn build_v8(is_asan: bool) {
if target_arch == "x86_64" {
maybe_install_sysroot("amd64");
}
gn_args.push(format!(r#"v8_target_cpu="{}""#, arch).to_string());
gn_args.push(format!(r#"target_cpu="{}""#, arch).to_string());
gn_args.push(format!(r#"v8_target_cpu="{arch}""#).to_string());
gn_args.push(format!(r#"target_cpu="{arch}""#).to_string());
gn_args.push(r#"target_os="android""#.to_string());
gn_args.push("treat_warnings_as_errors=false".to_string());
gn_args.push("use_sysroot=true".to_string());
Expand Down Expand Up @@ -302,14 +302,11 @@ fn build_v8(is_asan: bool) {
static CHROMIUM_URI: &str = "https://chromium.googlesource.com";
maybe_clone_repo(
"./third_party/android_platform",
&format!(
"{}/chromium/src/third_party/android_platform.git",
CHROMIUM_URI
),
&format!("{CHROMIUM_URI}/chromium/src/third_party/android_platform.git",),
);
maybe_clone_repo(
"./third_party/catapult",
&format!("{}/catapult.git", CHROMIUM_URI),
&format!("{CHROMIUM_URI}/catapult.git"),
);
}

Expand Down Expand Up @@ -351,11 +348,11 @@ fn maybe_clone_repo(dest: &str, repo: &str) {
}

fn maybe_install_sysroot(arch: &str) {
let sysroot_path = format!("build/linux/debian_sid_{}-sysroot", arch);
let sysroot_path = format!("build/linux/debian_sid_{arch}-sysroot");
if !PathBuf::from(sysroot_path).is_dir() {
assert!(Command::new(python())
.arg("./build/linux/sysroot_scripts/install-sysroot.py")
.arg(format!("--arch={}", arch))
.arg(format!("--arch={arch}"))
.status()
.unwrap()
.success());
Expand Down Expand Up @@ -431,10 +428,8 @@ fn static_lib_url() -> String {
let profile = prebuilt_profile();
let features = prebuilt_features_suffix();
format!(
"{}/v{}/{}.gz",
base,
version,
static_lib_name(&format!("{}_{}_{}", features, profile, target)),
"{base}/v{version}/{}.gz",
static_lib_name(&format!("{features}_{profile}_{target}")),
)
}

Expand Down Expand Up @@ -511,7 +506,7 @@ fn download_file(url: &str, filename: &Path) {

// Try downloading with python first. Python is a V8 build dependency,
// so this saves us from adding a Rust HTTP client dependency.
println!("Downloading (using Python) {}", url);
println!("Downloading (using Python) {url}");
let status = Command::new(python())
.arg("./tools/download_file.py")
.arg("--url")
Expand Down Expand Up @@ -554,7 +549,7 @@ fn download_file(url: &str, filename: &Path) {

fn download_static_lib_binaries() {
let url = static_lib_url();
println!("static lib URL: {}", url);
println!("static lib URL: {url}");

let dir = static_lib_dir();
std::fs::create_dir_all(&dir).unwrap();
Expand Down Expand Up @@ -653,7 +648,7 @@ fn print_link_flags() {
// Based on https://github.com/alexcrichton/cc-rs/blob/fba7feded71ee4f63cfe885673ead6d7b4f2f454/src/lib.rs#L2462
if let Ok(stdlib) = env::var("CXXSTDLIB") {
if !stdlib.is_empty() {
println!("cargo:rustc-link-lib=dylib={}", stdlib);
println!("cargo:rustc-link-lib=dylib={stdlib}");
}
} else {
let target = env::var("TARGET").unwrap();
Expand Down Expand Up @@ -692,20 +687,20 @@ fn print_link_flags() {

fn print_prebuilt_src_binding_path() {
if let Ok(binding) = env::var("RUSTY_V8_SRC_BINDING_PATH") {
println!("cargo:rustc-env=RUSTY_V8_SRC_BINDING_PATH={}", binding);
println!("cargo:rustc-env=RUSTY_V8_SRC_BINDING_PATH={binding}");
return;
}

let target = env::var("TARGET").unwrap();
let profile = prebuilt_profile();
let features = prebuilt_features_suffix();
let name = format!("src_binding{}_{}_{}.rs", features, profile, target);
let name = format!("src_binding{features}_{profile}_{target}.rs");

let src_binding_path = get_dirs().root.join("gen").join(name.clone());

if let Ok(base) = env::var("RUSTY_V8_MIRROR") {
let version = env::var("CARGO_PKG_VERSION").unwrap();
let url = format!("{}/v{}/{}", base, version, name);
let url = format!("{base}/v{version}/{name}");
download_file(&url, &src_binding_path);
}

Expand Down Expand Up @@ -778,7 +773,7 @@ fn clang_download() -> PathBuf {
}

fn cc_wrapper(gn_args: &mut Vec<String>, sccache_path: &Path) {
gn_args.push(format!("cc_wrapper={:?}", sccache_path));
gn_args.push(format!("cc_wrapper={sccache_path:?}"));
}

struct Dirs {
Expand Down Expand Up @@ -837,27 +832,27 @@ fn maybe_symlink_root_dir(dirs: &mut Dirs) {
let symlink = &*out.join("gn_root");
let target = &*root.canonicalize().unwrap();

println!("Creating symlink {:?} to {:?}", &symlink, &root);
println!("Creating symlink {symlink:?} to {root:?}");

let mut retries = 0;
loop {
match symlink.canonicalize() {
Ok(existing) if existing == target => break,
Ok(_) => remove_dir_all(symlink).expect("remove_dir_all failed"),
Err(err) => {
println!("symlink.canonicalize failed: {:?}", err);
println!("symlink.canonicalize failed: {err:?}");
// we're having very strange issues on GHA when the cache
// is restored, so trying this out temporarily
if let Err(err) = remove_dir_all(symlink) {
eprintln!("remove_dir_all failed: {:?}", err);
eprintln!("remove_dir_all failed: {err:?}");
if let Err(err) = remove_file(symlink) {
eprintln!("remove_file failed: {:?}", err);
eprintln!("remove_file failed: {err:?}");
}
}
match symlink_dir(target, symlink) {
Ok(_) => break,
Err(err) => {
println!("symlink_dir failed: {:?}", err);
println!("symlink_dir failed: {err:?}");
retries += 1;
std::thread::sleep(std::time::Duration::from_millis(
50 * retries,
Expand All @@ -884,7 +879,7 @@ pub fn is_debug() -> bool {
} else if m == "debug" {
true
} else {
panic!("unhandled PROFILE value {}", m)
panic!("unhandled PROFILE value {m}")
}
}

Expand Down
4 changes: 2 additions & 2 deletions examples/android/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ fn execute_script(
.map(|value| value.to_rust_string_lossy(try_catch))
.unwrap_or_else(|| "no stack trace".into());

panic!("{}", exception_string);
panic!("{exception_string}");
}
}

Expand All @@ -128,7 +128,7 @@ fn draw(
.map(|value| value.to_rust_string_lossy(try_catch))
.unwrap_or_else(|| "no stack trace".into());

panic!("{}", exception_string);
panic!("{exception_string}");
}
};

Expand Down
2 changes: 1 addition & 1 deletion examples/cppgc-object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,6 @@ fn execute_script(
|value| value.to_rust_string_lossy(try_catch),
);

panic!("{}", exception_string);
panic!("{exception_string}");
}
}
6 changes: 3 additions & 3 deletions examples/cppgc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ impl std::fmt::Display for Rope {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}", self.part)?;
if let Some(next) = self.next.borrow() {
write!(f, "{}", next)?;
write!(f, "{next}")?;
}
Ok(())
}
Expand Down Expand Up @@ -67,7 +67,7 @@ fn main() {
)
};

println!("{}", rope);
println!("{rope}");

// Manually trigger garbage collection.
heap.enable_detached_garbage_collections_for_testing();
Expand All @@ -80,7 +80,7 @@ fn main() {
}

// Should still be live here:
println!("{}", rope);
println!("{rope}");

println!("Collect: NoHeapPointers");
unsafe {
Expand Down
10 changes: 5 additions & 5 deletions examples/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ fn log_callback(
.unwrap()
.to_rust_string_lossy(scope);

println!("Logged: {}", message);
println!("Logged: {message}");
}

fn main() {
Expand All @@ -32,7 +32,7 @@ fn main() {
let mut scope = v8::HandleScope::new(&mut isolate);

let source = std::fs::read_to_string(&file)
.unwrap_or_else(|err| panic!("failed to open {}: {}", file, err));
.unwrap_or_else(|err| panic!("failed to open {file}: {err}"));
let source = v8::String::new(&mut scope, &source).unwrap();

let mut processor = JsHttpRequestProcessor::new(&mut scope, source, options);
Expand Down Expand Up @@ -223,7 +223,7 @@ where
.unwrap()
.to_rust_string_lossy(try_catch);

panic!("{}", exception_string);
panic!("{exception_string}");
}
}

Expand Down Expand Up @@ -251,7 +251,7 @@ where
.unwrap()
.to_rust_string_lossy(try_catch);

panic!("{}", exception_string);
panic!("{exception_string}");
}
}

Expand Down Expand Up @@ -376,7 +376,7 @@ where
let key = key.to_string(scope).unwrap().to_rust_string_lossy(scope);
let value = value.to_string(scope).unwrap().to_rust_string_lossy(scope);

println!("{}: {}", key, value);
println!("{key}: {value}");
}
}
}
12 changes: 6 additions & 6 deletions examples/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ fn run_shell(scope: &mut v8::HandleScope) {

execute_string(scope, &buf, "(shell)", true, true);
}
Err(error) => println!("error: {}", error),
Err(error) => println!("error: {error}"),
}
}
}
Expand Down Expand Up @@ -89,7 +89,7 @@ fn run_main(
}
arg => {
if arg.starts_with("--") {
eprintln!("Warning: unknown flag {}.\nTry --help for options", arg);
eprintln!("Warning: unknown flag {arg}.\nTry --help for options");
continue;
}

Expand Down Expand Up @@ -174,7 +174,7 @@ fn report_exceptions(mut try_catch: v8::TryCatch<v8::HandleScope>) {
let message = if let Some(message) = try_catch.message() {
message
} else {
eprintln!("{}", exception_string);
eprintln!("{exception_string}");
return;
};

Expand All @@ -191,7 +191,7 @@ fn report_exceptions(mut try_catch: v8::TryCatch<v8::HandleScope>) {
);
let line_number = message.get_line_number(&mut try_catch).unwrap_or_default();

eprintln!("{}:{}: {}", filename, line_number, exception_string);
eprintln!("{filename}:{line_number}: {exception_string}");

// Print line of source code.
let source_line = message
Expand All @@ -202,7 +202,7 @@ fn report_exceptions(mut try_catch: v8::TryCatch<v8::HandleScope>) {
.to_rust_string_lossy(&mut try_catch)
})
.unwrap();
eprintln!("{}", source_line);
eprintln!("{source_line}");

// Print wavy underline (GetUnderline is deprecated).
let start_column = message.get_start_column();
Expand Down Expand Up @@ -231,6 +231,6 @@ fn report_exceptions(mut try_catch: v8::TryCatch<v8::HandleScope>) {
.map(|s| s.to_rust_string_lossy(&mut try_catch));

if let Some(stack_trace) = stack_trace {
eprintln!("{}", stack_trace);
eprintln!("{stack_trace}");
}
}
4 changes: 2 additions & 2 deletions src/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,10 +253,10 @@ impl Display for DataError {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
match self {
Self::BadType { expected, actual } => {
write!(f, "expected type `{}`, got `{}`", expected, actual)
write!(f, "expected type `{expected}`, got `{actual}`")
}
Self::NoData { expected } => {
write!(f, "expected `Some({})`, found `None`", expected)
write!(f, "expected `Some({expected})`, found `None`")
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/inspector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -758,8 +758,8 @@ impl StringView<'static> {
impl fmt::Display for StringView<'_> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
Self::U16(v) => write!(f, "{}", v),
Self::U8(v) => write!(f, "{}", v),
Self::U16(v) => write!(f, "{v}"),
Self::U8(v) => write!(f, "{v}"),
}
}
}
Expand Down
Loading

0 comments on commit 7b0ee75

Please sign in to comment.