Turn collections of String into collections of Box<str> (#2594)

* Change `String` by `Box<str>` inside `RegexSet`

* Turn `raw_lines` into a `Box<str>`

* Turn keys and values of `module_lines` into `Box<str>`s

* Turn `input_headers` into a `Vec<Box<str>>`

* Turn `clang_args` into a `Vec<Box<str>>`

* Turn keys and values of `input_header_contents` into `Box<str>`s

* Add suggestions

* Run rustfmt

Author: pvdrz

bindgen/clang.rs

@@ -1813,14 +1813,14 @@ impl TranslationUnit {
     pub(crate) fn parse(
         ix: &Index,
         file: &str,
-        cmd_args: &[String],
+        cmd_args: &[Box<str>],
         unsaved: &[UnsavedFile],
         opts: CXTranslationUnit_Flags,
     ) -> Option<TranslationUnit> {
         let fname = CString::new(file).unwrap();
         let _c_args: Vec<CString> = cmd_args
             .iter()
-            .map(|s| CString::new(s.clone()).unwrap())
+            .map(|s| CString::new(s.clone().into_boxed_bytes()).unwrap())
             .collect();
         let c_args: Vec<*const c_char> =
             _c_args.iter().map(|s| s.as_ptr()).collect();
@@ -1923,9 +1923,9 @@ pub(crate) struct UnsavedFile {
 
 impl UnsavedFile {
     /// Construct a new unsaved file with the given `name` and `contents`.
-    pub(crate) fn new(name: String, contents: String) -> UnsavedFile {
-        let name = CString::new(name).unwrap();
-        let contents = CString::new(contents).unwrap();
+    pub(crate) fn new(name: &str, contents: &str) -> UnsavedFile {
+        let name = CString::new(name.as_bytes()).unwrap();
+        let contents = CString::new(contents.as_bytes()).unwrap();
         let x = CXUnsavedFile {
             Filename: name.as_ptr(),
             Contents: contents.as_ptr(),

bindgen/codegen/mod.rs

@@ -601,7 +601,10 @@ impl CodeGenerator for Module {
         let inner_items = result.inner(|result| {
             result.push(root_import(ctx, item));
 
-            let path = item.namespace_aware_canonical_path(ctx).join("::");
+            let path = item
+                .namespace_aware_canonical_path(ctx)
+                .join("::")
+                .into_boxed_str();
             if let Some(raw_lines) = ctx.options().module_lines.get(&path) {
                 for raw_line in raw_lines {
                     found_any = true;

bindgen/deps.rs

@@ -8,11 +8,11 @@ pub(crate) struct DepfileSpec {
 }
 
 impl DepfileSpec {
-    pub fn write(&self, deps: &BTreeSet<String>) -> std::io::Result<()> {
+    pub fn write(&self, deps: &BTreeSet<Box<str>>) -> std::io::Result<()> {
         std::fs::write(&self.depfile_path, self.to_string(deps))
     }
 
-    fn to_string(&self, deps: &BTreeSet<String>) -> String {
+    fn to_string(&self, deps: &BTreeSet<Box<str>>) -> String {
         // Transforms a string by escaping spaces and backslashes.
         let escape = |s: &str| s.replace('\\', "\\\\").replace(' ', "\\ ");
 
@@ -35,14 +35,14 @@ mod tests {
             depfile_path: PathBuf::new(),
         };
 
-        let deps: BTreeSet<String> = vec![
-            r"/absolute/path".to_owned(),
-            r"C:\win\absolute\path".to_owned(),
-            r"../relative/path".to_owned(),
-            r"..\win\relative\path".to_owned(),
-            r"../path/with spaces/in/it".to_owned(),
-            r"..\win\path\with spaces\in\it".to_owned(),
-            r"path\with/mixed\separators".to_owned(),
+        let deps: BTreeSet<_> = vec![
+            r"/absolute/path".into(),
+            r"C:\win\absolute\path".into(),
+            r"../relative/path".into(),
+            r"..\win\relative\path".into(),
+            r"../path/with spaces/in/it".into(),
+            r"..\win\path\with spaces\in\it".into(),
+            r"path\with/mixed\separators".into(),
         ]
         .into_iter()
         .collect();

bindgen/ir/context.rs

@@ -365,7 +365,7 @@ pub(crate) struct BindgenContext {
     includes: StdHashMap<String, (String, usize)>,
 
     /// A set of all the included filenames.
-    deps: BTreeSet<String>,
+    deps: BTreeSet<Box<str>>,
 
     /// The active replacements collected from replaces="xxx" annotations.
     replacements: HashMap<Vec<String>, ItemId>,
@@ -664,12 +664,12 @@ If you encounter an error missing from this list, please file an issue or a PR!"
     }
 
     /// Add an included file.
-    pub(crate) fn add_dep(&mut self, dep: String) {
+    pub(crate) fn add_dep(&mut self, dep: Box<str>) {
         self.deps.insert(dep);
     }
 
     /// Get any included files.
-    pub(crate) fn deps(&self) -> &BTreeSet<String> {
+    pub(crate) fn deps(&self) -> &BTreeSet<Box<str>> {
         &self.deps
     }
 

bindgen/ir/item.rs

@@ -1465,7 +1465,7 @@ impl Item {
                                 cb.include_file(&included_file);
                             }
 
-                            ctx.add_dep(included_file);
+                            ctx.add_dep(included_file.into_boxed_str());
                         }
                     }
                 }

bindgen/lib.rs

@@ -94,15 +94,15 @@ fn file_is_cpp(name_file: &str) -> bool {
         name_file.ends_with(".h++")
 }
 
-fn args_are_cpp(clang_args: &[String]) -> bool {
+fn args_are_cpp(clang_args: &[Box<str>]) -> bool {
     for w in clang_args.windows(2) {
-        if w[0] == "-xc++" || w[1] == "-xc++" {
+        if w[0].as_ref() == "-xc++" || w[1].as_ref() == "-xc++" {
             return true;
         }
-        if w[0] == "-x" && w[1] == "c++" {
+        if w[0].as_ref() == "-x" && w[1].as_ref() == "c++" {
             return true;
         }
-        if w[0] == "-include" && file_is_cpp(&w[1]) {
+        if w[0].as_ref() == "-include" && file_is_cpp(w[1].as_ref()) {
             return true;
         }
     }
@@ -319,22 +319,26 @@ impl Builder {
     /// Generate the Rust bindings using the options built up thus far.
     pub fn generate(mut self) -> Result<Bindings, BindgenError> {
         // Add any extra arguments from the environment to the clang command line.
-        self.options
-            .clang_args
-            .extend(get_extra_clang_args(&self.options.parse_callbacks));
+        self.options.clang_args.extend(
+            get_extra_clang_args(&self.options.parse_callbacks)
+                .into_iter()
+                .map(String::into_boxed_str),
+        );
 
         // Transform input headers to arguments on the clang command line.
         self.options.clang_args.extend(
             self.options.input_headers
                 [..self.options.input_headers.len().saturating_sub(1)]
                 .iter()
-                .flat_map(|header| ["-include".into(), header.to_string()]),
+                .flat_map(|header| ["-include".into(), header.clone()]),
         );
 
         let input_unsaved_files =
             std::mem::take(&mut self.options.input_header_contents)
                 .into_iter()
-                .map(|(name, contents)| clang::UnsavedFile::new(name, contents))
+                .map(|(name, contents)| {
+                    clang::UnsavedFile::new(name.as_ref(), contents.as_ref())
+                })
                 .collect::<Vec<_>>();
 
         Bindings::generate(self.options, input_unsaved_files)
@@ -401,7 +405,7 @@ impl Builder {
             .stdout(Stdio::piped());
 
         for a in &self.options.clang_args {
-            cmd.arg(a);
+            cmd.arg(a.as_ref());
         }
 
         for a in get_extra_clang_args(&self.options.parse_callbacks) {
@@ -668,16 +672,16 @@ pub(crate) const HOST_TARGET: &str =
 
 // Some architecture triplets are different between rust and libclang, see #1211
 // and duplicates.
-fn rust_to_clang_target(rust_target: &str) -> String {
+fn rust_to_clang_target(rust_target: &str) -> Box<str> {
     if rust_target.starts_with("aarch64-apple-") {
         let mut clang_target = "arm64-apple-".to_owned();
         clang_target
             .push_str(rust_target.strip_prefix("aarch64-apple-").unwrap());
-        return clang_target;
+        return clang_target.into();
     } else if rust_target.starts_with("riscv64gc-") {
         let mut clang_target = "riscv64-".to_owned();
         clang_target.push_str(rust_target.strip_prefix("riscv64gc-").unwrap());
-        return clang_target;
+        return clang_target.into();
     } else if rust_target.ends_with("-espidf") {
         let mut clang_target =
             rust_target.strip_suffix("-espidf").unwrap().to_owned();
@@ -686,32 +690,32 @@ fn rust_to_clang_target(rust_target: &str) -> String {
             clang_target = "riscv32-".to_owned() +
                 clang_target.strip_prefix("riscv32imc-").unwrap();
         }
-        return clang_target;
+        return clang_target.into();
     } else if rust_target.starts_with("riscv32imc-") {
         let mut clang_target = "riscv32-".to_owned();
         clang_target.push_str(rust_target.strip_prefix("riscv32imc-").unwrap());
-        return clang_target;
+        return clang_target.into();
     } else if rust_target.starts_with("riscv32imac-") {
         let mut clang_target = "riscv32-".to_owned();
         clang_target
             .push_str(rust_target.strip_prefix("riscv32imac-").unwrap());
-        return clang_target;
+        return clang_target.into();
     }
-    rust_target.to_owned()
+    rust_target.into()
 }
 
 /// Returns the effective target, and whether it was explicitly specified on the
 /// clang flags.
-fn find_effective_target(clang_args: &[String]) -> (String, bool) {
+fn find_effective_target(clang_args: &[Box<str>]) -> (Box<str>, bool) {
     let mut args = clang_args.iter();
     while let Some(opt) = args.next() {
         if opt.starts_with("--target=") {
             let mut split = opt.split('=');
             split.next();
-            return (split.next().unwrap().to_owned(), true);
+            return (split.next().unwrap().into(), true);
         }
 
-        if opt == "-target" {
+        if opt.as_ref() == "-target" {
             if let Some(target) = args.next() {
                 return (target.clone(), true);
             }
@@ -756,9 +760,10 @@ impl Bindings {
         // opening libclang.so, it has to be the same architecture and thus the
         // check is fine.
         if !explicit_target && !is_host_build {
-            options
-                .clang_args
-                .insert(0, format!("--target={}", effective_target));
+            options.clang_args.insert(
+                0,
+                format!("--target={}", effective_target).into_boxed_str(),
+            );
         };
 
         fn detect_include_paths(options: &mut BindgenOptions) {
@@ -779,7 +784,7 @@ impl Bindings {
                             return false;
                         }
 
-                        let arg = &**arg;
+                        let arg = arg.as_ref();
 
                         // https://clang.llvm.org/docs/ClangCommandLineReference.html
                         // -isystem and -isystem-after are harmless.
@@ -796,7 +801,7 @@ impl Bindings {
 
                         true
                     })
-                    .cloned()
+                    .map(|arg| arg.clone().into())
                     .collect::<Vec<_>>()
             };
 
@@ -828,8 +833,8 @@ impl Bindings {
             if let Some(search_paths) = search_paths {
                 for path in search_paths.into_iter() {
                     if let Ok(path) = path.into_os_string().into_string() {
-                        options.clang_args.push("-isystem".to_owned());
-                        options.clang_args.push(path);
+                        options.clang_args.push("-isystem".into());
+                        options.clang_args.push(path.into_boxed_str());
                     }
                 }
             }
@@ -849,7 +854,7 @@ impl Bindings {
         }
 
         if let Some(h) = options.input_headers.last() {
-            let path = Path::new(h);
+            let path = Path::new(h.as_ref());
             if let Ok(md) = std::fs::metadata(path) {
                 if md.is_dir() {
                     return Err(BindgenError::FolderAsHeader(path.into()));
@@ -859,18 +864,17 @@ impl Bindings {
                         path.into(),
                     ));
                 }
-                let h = h.clone();
-                options.clang_args.push(h);
+                options.clang_args.push(h.clone());
             } else {
                 return Err(BindgenError::NotExist(path.into()));
             }
         }
 
         for (idx, f) in input_unsaved_files.iter().enumerate() {
             if idx != 0 || !options.input_headers.is_empty() {
-                options.clang_args.push("-include".to_owned());
+                options.clang_args.push("-include".into());
             }
-            options.clang_args.push(f.name.to_str().unwrap().to_owned())
+            options.clang_args.push(f.name.to_str().unwrap().into())
         }
 
         debug!("Fixed-up options: {:?}", options);
@@ -1285,33 +1289,36 @@ fn commandline_flag_unit_test_function() {
 
 #[test]
 fn test_rust_to_clang_target() {
-    assert_eq!(rust_to_clang_target("aarch64-apple-ios"), "arm64-apple-ios");
+    assert_eq!(
+        rust_to_clang_target("aarch64-apple-ios").as_ref(),
+        "arm64-apple-ios"
+    );
 }
 
 #[test]
 fn test_rust_to_clang_target_riscv() {
     assert_eq!(
-        rust_to_clang_target("riscv64gc-unknown-linux-gnu"),
+        rust_to_clang_target("riscv64gc-unknown-linux-gnu").as_ref(),
         "riscv64-unknown-linux-gnu"
     );
     assert_eq!(
-        rust_to_clang_target("riscv32imc-unknown-none-elf"),
+        rust_to_clang_target("riscv32imc-unknown-none-elf").as_ref(),
         "riscv32-unknown-none-elf"
     );
     assert_eq!(
-        rust_to_clang_target("riscv32imac-unknown-none-elf"),
+        rust_to_clang_target("riscv32imac-unknown-none-elf").as_ref(),
         "riscv32-unknown-none-elf"
     );
 }
 
 #[test]
 fn test_rust_to_clang_target_espidf() {
     assert_eq!(
-        rust_to_clang_target("riscv32imc-esp-espidf"),
+        rust_to_clang_target("riscv32imc-esp-espidf").as_ref(),
         "riscv32-esp-elf"
     );
     assert_eq!(
-        rust_to_clang_target("xtensa-esp32-espidf"),
+        rust_to_clang_target("xtensa-esp32-espidf").as_ref(),
         "xtensa-esp32-elf"
     );
 }

bindgen/options/as_args.rs

@@ -24,7 +24,7 @@ impl AsArgs for bool {
 impl AsArgs for RegexSet {
     fn as_args(&self, args: &mut Vec<String>, flag: &str) {
         for item in self.get_items() {
-            args.extend_from_slice(&[flag.to_owned(), item.clone()]);
+            args.extend_from_slice(&[flag.to_owned(), item.clone().into()]);
         }
     }
 }