Use leading_colons_for_cpp_type feature flag and refactor NamespaceQualifier

> AI Disclaimer: the code and description are AI generated, but I have reviewed them so readers need not worry about AI slop.

This change uses the new Crubit feature flag, `leading_colons_for_cpp_type`. When enabled, it ensures that generated `cpp_type` annotations in Rust bindings have a leading `::` (e.g., `CRUBIT_ANNOTATE: cpp_type=::Foo`).

This change required some refactoring that spans both `rs_bindings_from_cc` and `cc_bindings_from_rs`. Here is a summary and justification for those changes:

1.  **Refactor of `NamespaceQualifier`**:
    *   **What**: Added a `use_leading_colons` field to `NamespaceQualifier` and updated `NamespaceQualifier::new` to accept this boolean explicitly. Centralized the logic for prepending `::` in `format_for_cc`.
    *   **Why**: Instead of manually handling string manipulation at every annotation generation site, centralizing it in `NamespaceQualifier` is less error-prone. Forcing the argument in `new` ensures that we explicitly consider the feature flag at every call site rather than defaulting to `false`.

2.  **Refactor of `crate_features` in `cc_bindings_from_rs`**:
    *   **What**: Moved the `crate_features` free function from `generate_bindings/lib.rs` to be a method on `BindingsGenerator` in `database/db.rs`.
    *   **Why**: To support the feature flag in `cc_bindings_from_rs` (specifically in `fully_qualified_name.rs` where `ExportedPath` is converted to `NamespaceQualifier`), we needed access to crate features. `fully_qualified_name.rs` belongs to the `database` crate and could not access the free function in `lib.rs`. Moving it to `BindingsGenerator` resolved this dependency issue and is architecturally cleaner, making feature lookup part of the database query.

3.  **Cleanup and Inlining**:
    *   **What**: Inlined the remaining free function `crate_features` in `lib.rs` and updated all call sites in `lib.rs` and `generate_struct_and_union.rs` to use `db.crate_features` directly.

4.  **Test consolidation**:
    *   **What**: Updated existing tests for `NamespaceQualifier` to use `true` for the feature flag instead of adding duplicate tests for both boolean states.

PiperOrigin-RevId: 900832916

Author: Googler

cc_bindings_from_rs/generate_bindings/database/db.rs

@@ -337,3 +337,18 @@ memoized::query_group! {
       ) -> Option<Result<RsStdEnumTemplateSpecialization<'tcx>>>;
   }
 }
+
+impl<'tcx> BindingsGenerator<'tcx> {
+    pub fn crate_features(
+        &self,
+        krate: CrateNum,
+    ) -> flagset::FlagSet<crubit_feature::CrubitFeature> {
+        let crate_features = self.crate_name_to_features();
+        let features = if krate == self.source_crate_num() {
+            crate_features.get("self")
+        } else {
+            crate_features.get(self.tcx().crate_name(krate).as_str())
+        };
+        features.copied().unwrap_or_else(|| self.default_features())
+    }
+}

cc_bindings_from_rs/generate_bindings/database/fully_qualified_name.rs

@@ -175,14 +175,18 @@ pub fn rename_clang_builtin_macros(ns: Rc<str>) -> Rc<str> {
     ns
 }
 
-impl From<&ExportedPath> for NamespaceQualifier {
-    fn from(this: &ExportedPath) -> Self {
+impl ExportedPath {
+    pub fn to_namespace_qualifier(&self, db: &BindingsGenerator) -> NamespaceQualifier {
+        let features = db.crate_features(self.krate);
+        let use_leading_colons =
+            features.contains(crubit_feature::CrubitFeature::LeadingColonsForCppType);
         NamespaceQualifier::new(
-            this.path
+            self.path
                 .iter()
                 .map(|s| Rc::<str>::from(s.as_str()))
                 .map(rename_clang_builtin_macros)
                 .collect::<Vec<_>>(),
+            use_leading_colons,
         )
     }
 }

cc_bindings_from_rs/generate_bindings/generate_bindings_test.rs

@@ -2204,9 +2204,9 @@ fn test_format_item_generate_bindings_for_top_level_type_alias() {
 fn test_format_namespace_bound_cc_tokens() {
     run_compiler_for_testing("", |tcx| {
         let db = bindings_db_for_tests(tcx);
-        let top_level = NamespaceQualifier::new::<&str>([]);
-        let m1 = NamespaceQualifier::new(["m1"]);
-        let m2 = NamespaceQualifier::new(["m2"]);
+        let top_level = NamespaceQualifier::new::<&str>([], true);
+        let m1 = NamespaceQualifier::new(["m1"], true);
+        let m2 = NamespaceQualifier::new(["m2"], true);
         let input = [
             (None, top_level.clone(), quote! { void f0a(); }),
             (None, m1.clone(), quote! { void f1a(); }),

cc_bindings_from_rs/generate_bindings/generate_struct_and_union.rs

@@ -14,10 +14,9 @@ use crate::generate_function::{
     format_variant_ctor_cc_name, generate_thunk_call, Param, ThunkSelfParameter,
 };
 use crate::{
-    crate_features, generate_const, generate_deprecated_tag, generate_must_use_tag,
-    generate_trait_thunks, generate_unsupported_def, get_layout, get_scalar_int_type,
-    get_tag_size_with_padding, is_bridged_type, is_copy, BridgedBuiltin, RsSnippet, SortedByDef,
-    TraitThunks,
+    generate_const, generate_deprecated_tag, generate_must_use_tag, generate_trait_thunks,
+    generate_unsupported_def, get_layout, get_scalar_int_type, get_tag_size_with_padding,
+    is_bridged_type, is_copy, BridgedBuiltin, RsSnippet, SortedByDef, TraitThunks,
 };
 use arc_anyhow::{Context, Result};
 use code_gen_utils::{expect_format_cc_type_name, make_rs_ident, CcInclude};
@@ -339,7 +338,7 @@ pub(crate) fn generate_associated_item<'tcx>(
         }
     };
     let result = result.and_then(|snippet| {
-        snippet.resolve_feature_requirements(crate_features(db, db.source_crate_num()))
+        snippet.resolve_feature_requirements(db.crate_features(db.source_crate_num()))
     });
     match result {
         Err(err) => {
@@ -1461,10 +1460,9 @@ pub(crate) fn generate_fields<'tcx>(
                                     size,
                                     cpp_type: db
                                         .format_ty_for_cc(ty, TypeLocation::Other)?
-                                        .resolve_feature_requirements(crate_features(
-                                            db,
-                                            db.source_crate_num(),
-                                        ))?,
+                                        .resolve_feature_requirements(
+                                            db.crate_features(db.source_crate_num()),
+                                        )?,
                                 })
                             });
                             let name = field_def.ident(tcx).to_string();

cc_bindings_from_rs/generate_bindings/lib.rs

@@ -244,7 +244,8 @@ pub fn generate_bindings(db: &BindingsGenerator) -> Result<BindingsTokens> {
         let source_crate_num = db.source_crate_num();
         let crate_name = tcx.crate_name(source_crate_num);
         let crubit_features = {
-            let mut crubit_features: Vec<&str> = crate_features(db, source_crate_num)
+            let mut crubit_features: Vec<&str> = db
+                .crate_features(source_crate_num)
                 .into_iter()
                 .map(|feature| feature.short_name())
                 .collect();
@@ -336,19 +337,6 @@ pub fn generate_bindings(db: &BindingsGenerator) -> Result<BindingsTokens> {
     Ok(BindingsTokens { cc_api, cc_api_impl })
 }
 
-pub(crate) fn crate_features(
-    db: &BindingsGenerator,
-    krate: CrateNum,
-) -> flagset::FlagSet<crubit_feature::CrubitFeature> {
-    let crate_features = db.crate_name_to_features();
-    let features = if krate == db.source_crate_num() {
-        crate_features.get("self")
-    } else {
-        crate_features.get(db.tcx().crate_name(krate).as_str())
-    };
-    features.copied().unwrap_or_else(|| db.default_features())
-}
-
 fn check_feature_enabled_on_self_and_all_deps(
     db: &BindingsGenerator,
     feature: FineGrainedFeature,
@@ -705,9 +693,15 @@ fn symbol_canonical_name(db: &BindingsGenerator<'_>, def_id: DefId) -> Option<Fu
         }
     }
 
-    let rs_mod_path = NamespaceQualifier::new(full_path_strs.clone());
-    let cpp_ns_path =
-        NamespaceQualifier::new(full_path_strs.into_iter().map(rename_clang_builtin_macros));
+    let features = db.crate_features(krate_num);
+    let use_leading_colons =
+        features.contains(crubit_feature::CrubitFeature::LeadingColonsForCppType);
+
+    let rs_mod_path = NamespaceQualifier::new(full_path_strs.clone(), use_leading_colons);
+    let cpp_ns_path = NamespaceQualifier::new(
+        full_path_strs.into_iter().map(rename_clang_builtin_macros),
+        use_leading_colons,
+    );
     let cpp_top_level_ns = format_top_level_ns_for_crate(db, krate_num);
     Some(FullyQualifiedName {
         krate,
@@ -1661,7 +1655,7 @@ fn generate_item_impl<'tcx>(
     };
 
     if let Ok(Some(item)) = item {
-        Ok(Some(item.resolve_feature_requirements(crate_features(db, db.source_crate_num()))?))
+        Ok(Some(item.resolve_feature_requirements(db.crate_features(db.source_crate_num()))?))
     } else {
         item
     }
@@ -1937,7 +1931,7 @@ fn generate_crate(db: &BindingsGenerator) -> Result<BindingsTokens> {
                 // `format_namespace_bound_cc_tokens`, so we want to carry it alongside our alias
                 // even though we technically no longer need it.
                 def_id,
-                NamespaceQualifier::from(alias),
+                alias.to_namespace_qualifier(db),
                 using_snippets.into_tokens(&mut cc_details_prereqs),
             ));
         }
@@ -2096,26 +2090,45 @@ fn generate_crate(db: &BindingsGenerator) -> Result<BindingsTokens> {
             .into_iter()
             .chain(ordered_main_apis)
             .map(|(node, tokens)| match node {
-                Node::Def(def_id) => (
-                    tcx.opt_parent(def_id),
-                    NamespaceQualifier::new(cpp_top_level_ns.iter().cloned().chain({
-                        db.symbol_canonical_name(def_id)
-                            .unwrap_or_else(|| {
-                                panic!("Exported item {def_id:?} should have a canonical name")
-                            })
-                            .cpp_ns_path
-                            .namespaces
-                    })),
-                    tokens,
-                ),
+                Node::Def(def_id) => {
+                    let features = db.crate_features(def_id.krate);
+                    let use_leading_colons =
+                        features.contains(crubit_feature::CrubitFeature::LeadingColonsForCppType);
+                    (
+                        tcx.opt_parent(def_id),
+                        NamespaceQualifier::new(
+                            cpp_top_level_ns.iter().cloned().chain({
+                                db.symbol_canonical_name(def_id)
+                                    .unwrap_or_else(|| {
+                                        panic!(
+                                            "Exported item {def_id:?} should have a canonical name"
+                                        )
+                                    })
+                                    .cpp_ns_path
+                                    .namespaces
+                            }),
+                            use_leading_colons,
+                        ),
+                        tokens,
+                    )
+                }
                 // Specializations always live in the top-level namespace.
-                Node::Specialization(_) => (None, NamespaceQualifier::new::<Rc<str>>([]), tokens),
+                Node::Specialization(_) => {
+                    let use_leading_colons = db
+                        .crate_features(db.source_crate_num())
+                        .contains(crubit_feature::CrubitFeature::LeadingColonsForCppType);
+                    (None, NamespaceQualifier::new::<Rc<str>>([], use_leading_colons), tokens)
+                }
             })
             .chain(cc_details.into_iter().map(|details| {
+                let use_leading_colons = db
+                    .crate_features(details.def_id.krate)
+                    .contains(crubit_feature::CrubitFeature::LeadingColonsForCppType);
                 (
                     tcx.opt_parent(details.def_id),
                     NamespaceQualifier::new(
                         cpp_top_level_ns.iter().cloned().chain(details.namespace.namespaces),
+                        use_leading_colons,
                     ),
                     details.tokens,
                 )

common/code_gen_utils.rs

@@ -348,17 +348,27 @@ pub struct NamespaceQualifier {
     pub namespaces: Vec<Rc<str>>,
     // Outer to innermost. Paired as (rs_name, cc_name)
     pub nested_records: Vec<(Rc<str>, Rc<str>)>,
+    /// Whether to prepend `::` when formatting for C++.
+    // TODO(b/502939407): Remove this field (it will always be true).
+    pub use_leading_colons: bool,
 }
 
 impl NamespaceQualifier {
     /// Constructs a new `NamespaceQualifier` from a sequence of names.
-    pub fn new<T: Into<Rc<str>>>(iter: impl IntoIterator<Item = T>) -> Self {
+    pub fn new<T: Into<Rc<str>>>(
+        iter: impl IntoIterator<Item = T>,
+        use_leading_colons: bool,
+    ) -> Self {
         // TODO(b/258265044): Catch most (all if possible) error conditions early.  For
         // example:
         // - Panic early if any strings are empty, or are not Rust identifiers
         // - Report an error early if any strings are C++ reserved keywords
         // This may make `format_for_cc` and `format_namespace_bound_cc_tokens` infallible.
-        Self { namespaces: iter.into_iter().map(Into::into).collect(), nested_records: vec![] }
+        Self {
+            namespaces: iter.into_iter().map(Into::into).collect(),
+            nested_records: vec![],
+            use_leading_colons,
+        }
     }
 
     pub fn is_empty(&self) -> bool {
@@ -387,7 +397,11 @@ impl NamespaceQualifier {
 
     /// Returns `foo::bar::baz::` (reporting errors for C++ keywords).
     pub fn format_for_cc(&self) -> Result<TokenStream> {
-        let mut path = quote! {};
+        let mut path = if self.use_leading_colons {
+            quote! { :: }
+        } else {
+            quote! {}
+        };
         for namespace in &self.namespaces {
             let namespace = format_cc_ident(namespace)?;
             path.extend(quote! { #namespace :: });
@@ -745,34 +759,34 @@ pub mod tests {
 
     #[gtest]
     fn test_namespace_qualifier_empty() {
-        let ns = NamespaceQualifier::new::<&str>([]);
+        let ns = NamespaceQualifier::new::<&str>([], true);
         let actual_rs = ns.format_for_rs();
         assert!(actual_rs.is_empty());
         let actual_cc = ns.format_for_cc().unwrap();
-        assert!(actual_cc.is_empty());
+        assert_cc_matches!(actual_cc, quote! { :: });
     }
 
     #[gtest]
     fn test_namespace_qualifier_basic() {
-        let ns = NamespaceQualifier::new(["foo", "bar"]);
+        let ns = NamespaceQualifier::new(["foo", "bar"], true);
         let actual_rs = ns.format_for_rs();
         assert_rs_matches!(actual_rs, quote! { foo::bar:: });
         let actual_cc = ns.format_for_cc().unwrap();
-        assert_cc_matches!(actual_cc, quote! { foo::bar:: });
+        assert_cc_matches!(actual_cc, quote! { :: foo::bar:: });
     }
 
     #[gtest]
     fn test_namespace_qualifier_reserved_cc_keyword() {
-        let ns = NamespaceQualifier::new(["foo", "impl", "bar"]);
+        let ns = NamespaceQualifier::new(["foo", "impl", "bar"], true);
         let actual_rs = ns.format_for_rs();
         assert_rs_matches!(actual_rs, quote! { foo :: r#impl :: bar :: });
         let actual_cc = ns.format_for_cc().unwrap();
-        assert_cc_matches!(actual_cc, quote! { foo::impl::bar:: });
+        assert_cc_matches!(actual_cc, quote! { :: foo::impl::bar:: });
     }
 
     #[gtest]
     fn test_namespace_qualifier_reserved_rust_keyword() {
-        let ns = NamespaceQualifier::new(["foo", "reinterpret_cast", "bar"]);
+        let ns = NamespaceQualifier::new(["foo", "reinterpret_cast", "bar"], true);
         let actual_rs = ns.format_for_rs();
         assert_rs_matches!(actual_rs, quote! { foo :: reinterpret_cast :: bar :: });
         let cc_error = ns.format_for_cc().unwrap_err();

rs_bindings_from_cc/generate_bindings/database/db.rs

@@ -480,7 +480,17 @@ impl<'db> BindingsGenerator<'db> {
         }
         namespaces.reverse();
         nested_records.reverse();
-        code_gen_utils::NamespaceQualifier { namespaces, nested_records }
+        let use_leading_colons =
+            if let Some(target) = self.find_untyped_decl(item_id).owning_target() {
+                self.ir()
+                    .target_crubit_features(&target)
+                    .contains(crubit_feature::CrubitFeature::LeadingColonsForCppType)
+            } else {
+                // We default to true here because the final change will always be to add `::` to
+                // the beginning of the type.
+                true
+            };
+        code_gen_utils::NamespaceQualifier { namespaces, nested_records, use_leading_colons }
     }
 
     /// Returns the name of the snake-cased module that exposes the given record's nested items.

rs_bindings_from_cc/generate_bindings/database/rs_snippet.rs

@@ -169,7 +169,10 @@ impl CratePath {
         namespace_qualifier: NamespaceQualifier,
         crate_ident: Option<Ident>,
     ) -> CratePath {
-        let crate_root_path = NamespaceQualifier::new(ir.crate_root_path());
+        let use_leading_colons = ir
+            .target_crubit_features(ir.current_target())
+            .contains(CrubitFeature::LeadingColonsForCppType);
+        let crate_root_path = NamespaceQualifier::new(ir.crate_root_path(), use_leading_colons);
         CratePath { crate_ident, crate_root_path, namespace_qualifier }
     }
 }
@@ -2173,7 +2176,11 @@ mod tests {
     }
 
     fn make_crate_path() -> Rc<CratePath> {
-        let ns = NamespaceQualifier { namespaces: vec![], nested_records: vec![] };
+        let ns = NamespaceQualifier {
+            namespaces: vec![],
+            nested_records: vec![],
+            use_leading_colons: true,
+        };
         Rc::new(CratePath {
             crate_ident: None,
             crate_root_path: ns.clone(),

rs_bindings_from_cc/generate_bindings/generate_bindings_test.rs

@@ -724,6 +724,24 @@ fn test_rust_keywords_are_escaped_in_rs_api_file() -> Result<()> {
     Ok(())
 }
 
+#[gtest]
+fn test_leading_colons_for_cpp_type() -> Result<()> {
+    let mut ir = ir_from_cc("struct S {};")?;
+    let target = ir.current_target().clone();
+    let features = ir.target_crubit_features(&target);
+    *ir.target_crubit_features_mut(&target) =
+        features | crubit_feature::CrubitFeature::LeadingColonsForCppType;
+    let rs_api = generate_bindings_tokens_for_test(ir)?.rs_api;
+    assert_rs_matches!(
+        rs_api,
+        quote! {
+            #[doc = "CRUBIT_ANNOTATE: cpp_type=:: S"]
+            pub struct S
+        }
+    );
+    Ok(())
+}
+
 #[gtest]
 fn test_rust_keywords_are_not_escaped_in_rs_api_impl_file() -> Result<()> {
     let ir = ir_from_cc("struct type { int dyn; };")?;

rs_bindings_from_cc/generate_bindings/generate_function_thunk.rs

@@ -270,8 +270,11 @@ fn generate_function_assertion_for_identifier(
     id: &Identifier,
 ) -> Result<ThunkImpl> {
     let fn_ident = format_cc_ident(&id.identifier)?;
-    let path_to_func = db.namespace_qualifier(func).format_for_cc()?;
-    let implementation_function = quote! { :: #path_to_func #fn_ident };
+    let mut namespace_qualifier = db.namespace_qualifier(func);
+    // Keep goldens the same.
+    namespace_qualifier.use_leading_colons = true;
+    let path_to_func = namespace_qualifier.format_for_cc()?;
+    let implementation_function = quote! { #path_to_func #fn_ident };
     let method_qualification;
     let member_function_prefix;
     let func_params;