Skip to content

Commit

Permalink
Merge pull request #784 from google/never-use-rust-name-attribute
Browse files Browse the repository at this point in the history
Always RenameInOutputMod instead of #[rust_name]
  • Loading branch information
adetaylor authored Feb 11, 2022
2 parents c9dc473 + a4f9b51 commit 4966584
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 91 deletions.
5 changes: 5 additions & 0 deletions engine/src/conversion/analysis/fun/bridge_name_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ impl BridgeNameTracker {
found_name: &str,
ns: &Namespace,
) -> String {
let found_name = if found_name == "new" {
"new_autocxx"
} else {
found_name
};
let count = self
.next_cxx_bridge_name_for_prefix
.entry(found_name.to_string())
Expand Down
35 changes: 5 additions & 30 deletions engine/src/conversion/analysis/fun/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ pub(crate) mod function_wrapper;
mod implicit_constructor_rules;
mod implicit_constructors;
mod overload_tracker;
mod rust_name_tracker;
mod subclass;

use crate::{
Expand Down Expand Up @@ -64,7 +63,6 @@ use self::{
function_wrapper::RustConversionType,
implicit_constructors::find_missing_constructors,
overload_tracker::OverloadTracker,
rust_name_tracker::RustNameTracker,
subclass::{create_subclass_constructor, create_subclass_fn_wrapper, create_subclass_function},
};

Expand Down Expand Up @@ -137,8 +135,6 @@ pub(crate) enum FnKind {
pub(crate) enum RustRenameStrategy {
/// cxx::bridge name matches user expectations
None,
/// We can rename using the #[rust_name] attribute in the cxx::bridge
RenameUsingRustAttr,
/// Even the #[rust_name] attribute would cause conflicts, and we need
/// to use a 'use XYZ as ABC'
RenameInOutputMod(Ident),
Expand Down Expand Up @@ -213,7 +209,6 @@ impl AnalysisPhase for FnPhase {

pub(crate) struct FnAnalyzer<'a> {
unsafe_policy: UnsafePolicy,
rust_name_tracker: RustNameTracker,
extra_apis: Vec<UnanalyzedApi>,
type_converter: TypeConverter<'a>,
bridge_name_tracker: BridgeNameTracker,
Expand All @@ -233,7 +228,6 @@ impl<'a> FnAnalyzer<'a> {
) -> Vec<Api<FnPhase>> {
let mut me = Self {
unsafe_policy,
rust_name_tracker: RustNameTracker::new(),
extra_apis: Vec::new(),
type_converter: TypeConverter::new(config, &apis),
bridge_name_tracker: BridgeNameTracker::new(),
Expand Down Expand Up @@ -346,10 +340,6 @@ impl<'a> FnAnalyzer<'a> {
.get_unique_cxx_bridge_name(type_name, found_name, ns)
}

fn ok_to_use_rust_name(&mut self, rust_name: &str) -> bool {
self.rust_name_tracker.ok_to_use_rust_name(rust_name)
}

fn is_on_allowlist(&self, type_name: &QualifiedName) -> bool {
self.config.is_on_allowlist(&type_name.to_cpp_name())
}
Expand Down Expand Up @@ -1087,26 +1077,11 @@ impl<'a> FnAnalyzer<'a> {
validate_ident_ok_for_cxx(&cxxbridge_name.to_string()).unwrap_or_else(set_ignore_reason);
let rust_name_ident = make_ident(&rust_name);
let (id, rust_rename_strategy) = match kind {
FnKind::Method(..) | FnKind::TraitMethod { .. } => {
(rust_name_ident, RustRenameStrategy::None)
}
FnKind::Function => {
// Keep the original Rust name the same so callers don't
// need to know about all of these shenanigans.
// There is a global space of rust_names even if they're in
// different namespaces.
let rust_name_ok = self.ok_to_use_rust_name(&rust_name);
if cxxbridge_name == rust_name {
(rust_name_ident, RustRenameStrategy::None)
} else if rust_name_ok {
(rust_name_ident, RustRenameStrategy::RenameUsingRustAttr)
} else {
(
cxxbridge_name.clone(),
RustRenameStrategy::RenameInOutputMod(rust_name_ident),
)
}
}
FnKind::Function if cxxbridge_name != rust_name => (
cxxbridge_name.clone(),
RustRenameStrategy::RenameInOutputMod(rust_name_ident),
),
_ => (rust_name_ident, RustRenameStrategy::None),
};

let analysis = FnAnalysis {
Expand Down
50 changes: 0 additions & 50 deletions engine/src/conversion/analysis/fun/rust_name_tracker.rs

This file was deleted.

18 changes: 7 additions & 11 deletions engine/src/conversion/codegen_rs/fun_codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,6 @@ pub(super) fn gen_function(
let mut cpp_name_attr = Vec::new();
let mut impl_entry = None;
let mut trait_impl_entry = None;
let rust_name_attr: Vec<_> = match &analysis.rust_rename_strategy {
RustRenameStrategy::RenameUsingRustAttr => Attribute::parse_outer
.parse2(quote!(
#[rust_name = #rust_name]
))
.unwrap(),
_ => Vec::new(),
};
let wrapper_unsafety = analysis.requires_unsafe.wrapper_token();
let fn_generator = FnGenerator {
param_details: &param_details,
Expand All @@ -110,7 +102,11 @@ pub(super) fn gen_function(
.any(|pd| pd.conversion.rust_work_needed());
let rust_wrapper_needed = match kind {
FnKind::TraitMethod { .. } => true,
FnKind::Method(..) => any_param_needs_rust_conversion || cxxbridge_name != rust_name,
FnKind::Method(..) => {
any_param_needs_rust_conversion
|| cxxbridge_name != rust_name
|| wrapper_function_needed
}
_ => any_param_needs_rust_conversion,
};
if rust_wrapper_needed {
Expand Down Expand Up @@ -176,7 +172,6 @@ pub(super) fn gen_function(
let bridge_unsafety = analysis.requires_unsafe.bridge_token();
let extern_c_mod_item = ForeignItem::Fn(parse_quote!(
#(#namespace_attr)*
#(#rust_name_attr)*
#(#cpp_name_attr)*
#doc_attr
#vis #bridge_unsafety fn #cxxbridge_name #lifetime_tokens ( #params ) #ret_type;
Expand Down Expand Up @@ -335,12 +330,13 @@ impl<'a> FnGenerator<'a> {
fn generate_function_impl(&self, ret_type: &ReturnType) -> Box<Item> {
let (wrapper_params, arg_list) = self.generate_arg_lists(false);
let rust_name = make_ident(self.rust_name);
let cxxbridge_name = self.cxxbridge_name;
let doc_attr = self.doc_attr;
let unsafety = self.unsafety;
Box::new(Item::Fn(parse_quote! {
#doc_attr
pub #unsafety fn #rust_name ( #wrapper_params ) #ret_type {
cxxbridge::#rust_name ( #(#arg_list),* )
cxxbridge::#cxxbridge_name ( #(#arg_list),* )
}
}))
}
Expand Down

0 comments on commit 4966584

Please sign in to comment.