Skip to content

Commit

Permalink
Turn collections of String into collections of Box<str> (#2594)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
pvdrz committed Aug 8, 2023
1 parent bf5e79b commit 6a9a089
Show file tree
Hide file tree
Showing 9 changed files with 94 additions and 84 deletions.
10 changes: 5 additions & 5 deletions bindgen/clang.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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(),
Expand Down
5 changes: 4 additions & 1 deletion bindgen/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
20 changes: 10 additions & 10 deletions bindgen/deps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(' ', "\\ ");

Expand All @@ -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();
Expand Down
6 changes: 3 additions & 3 deletions bindgen/ir/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion bindgen/ir/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1465,7 +1465,7 @@ impl Item {
cb.include_file(&included_file);
}

ctx.add_dep(included_file);
ctx.add_dep(included_file.into_boxed_str());
}
}
}
Expand Down
83 changes: 45 additions & 38 deletions bindgen/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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();
Expand All @@ -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);
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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.
Expand All @@ -796,7 +801,7 @@ impl Bindings {

true
})
.cloned()
.map(|arg| arg.clone().into())
.collect::<Vec<_>>()
};

Expand Down Expand Up @@ -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());
}
}
}
Expand All @@ -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()));
Expand All @@ -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);
Expand Down Expand Up @@ -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"
);
}
2 changes: 1 addition & 1 deletion bindgen/options/as_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()]);
}
}
}
Expand Down
Loading

0 comments on commit 6a9a089

Please sign in to comment.