From 69d615da622cecd93623fada829db949092d1a3e Mon Sep 17 00:00:00 2001 From: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> Date: Fri, 29 Nov 2024 00:48:45 +1100 Subject: [PATCH 1/6] Optimize RustcCodegenFlags Use `&str` instead of `String`. --- src/flags.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/flags.rs b/src/flags.rs index a94743fc..a00fa3ea 100644 --- a/src/flags.rs +++ b/src/flags.rs @@ -5,16 +5,16 @@ use std::ffi::OsString; use std::path::Path; #[derive(Debug, PartialEq, Default)] -pub(crate) struct RustcCodegenFlags { - branch_protection: Option, - code_model: Option, +pub(crate) struct RustcCodegenFlags<'a> { + branch_protection: Option<&'a str>, + code_model: Option<&'a str>, no_vectorize_loops: bool, no_vectorize_slp: bool, - profile_generate: Option, - profile_use: Option, - control_flow_guard: Option, - lto: Option, - relocation_model: Option, + profile_generate: Option<&'a str>, + profile_use: Option<&'a str>, + control_flow_guard: Option<&'a str>, + lto: Option<&'a str>, + relocation_model: Option<&'a str>, embed_bitcode: Option, force_frame_pointers: Option, link_dead_code: Option, @@ -89,12 +89,12 @@ impl RustcCodegenFlags { } let (flag, value) = if let Some((flag, value)) = flag.split_once('=') { - (flag, Some(value.to_owned())) + (flag, Some(value)) } else { (flag, None) }; - fn flag_ok_or(flag: Option, msg: &'static str) -> Result { + fn flag_ok_or(flag: Option<&str>, msg: &'static str) -> Result<&str, Error> { flag.ok_or(Error::new(ErrorKind::InvalidFlag, msg)) } From 0270aff0008173cd4701ebd4cd00386074bec2b9 Mon Sep 17 00:00:00 2001 From: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> Date: Fri, 29 Nov 2024 00:54:05 +1100 Subject: [PATCH 2/6] Fix lifetime annotations --- src/flags.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/flags.rs b/src/flags.rs index a00fa3ea..3d2d278c 100644 --- a/src/flags.rs +++ b/src/flags.rs @@ -22,9 +22,9 @@ pub(crate) struct RustcCodegenFlags<'a> { soft_float: Option, } -impl RustcCodegenFlags { +impl RustcCodegenFlags<'_> { // Parse flags obtained from CARGO_ENCODED_RUSTFLAGS - pub(crate) fn parse(rustflags_env: &str) -> Result { + pub(crate) fn parse(rustflags_env: &str) -> Result, Error> { fn is_flag_prefix(flag: &str) -> bool { [ "-Z", @@ -94,7 +94,7 @@ impl RustcCodegenFlags { (flag, None) }; - fn flag_ok_or(flag: Option<&str>, msg: &'static str) -> Result<&str, Error> { + fn flag_ok_or<'flag>(flag: Option<&'flag str>, msg: &'static str) -> Result<&'flag str, Error> { flag.ok_or(Error::new(ErrorKind::InvalidFlag, msg)) } From 0d0bc63a581c92f714b0e258db626e98901d3cdc Mon Sep 17 00:00:00 2001 From: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> Date: Fri, 29 Nov 2024 01:05:29 +1100 Subject: [PATCH 3/6] Fix compilation --- src/flags.rs | 54 +++++++++++++++++++++++++--------------------------- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/src/flags.rs b/src/flags.rs index 3d2d278c..a1fe63dd 100644 --- a/src/flags.rs +++ b/src/flags.rs @@ -22,9 +22,9 @@ pub(crate) struct RustcCodegenFlags<'a> { soft_float: Option, } -impl RustcCodegenFlags<'_> { +impl<'this> RustcCodegenFlags<'this> { // Parse flags obtained from CARGO_ENCODED_RUSTFLAGS - pub(crate) fn parse(rustflags_env: &str) -> Result, Error> { + pub(crate) fn parse(rustflags_env: &'this str) -> Result { fn is_flag_prefix(flag: &str) -> bool { [ "-Z", @@ -176,13 +176,13 @@ impl RustcCodegenFlags<'_> { match family { ToolFamily::Clang { .. } | ToolFamily::Gnu => { // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-mbranch-protection - if let Some(value) = &self.branch_protection { + if let Some(value) = self.branch_protection { push_if_supported( format!("-mbranch-protection={}", value.replace(",", "+")).into(), ); } // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-mcmodel - if let Some(value) = &self.code_model { + if let Some(value) = self.code_model { push_if_supported(format!("-mcmodel={value}").into()); } // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fno-vectorize @@ -194,16 +194,16 @@ impl RustcCodegenFlags<'_> { push_if_supported("-fno-slp-vectorize".into()); } // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fprofile-generate - if let Some(value) = &self.profile_generate { + if let Some(value) = self.profile_generate { push_if_supported(format!("-fprofile-generate={value}").into()); } // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fprofile-use - if let Some(value) = &self.profile_use { + if let Some(value) = self.profile_use { push_if_supported(format!("-fprofile-use={value}").into()); } // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-mguard - if let Some(value) = &self.control_flow_guard { - let cc_val = match value.as_str() { + if let Some(value) = self.control_flow_guard { + let cc_val = match value { "y" | "yes" | "on" | "true" | "checks" => Some("cf"), "nochecks" => Some("cf-nochecks"), "n" | "no" | "off" | "false" => Some("none"), @@ -214,8 +214,8 @@ impl RustcCodegenFlags<'_> { } } // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-flto - if let Some(value) = &self.lto { - let cc_val = match value.as_str() { + if let Some(value) = self.lto { + let cc_val = match value { "y" | "yes" | "on" | "true" | "fat" => Some("full"), "thin" => Some("thin"), _ => None, @@ -227,8 +227,8 @@ impl RustcCodegenFlags<'_> { // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fPIC // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fPIE // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-mdynamic-no-pic - if let Some(value) = &self.relocation_model { - let cc_flag = match value.as_str() { + if let Some(value) = self.relocation_model { + let cc_flag = match value { "pic" => Some("-fPIC"), "pie" => Some("-fPIE"), "dynamic-no-pic" => Some("-mdynamic-no-pic"), @@ -239,14 +239,14 @@ impl RustcCodegenFlags<'_> { } } // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fembed-bitcode - if let Some(value) = &self.embed_bitcode { - let cc_val = if *value { "all" } else { "off" }; + if let Some(value) = self.embed_bitcode { + let cc_val = if value { "all" } else { "off" }; push_if_supported(format!("-fembed-bitcode={cc_val}").into()); } // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fno-omit-frame-pointer // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fomit-frame-pointer - if let Some(value) = &self.force_frame_pointers { - let cc_flag = if *value { + if let Some(value) = self.force_frame_pointers { + let cc_flag = if value { "-fno-omit-frame-pointer" } else { "-fomit-frame-pointer" @@ -254,15 +254,13 @@ impl RustcCodegenFlags<'_> { push_if_supported(cc_flag.into()); } // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-dead_strip - if let Some(value) = &self.link_dead_code { - if !value { - push_if_supported("-dead_strip".into()); - } + if let Some(false) = self.link_dead_code { + push_if_supported("-dead_strip".into()); } // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-mno-red-zone // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-mred-zone - if let Some(value) = &self.no_redzone { - let cc_flag = if *value { + if let Some(value) = self.no_redzone { + let cc_flag = if value { "-mno-red-zone" } else { "-mred-zone" @@ -271,8 +269,8 @@ impl RustcCodegenFlags<'_> { } // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-msoft-float // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-mno-soft-float - if let Some(value) = &self.soft_float { - let cc_flag = if *value { + if let Some(value) = self.soft_float { + let cc_flag = if value { "-msoft-float" } else { "-mno-soft-float" @@ -282,8 +280,8 @@ impl RustcCodegenFlags<'_> { } ToolFamily::Msvc { .. } => { // https://learn.microsoft.com/en-us/cpp/build/reference/guard-enable-control-flow-guard - if let Some(value) = &self.control_flow_guard { - let cc_val = match value.as_str() { + if let Some(value) = self.control_flow_guard { + let cc_val = match value { "y" | "yes" | "on" | "true" | "checks" => Some("cf"), "n" | "no" | "off" | "false" => Some("cf-"), _ => None, @@ -293,8 +291,8 @@ impl RustcCodegenFlags<'_> { } } // https://learn.microsoft.com/en-us/cpp/build/reference/oy-frame-pointer-omission - if let Some(value) = &self.force_frame_pointers { - let cc_flag = if *value { "/Oy-" } else { "/Oy" }; + if let Some(value) = self.force_frame_pointers { + let cc_flag = if value { "/Oy-" } else { "/Oy" }; push_if_supported(cc_flag.into()); } } From 8cdfe89e7ef4bf073419ccb5f1a5a85ec85fdf88 Mon Sep 17 00:00:00 2001 From: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> Date: Fri, 29 Nov 2024 19:39:28 +1100 Subject: [PATCH 4/6] Fix lifetime in flags.rs And remove unnecessary `.into()` --- src/flags.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/flags.rs b/src/flags.rs index a1fe63dd..eb91d740 100644 --- a/src/flags.rs +++ b/src/flags.rs @@ -78,7 +78,7 @@ impl<'this> RustcCodegenFlags<'this> { Ok(codegen_flags) } - fn set_rustc_flag(&mut self, flag: &str) -> Result<(), Error> { + fn set_rustc_flag(&mut self, flag: &'this str) -> Result<(), Error> { // Convert a textual representation of a bool-like rustc flag argument into an actual bool fn arg_to_bool(arg: impl AsRef) -> Option { match arg.as_ref() { @@ -117,9 +117,9 @@ impl<'this> RustcCodegenFlags<'this> { self.profile_use = Some(flag_ok_or(value, "-Cprofile-use must have a value")?); } // https://doc.rust-lang.org/rustc/codegen-options/index.html#control-flow-guard - "-Ccontrol-flow-guard" => self.control_flow_guard = value.or(Some("true".into())), + "-Ccontrol-flow-guard" => self.control_flow_guard = value.or(Some("true")), // https://doc.rust-lang.org/rustc/codegen-options/index.html#lto - "-Clto" => self.lto = value.or(Some("true".into())), + "-Clto" => self.lto = value.or(Some("true")), // https://doc.rust-lang.org/rustc/codegen-options/index.html#relocation-model "-Crelocation-model" => { self.relocation_model = From e598ec995202cca8327346dfdd692e232808d05c Mon Sep 17 00:00:00 2001 From: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> Date: Fri, 29 Nov 2024 20:13:13 +1100 Subject: [PATCH 5/6] Fix compilation in flags.rs: Avoid allocation in prefix handling --- src/flags.rs | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/src/flags.rs b/src/flags.rs index eb91d740..d3da3b26 100644 --- a/src/flags.rs +++ b/src/flags.rs @@ -45,19 +45,19 @@ impl<'this> RustcCodegenFlags<'this> { .contains(&flag) } - fn handle_flag_prefix<'a>(prev: &'a str, curr: &'a str) -> Cow<'a, str> { + fn handle_flag_prefix<'a>(prev: &'a str, curr: &'a str) -> (&'a str, &'a str) { match prev { - "--codegen" | "-C" => Cow::from(format!("-C{}", curr)), + "--codegen" | "-C" => ("-C", curr), // Handle flags passed like --codegen=code-model=small - _ if curr.starts_with("--codegen=") => Cow::from(format!("-C{}", &curr[10..])), - "-Z" => Cow::from(format!("-Z{}", curr)), - "-L" | "-l" | "-o" => Cow::from(format!("{}{}", prev, curr)), + _ if curr.starts_with("--codegen=") => ("-C", &curr[10..]), + "-Z" => ("-Z", curr), + "-L" | "-l" | "-o" => (prev, curr), // Handle lint flags - "-W" | "--warn" => Cow::from(format!("-W{}", curr)), - "-A" | "--allow" => Cow::from(format!("-A{}", curr)), - "-D" | "--deny" => Cow::from(format!("-D{}", curr)), - "-F" | "--forbid" => Cow::from(format!("-F{}", curr)), - _ => Cow::from(curr), + "-W" | "--warn" => ("-W", curr), + "-A" | "--allow" => ("-A", curr), + "-D" | "--deny" => ("-D", curr), + "-F" | "--forbid" => ("-F", curr), + _ => ("", curr), } } @@ -71,14 +71,14 @@ impl<'this> RustcCodegenFlags<'this> { continue; } - let rustc_flag = handle_flag_prefix(prev, curr); - codegen_flags.set_rustc_flag(&rustc_flag)?; + let (prefix, rustc_flag) = handle_flag_prefix(prev, curr); + codegen_flags.set_rustc_flag(prefix, rustc_flag)?; } Ok(codegen_flags) } - fn set_rustc_flag(&mut self, flag: &'this str) -> Result<(), Error> { + fn set_rustc_flag(&mut self, prefix: &str, flag: &'this str) -> Result<(), Error> { // Convert a textual representation of a bool-like rustc flag argument into an actual bool fn arg_to_bool(arg: impl AsRef) -> Option { match arg.as_ref() { @@ -93,12 +93,17 @@ impl<'this> RustcCodegenFlags<'this> { } else { (flag, None) }; + let flag = if prefix.is_empty() { + Cow::Borrowed(flag) + } else { + Cow::Owned(format!("{prefix}{flag}")) + }; fn flag_ok_or<'flag>(flag: Option<&'flag str>, msg: &'static str) -> Result<&'flag str, Error> { flag.ok_or(Error::new(ErrorKind::InvalidFlag, msg)) } - match flag { + match flag.as_ref() { // https://doc.rust-lang.org/rustc/codegen-options/index.html#code-model "-Ccode-model" => { self.code_model = Some(flag_ok_or(value, "-Ccode-model must have a value")?); From 9de740443c306b170cc1b1784606937030107114 Mon Sep 17 00:00:00 2001 From: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> Date: Fri, 29 Nov 2024 20:21:37 +1100 Subject: [PATCH 6/6] FIx cargo-fmt in flags.rs --- src/flags.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/flags.rs b/src/flags.rs index d3da3b26..81834cf6 100644 --- a/src/flags.rs +++ b/src/flags.rs @@ -99,7 +99,10 @@ impl<'this> RustcCodegenFlags<'this> { Cow::Owned(format!("{prefix}{flag}")) }; - fn flag_ok_or<'flag>(flag: Option<&'flag str>, msg: &'static str) -> Result<&'flag str, Error> { + fn flag_ok_or<'flag>( + flag: Option<&'flag str>, + msg: &'static str, + ) -> Result<&'flag str, Error> { flag.ok_or(Error::new(ErrorKind::InvalidFlag, msg)) } @@ -265,11 +268,7 @@ impl<'this> RustcCodegenFlags<'this> { // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-mno-red-zone // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-mred-zone if let Some(value) = self.no_redzone { - let cc_flag = if value { - "-mno-red-zone" - } else { - "-mred-zone" - }; + let cc_flag = if value { "-mno-red-zone" } else { "-mred-zone" }; push_if_supported(cc_flag.into()); } // https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-msoft-float