s/ `String` / `Rc<str>` / in `NamespaceQualifier` and `ir::Identifier`.
This CL was motivated by trying to improve the efficiency of
`NamespaceQualifier`'s `Clone` implementation - cloning of `Rc<str>`
requires just increasing the ref-count and is therefore cheaper than
cloning of `String` which requires a heap allocation and copying the
string contents.
This CL also changes the type of `ir::Identifier` because this is
the representation of `ir::Namespace::name`.
`Rc<str>` instead of `String` is possible because these strings are
never mutated.
PiperOrigin-RevId: 490605310
diff --git a/common/code_gen_utils.rs b/common/code_gen_utils.rs
index 78b151a..7087243 100644
--- a/common/code_gen_utils.rs
+++ b/common/code_gen_utils.rs
@@ -59,7 +59,7 @@
// catch some error conditions early (e.g. an empty path component may trigger a
// panic in `make_rs_ident`; a reserved C++ keyword might trigger a late error
// in `format_for_cc` / `format_cc_ident`).
-pub struct NamespaceQualifier(pub Vec<String>);
+pub struct NamespaceQualifier(pub Vec<Rc<str>>);
impl NamespaceQualifier {
pub fn format_for_rs(&self) -> TokenStream {
@@ -435,7 +435,7 @@
}
fn create_namespace_qualifier_for_tests(input: &[&str]) -> NamespaceQualifier {
- NamespaceQualifier(input.into_iter().map(|s| s.to_string()).collect())
+ NamespaceQualifier(input.into_iter().map(|&s| s.into()).collect())
}
#[test]
diff --git a/rs_bindings_from_cc/ir.rs b/rs_bindings_from_cc/ir.rs
index d60ef34..51bf7af 100644
--- a/rs_bindings_from_cc/ir.rs
+++ b/rs_bindings_from_cc/ir.rs
@@ -162,7 +162,7 @@
#[derive(PartialEq, Eq, Hash, Clone, Deserialize)]
pub struct Identifier {
- pub identifier: String,
+ pub identifier: Rc<str>,
}
impl fmt::Debug for Identifier {
@@ -254,7 +254,7 @@
impl UnqualifiedIdentifier {
pub fn identifier_as_str(&self) -> Option<&str> {
match self {
- UnqualifiedIdentifier::Identifier(identifier) => Some(identifier.identifier.as_str()),
+ UnqualifiedIdentifier::Identifier(identifier) => Some(identifier.identifier.as_ref()),
_ => None,
}
}
@@ -878,7 +878,7 @@
#[test]
fn test_identifier_debug_print() {
- assert_eq!(format!("{:?}", Identifier { identifier: "hello".to_string() }), "\"hello\"");
+ assert_eq!(format!("{:?}", Identifier { identifier: "hello".into() }), "\"hello\"");
}
#[test]
@@ -886,7 +886,7 @@
assert_eq!(
format!(
"{:?}",
- UnqualifiedIdentifier::Identifier(Identifier { identifier: "hello".to_string() })
+ UnqualifiedIdentifier::Identifier(Identifier { identifier: "hello".into() })
),
"\"hello\""
);
diff --git a/rs_bindings_from_cc/ir_from_cc_test.rs b/rs_bindings_from_cc/ir_from_cc_test.rs
index 521ada9..55cf117 100644
--- a/rs_bindings_from_cc/ir_from_cc_test.rs
+++ b/rs_bindings_from_cc/ir_from_cc_test.rs
@@ -596,7 +596,7 @@
.functions()
.map(|f| {
if let UnqualifiedIdentifier::Identifier(id) = &f.name {
- (id.identifier.as_str(), f.doc_comment.as_deref())
+ (id.identifier.as_ref(), f.doc_comment.as_deref())
} else {
panic!("No constructors/destructors expected in this test.")
}
@@ -1138,7 +1138,7 @@
let method_mangled_names = ir
.functions()
.filter_map(|f| match &f.name {
- UnqualifiedIdentifier::Identifier(id) if id.identifier.as_str() == "MyMethod" => {
+ UnqualifiedIdentifier::Identifier(id) if id.identifier.as_ref() == "MyMethod" => {
Some(&f.mangled_name)
}
_ => None,
@@ -2382,7 +2382,7 @@
assert_eq!(1, ir.namespaces().count());
let ns = ir.namespaces().next().unwrap();
- assert_eq!("MyNamespace", ns.name.identifier);
+ assert_eq!("MyNamespace", ns.name.identifier.as_ref());
assert_eq!(1, ns.child_item_ids.len());
let ns_id = ns.id;
@@ -2494,7 +2494,8 @@
.unwrap();
let foo_func =
ir.functions().find(|f| f.name == UnqualifiedIdentifier::Identifier(ir_id("Foo"))).unwrap();
- let param_names: Vec<_> = foo_func.params.iter().map(|p| &p.identifier.identifier).collect();
+ let param_names: Vec<_> =
+ foo_func.params.iter().map(|p| p.identifier.identifier.as_ref()).collect();
assert_eq!(param_names, vec!["__this", "x", "y"]);
}
diff --git a/rs_bindings_from_cc/ir_testing.rs b/rs_bindings_from_cc/ir_testing.rs
index e9b3ef5..8be4bce 100644
--- a/rs_bindings_from_cc/ir_testing.rs
+++ b/rs_bindings_from_cc/ir_testing.rs
@@ -82,7 +82,7 @@
/// Creates an identifier
pub fn ir_id(name: &str) -> Identifier {
- Identifier { identifier: name.to_string() }
+ Identifier { identifier: name.into() }
}
/// Creates a simple `Item::Record` with a given name.
diff --git a/rs_bindings_from_cc/src_code_gen.rs b/rs_bindings_from_cc/src_code_gen.rs
index 11153c4..544dde2 100644
--- a/rs_bindings_from_cc/src_code_gen.rs
+++ b/rs_bindings_from_cc/src_code_gen.rs
@@ -366,7 +366,7 @@
let record: Option<&str> = ir.record_for_member_func(func)?.map(|r| &*r.cc_name);
let func_name = match &func.name {
- UnqualifiedIdentifier::Identifier(id) => id.identifier.clone(),
+ UnqualifiedIdentifier::Identifier(id) => id.identifier.to_string(),
UnqualifiedIdentifier::Operator(op) => op.cc_name(),
UnqualifiedIdentifier::Destructor => {
format!("~{}", record.expect("destructor must be associated with a record"))
@@ -1213,7 +1213,7 @@
*param = RsTypeKind::RvalueReference {
referent: Rc::new(value),
mutability: Mutability::Mut,
- lifetime: new_lifetime_param(func_param.identifier.identifier.clone()),
+ lifetime: new_lifetime_param(func_param.identifier.identifier.to_string()),
};
}
}
@@ -2857,7 +2857,7 @@
};
let idents = std::iter::once(crate_ident)
.chain(ir.crate_root_path().into_iter().map(ToOwned::to_owned))
- .chain(namespace_qualifiers.iter().cloned())
+ .chain(namespace_qualifiers.iter().map(|s| s.to_string()))
.collect();
CratePath { idents }
}
@@ -6296,7 +6296,7 @@
})
.find(|f| {
matches!(&f.name, UnqualifiedIdentifier::Constructor)
- && f.params.get(1).map(|p| p.identifier.identifier == "i").unwrap_or_default()
+ && f.params.get(1).map(|p| p.identifier.identifier.as_ref() == "i").unwrap_or_default()
})
.unwrap();
{
@@ -7164,7 +7164,7 @@
// const-ref + lifetimes in C++ ===> shared-ref in Rust
assert_eq!(foo_func.params.len(), 1);
let foo_param = &foo_func.params[0];
- assert_eq!(&foo_param.identifier.identifier, "foo_param");
+ assert_eq!(foo_param.identifier.identifier.as_ref(), "foo_param");
let foo_type = db.rs_type_kind(foo_param.type_.rs_type.clone())?;
assert!(foo_type.is_shared_ref_to(record));
assert!(matches!(foo_type, RsTypeKind::Reference { mutability: Mutability::Const, .. }));
@@ -7172,7 +7172,7 @@
// non-const-ref + lifetimes in C++ ===> mutable-ref in Rust
assert_eq!(bar_func.params.len(), 1);
let bar_param = &bar_func.params[0];
- assert_eq!(&bar_param.identifier.identifier, "bar_param");
+ assert_eq!(bar_param.identifier.identifier.as_ref(), "bar_param");
let bar_type = db.rs_type_kind(bar_param.type_.rs_type.clone())?;
assert!(!bar_type.is_shared_ref_to(record));
assert!(matches!(bar_type, RsTypeKind::Reference { mutability: Mutability::Mut, .. }));
@@ -7193,7 +7193,7 @@
// const-ref + *no* lifetimes in C++ ===> const-pointer in Rust
assert_eq!(foo_func.params.len(), 1);
let foo_param = &foo_func.params[0];
- assert_eq!(&foo_param.identifier.identifier, "foo_param");
+ assert_eq!(foo_param.identifier.identifier.as_ref(), "foo_param");
let foo_type = db.rs_type_kind(foo_param.type_.rs_type.clone())?;
assert!(!foo_type.is_shared_ref_to(record));
assert!(matches!(foo_type, RsTypeKind::Pointer { mutability: Mutability::Const, .. }));
