Skip to content
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

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

Merged
merged 8 commits into from
Aug 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading