diff --git a/cc_bindings_from_rs/generate_bindings/database/code_snippet.rs b/cc_bindings_from_rs/generate_bindings/database/code_snippet.rs index 66cf15f13..11dcc49d3 100644 --- a/cc_bindings_from_rs/generate_bindings/database/code_snippet.rs +++ b/cc_bindings_from_rs/generate_bindings/database/code_snippet.rs @@ -341,9 +341,7 @@ impl<'tcx> CcPrerequisites<'tcx> { /// or this will fail. pub fn depend_on_def(&mut self, db: &BindingsGenerator<'tcx>, def_id: DefId) -> Result<()> { let tcx = db.tcx(); - let canonical_name = db.symbol_canonical_name(def_id).ok_or_else(|| { - anyhow!("Failed to generate canonical name for `{}`", tcx.def_path_str(def_id)) - })?; + let canonical_name = db.symbol_canonical_name(def_id)?; // Definition with a local canonical name can be immediately added to the `defs` set. if canonical_name.krate_num == db.source_crate_num() { self.defs.insert(def_id); diff --git a/cc_bindings_from_rs/generate_bindings/database/db.rs b/cc_bindings_from_rs/generate_bindings/database/db.rs index 1d220ee16..835bec793 100644 --- a/cc_bindings_from_rs/generate_bindings/database/db.rs +++ b/cc_bindings_from_rs/generate_bindings/database/db.rs @@ -177,11 +177,11 @@ memoized::query_group! { /// at either `Bar` or `foo::Bar` due to our `use` statements. This method would give `Bar` /// the canonical name `foo::Bar`, preferring the more specific of our two available paths. /// - /// If no canonical name can be determined, `None` is returned. This will occur when our + /// If no canonical name can be determined, an error is returned. This will occur when our /// `def_id` has no publicly visible paths, for example. /// /// Implementation: cc_bindings_from_rs/generate_bindings/lib.rs?q=function:symbol_canonical_name - fn symbol_canonical_name(&self, def_id: DefId) -> Option; + fn symbol_canonical_name(&self, def_id: DefId) -> Result; /// Computes a mapping from a `DefId` to a list of public paths that reference it in a given /// crate. This accounts for `use` statements that reexport, and optionally alias, the same diff --git a/cc_bindings_from_rs/generate_bindings/format_type.rs b/cc_bindings_from_rs/generate_bindings/format_type.rs index f4eb18ae6..508b5b8ca 100644 --- a/cc_bindings_from_rs/generate_bindings/format_type.rs +++ b/cc_bindings_from_rs/generate_bindings/format_type.rs @@ -942,10 +942,7 @@ pub fn format_ty_for_cc<'tcx>( "Generic types are not supported yet (b/259749095)" ); crate::should_receive_bindings(db, adt.did())?; - ensure!( - db.symbol_canonical_name(adt.did()).is_some(), - "Not a public or a supported reexported type (b/262052635)." - ); + db.symbol_canonical_name(adt.did())?; prereqs.depend_on_def(db, def_id)?; @@ -955,9 +952,7 @@ pub fn format_ty_for_cc<'tcx>( })?; } - let canonical_name = db - .symbol_canonical_name(def_id) - .ok_or_else(|| anyhow!("Failed to generate canonical name for `{ty}`"))?; + let canonical_name = db.symbol_canonical_name(def_id)?; let mut tokens = canonical_name.format_for_cc(db)?; // Add generic arguments for a generic ADT. @@ -1597,9 +1592,7 @@ pub fn format_ty_for_rs<'tcx>(db: &BindingsGenerator<'tcx>, ty: Ty<'tcx>) -> Res has_cpp_type || is_supported_generic_type || has_composable_bridging, "Generic types without composable bridging are not supported yet (b/259749095)" ); - let canonical_name = db - .symbol_canonical_name(adt.did()) - .ok_or_else(|| anyhow!("Failed to get canonical name for {:?}", adt.did()))?; + let canonical_name = db.symbol_canonical_name(adt.did())?; let type_name = canonical_name.format_for_rs(); let generic_params = if substs.is_empty() { quote! {} @@ -1824,10 +1817,7 @@ pub fn crubit_abi_type_from_ty<'tcx>( include_paths, cpp_type, } => { - let fully_qualified_name = - db.symbol_canonical_name(adt.did()).ok_or_else(|| { - anyhow!("Failed to get canonical name for {:?}", adt.did()) - })?; + let fully_qualified_name = db.symbol_canonical_name(adt.did())?; let mut prereqs = CcPrerequisites::default(); for path in &include_paths { prereqs.includes.insert(CcInclude::from_path(path.as_str())); @@ -1875,9 +1865,7 @@ pub fn crubit_abi_type_from_ty<'tcx>( return Ok(CrubitAbiTypeWithCcPrereqs { crubit_abi_type, prereqs }); } - let fully_qualified_name = db - .symbol_canonical_name(adt.did()) - .ok_or_else(|| anyhow!("Failed to get canonical name for {:?}", adt.did()))?; + let fully_qualified_name = db.symbol_canonical_name(adt.did())?; // It's just a regular old type. // Question: do we need to check that it doesn't have any generics? diff --git a/cc_bindings_from_rs/generate_bindings/generate_bindings_test.rs b/cc_bindings_from_rs/generate_bindings/generate_bindings_test.rs index 516ee21de..c9c09a528 100644 --- a/cc_bindings_from_rs/generate_bindings/generate_bindings_test.rs +++ b/cc_bindings_from_rs/generate_bindings/generate_bindings_test.rs @@ -2509,7 +2509,7 @@ fn test_trait_operator_without_core_crate_header_returns_error() { let bindings = generate_bindings::generate_bindings(&db).unwrap(); let cc_api = cc_tokens_to_formatted_string_for_tests(bindings.cc_api).unwrap(); assert!( - cc_api.contains("trait does not have a canonical"), + cc_api.contains("public or a supported reexported type"), "Expected unsupported error message in cc_api, got:\n{cc_api}" ); }); diff --git a/cc_bindings_from_rs/generate_bindings/generate_function.rs b/cc_bindings_from_rs/generate_bindings/generate_function.rs index 269ee198b..303ac5103 100644 --- a/cc_bindings_from_rs/generate_bindings/generate_function.rs +++ b/cc_bindings_from_rs/generate_bindings/generate_function.rs @@ -931,7 +931,7 @@ fn format_trait_ref_for_cc<'tcx>( ) -> Result> { let trait_name = db .symbol_canonical_name(trait_ref.def_id) - .and_then(|fully_qualified_name| fully_qualified_name.format_for_cc(db).ok()) + .and_then(|fully_qualified_name| fully_qualified_name.format_for_cc(db)) .expect("Generated trait method for a trait with an invalid cc name"); let mut trait_args = trait_ref.args[1..].iter().filter_map(|arg| arg.as_type()).peekable(); let mut prereqs = CcPrerequisites::default(); @@ -954,13 +954,7 @@ fn format_trait_ref_for_rs<'tcx>( ) -> Result { let trait_name = db .symbol_canonical_name(trait_ref.def_id) - .map(|fully_qualified_name| fully_qualified_name.format_for_rs()) - .ok_or_else(|| { - anyhow!( - "Failed to format trait name `{}`: trait does not have a canonical name", - db.tcx().def_path_str(trait_ref.def_id) - ) - })?; + .map(|fully_qualified_name| fully_qualified_name.format_for_rs())?; let mut trait_args = trait_ref.args[1..].iter().filter_map(|arg| arg.as_type()).peekable(); if trait_args.peek().is_none() { Ok(quote! { #trait_name }) @@ -1165,7 +1159,7 @@ pub fn generate_function<'tcx>( Some(ty) => match ty.kind() { ty::TyKind::Adt(adt, substs) => { assert!(!has_non_lifetime_substs(substs), "Callers should filter out generics"); - db.symbol_canonical_name(adt.did()) + db.symbol_canonical_name(adt.did()).ok() } _ => panic!("Non-ADT `impl`s should be filtered by caller"), }, @@ -1344,7 +1338,7 @@ pub fn generate_function<'tcx>( let fn_name = make_rs_ident(unqualified_rust_fn_name.as_str()); let struct_name = struct_name.format_for_rs(); quote! { #struct_name :: #fn_name } - } else if let Some(canonical) = db.symbol_canonical_name(def_id) { + } else if let Ok(canonical) = db.symbol_canonical_name(def_id) { canonical.format_for_rs() } else { panic!( diff --git a/cc_bindings_from_rs/generate_bindings/generate_function_thunk.rs b/cc_bindings_from_rs/generate_bindings/generate_function_thunk.rs index c4782212c..267b6f972 100644 --- a/cc_bindings_from_rs/generate_bindings/generate_function_thunk.rs +++ b/cc_bindings_from_rs/generate_bindings/generate_function_thunk.rs @@ -395,9 +395,7 @@ fn format_ty_for_closure_param_rs<'tcx>( _ => {} } } - let canonical_name = db - .symbol_canonical_name(adt.did()) - .ok_or_else(|| anyhow!("Failed to get canonical name for {:?}", adt.did()))?; + let canonical_name = db.symbol_canonical_name(adt.did())?; let type_name = canonical_name.format_for_rs(); let generic_params = if substs.is_empty() { quote! {} @@ -980,7 +978,7 @@ pub fn generate_trait_thunks<'tcx>( type_args.iter().copied().map(ty::GenericArg::from), ) { let display_name = def_id - .and_then(|id| db.symbol_canonical_name(id)) + .and_then(|id| db.symbol_canonical_name(id).ok()) .map(|canon| { let parts = canon.rs_name_parts().map(|s| format!("{}", s)).collect::>(); parts.join("::") @@ -1073,12 +1071,7 @@ pub fn generate_trait_thunks<'tcx>( } }) } else { - let fully_qualified_trait_name = db - .symbol_canonical_name(trait_id) - .ok_or_else(|| { - anyhow!("Failed to get canonical name for {}", tcx.def_path_str(trait_id)) - })? - .format_for_rs(); + let fully_qualified_trait_name = db.symbol_canonical_name(trait_id)?.format_for_rs(); let method_name = make_rs_ident(method.name().as_str()); let args = type_args .iter() diff --git a/cc_bindings_from_rs/generate_bindings/generate_struct_and_union.rs b/cc_bindings_from_rs/generate_bindings/generate_struct_and_union.rs index 62c74f8fc..c32a5cb33 100644 --- a/cc_bindings_from_rs/generate_bindings/generate_struct_and_union.rs +++ b/cc_bindings_from_rs/generate_bindings/generate_struct_and_union.rs @@ -892,7 +892,7 @@ fn generate_constructor_impls<'tcx>( let is_src_local_adt = match src_ty.kind() { ty::TyKind::Adt(adt_def, _) => db .symbol_canonical_name(adt_def.did()) - .is_none_or(|name| name.krate_num == db.source_crate_num()), + .map_or(true, |name| name.krate_num == db.source_crate_num()), _ => false, }; if is_src_local_adt { @@ -2144,9 +2144,7 @@ pub fn adt_needs_bindings<'tcx>( let tcx = db.tcx(); let attributes = crubit_attr::get_attrs(tcx, def_id).unwrap(); - let Some(fully_qualified_name) = db.symbol_canonical_name(def_id) else { - bail!("No public path could be found for type {}", tcx.def_path_str(def_id)); - }; + let fully_qualified_name = db.symbol_canonical_name(def_id)?; if let Some(cpp_type) = fully_qualified_name.unqualified.cpp_type { let item_name = tcx.def_path_str(def_id); bail!( @@ -2178,9 +2176,7 @@ pub fn generate_generic_adt_declaration<'tcx>( def_id: DefId, ) -> Result> { let tcx = db.tcx(); - let Some(fully_qualified_name) = db.symbol_canonical_name(def_id) else { - bail!("No public path could be found for type {}", tcx.def_path_str(def_id)); - }; + let fully_qualified_name = db.symbol_canonical_name(def_id)?; let attributes = crubit_attr::get_attrs(tcx, def_id).unwrap_or_default(); if let Some(cpp_type) = fully_qualified_name.unqualified.cpp_type { @@ -2269,11 +2265,7 @@ pub fn generate_adt_core<'tcx>( crate::normalize_ty(tcx, tcx.param_env(def_id), tcx.type_of(def_id).instantiate_identity()), ); assert!(self_ty.is_adt()); - assert!(db.symbol_canonical_name(def_id).is_some(), "Caller should verify"); - - let Some(fully_qualified_name) = db.symbol_canonical_name(def_id) else { - bail!("`generate_adt_core` called on non-reachable type {}", tcx.def_path_str(def_id)); - }; + let fully_qualified_name = db.symbol_canonical_name(def_id)?; let rs_fully_qualified_name = fully_qualified_name.format_for_rs(); let cpp_name = format_cc_ident(db, fully_qualified_name.unqualified.cpp_name.as_str()) .context("Error formatting item name")?; diff --git a/cc_bindings_from_rs/generate_bindings/generate_template_specialization.rs b/cc_bindings_from_rs/generate_bindings/generate_template_specialization.rs index 622936cb0..97122dec7 100644 --- a/cc_bindings_from_rs/generate_bindings/generate_template_specialization.rs +++ b/cc_bindings_from_rs/generate_bindings/generate_template_specialization.rs @@ -1674,7 +1674,7 @@ fn append_explicit_trait_impls<'tcx>( continue; }; // Only bind implementations for supported ADTs. - let Some(canonical_name) = db.symbol_canonical_name(*did) else { + let Ok(canonical_name) = db.symbol_canonical_name(*did) else { continue; }; // We explicitly want to allow ADTs that specify cpp_type. @@ -1719,7 +1719,7 @@ fn append_negative_auto_trait_impls<'tcx>( }) { continue; } - let Some(canonical_name) = db.symbol_canonical_name(self_def_id) else { + let Ok(canonical_name) = db.symbol_canonical_name(self_def_id) else { continue; }; if canonical_name.krate_num != db.source_crate_num() { diff --git a/cc_bindings_from_rs/generate_bindings/lib.rs b/cc_bindings_from_rs/generate_bindings/lib.rs index 3ab9ae9eb..1e387c101 100644 --- a/cc_bindings_from_rs/generate_bindings/lib.rs +++ b/cc_bindings_from_rs/generate_bindings/lib.rs @@ -897,7 +897,7 @@ fn renamed_crate_original_name(db: &BindingsGenerator<'_>, krate_id: CrateNum) - } /// Implementation of `BindingsGenerator::symbol_canonical_name`. -fn symbol_canonical_name(db: &BindingsGenerator<'_>, def_id: DefId) -> Option { +fn symbol_canonical_name(db: &BindingsGenerator<'_>, def_id: DefId) -> Result { let tcx = db.tcx(); // TODO: b/433286909 - We shouldn't pass DefKind::Use to this method and instead should keep what our use @@ -908,12 +908,13 @@ fn symbol_canonical_name(db: &BindingsGenerator<'_>, def_id: DefId) -> Option, def_id: DefId) -> Option, def_id: DefId) -> Option = full_path_strs.iter().map(|x| &**x).collect(); if matches!(&*path_strs, ["dsl"]) { - return None; + bail!("Unsupported ambiguous re-export in polars_plan::dsl"); } } @@ -976,7 +980,7 @@ fn symbol_canonical_name(db: &BindingsGenerator<'_>, def_id: DefId) -> Option( bail!("Unable to `use` function whose bindings failed: {err:?}"); } }; - let fully_qualified_fn_name = db - .symbol_canonical_name(def_id) - .unwrap_or_else(|| panic!("Failed to get canonical name for {:?}", def_id)); + let fully_qualified_fn_name = db.symbol_canonical_name(def_id).unwrap_or_else(|err| { + panic!("Failed to get canonical name for {:?}: {err}", def_id) + }); let formatted_fully_qualified_fn_name = fully_qualified_fn_name.format_for_cc(db)?; let main_api_fn_name = format_cc_ident(db, fully_qualified_fn_name.unqualified.cpp_name.as_str()) @@ -1378,7 +1382,7 @@ fn supported_traits(db: &BindingsGenerator<'_>) -> Rc<[DefId]> { .visible_traits() .filter(|trait_id| { // Does the trait get bindings? - db.symbol_canonical_name(*trait_id).is_some() + db.symbol_canonical_name(*trait_id).is_ok() }) .filter(|trait_id| { // Traits do not support const generics. @@ -1536,9 +1540,7 @@ fn create_type_alias<'tcx>( alias_name: &str, alias_type: Ty<'tcx>, ) -> Result> { - let fully_qualified_name = db - .symbol_canonical_name(def_id) - .ok_or_else(|| anyhow!("Failed to get canonical name for {:?}", def_id))?; + let fully_qualified_name = db.symbol_canonical_name(def_id)?; let rs_type = format!("{}", fully_qualified_name.format_for_rs()); create_type_alias_with_rs_type(db, def_id, &rs_type, alias_name, alias_type) } @@ -1783,7 +1785,7 @@ fn copy_codegen_style_to_snippets<'tcx>( None => { let display_name = core .def_id - .and_then(|id| db.symbol_canonical_name(id)) + .and_then(|id| db.symbol_canonical_name(id).ok()) .map(|canon| { let parts = canon.rs_name_parts().map(|s| format!("{}", s)).collect::>(); @@ -2200,7 +2202,7 @@ fn generate_item_impl<'tcx>( def_id: DefId, ) -> Result>> { let tcx = db.tcx(); - if db.symbol_canonical_name(def_id).is_none() { + if db.symbol_canonical_name(def_id).is_err() { return Ok(None); }; let item = match tcx.def_kind(def_id) { @@ -2456,7 +2458,7 @@ fn formatted_items_in_crate<'tcx>( .into_iter() .filter_map(|(def_id, paths)| { let mut snippets = None; - let canonical_name = db.symbol_canonical_name(def_id)?; + let canonical_name = db.symbol_canonical_name(def_id).ok()?; let aliases = if canonical_name.krate_num == db.source_crate_num() { // We only want to call `generate_item` on DefIds from our source crate. External // crate DefIds might appear in this map if our crate re-exports them, but we don't @@ -2613,9 +2615,9 @@ fn generate_crate(db: &BindingsGenerator) -> Result { cc_details.push(CcDetails::new( def_id, db.symbol_canonical_name(def_id) - .unwrap_or_else(|| { + .unwrap_or_else(|err| { panic!( - "Exported item {} should have a canonical name", + "Exported item {} should have a canonical name: {err}", db.tcx().def_path_str(def_id) ) }) @@ -2784,9 +2786,9 @@ fn generate_crate(db: &BindingsGenerator) -> Result { NamespaceQualifier::new( cpp_top_level_ns.iter().cloned().chain({ db.symbol_canonical_name(def_id) - .unwrap_or_else(|| { + .unwrap_or_else(|err| { panic!( - "Exported item {} should have a canonical name", + "Exported item {} should have a canonical name: {err}", tcx.def_path_str(def_id), ) })