From 921d31b580b4a702cc7c612ba66864d569a6cb34 Mon Sep 17 00:00:00 2001 From: Luke Zarko Date: Fri, 27 Mar 2026 20:41:56 -0700 Subject: [PATCH] Add a bit to pointer types to decide if they should be CRef/CMut. This is not a new PointerTypeKind because it's a Rust-side representation decision. Right now we're aiming to lower LValueRef in output positions to CRef/CMut, but in the future we may also want to lower NonNull. (These require us to generate slightly different code on the C++ side.) Making this a property of the pointer type reduces the state we need to carry around later on, as the decision to use CRef vs const& again depends on the semantic location of the reference. (It's kind of clear when you think about returns vs parameters, but this becomes more complicated when we consider function types.) PiperOrigin-RevId: 890766422 --- .../generate_bindings/generate_function.rs | 2 +- rs_bindings_from_cc/ir.cc | 47 ++++++++++--------- rs_bindings_from_cc/ir.h | 2 + rs_bindings_from_cc/ir.rs | 5 ++ rs_bindings_from_cc/ir_from_cc_test.rs | 7 +++ 5 files changed, 40 insertions(+), 23 deletions(-) diff --git a/rs_bindings_from_cc/generate_bindings/generate_function.rs b/rs_bindings_from_cc/generate_bindings/generate_function.rs index 73d2c3291..5dbbf45e1 100644 --- a/rs_bindings_from_cc/generate_bindings/generate_function.rs +++ b/rs_bindings_from_cc/generate_bindings/generate_function.rs @@ -1307,7 +1307,7 @@ fn rs_type_kinds_for_func( if i == 0 && func.is_instance_method() { // `param_type` is a `this` pointer, but its semantics are really that of // references. That is, `this` in these operators is non-null. - let CcTypeVariant::Pointer(PointerType { kind, lifetime, pointee_type: _ }) = + let CcTypeVariant::Pointer(PointerType { kind, lifetime, pointee_type: _, is_cref: _}) = &mut param_type.variant else { panic!( diff --git a/rs_bindings_from_cc/ir.cc b/rs_bindings_from_cc/ir.cc index 8c19f3934..ea9a99dc7 100644 --- a/rs_bindings_from_cc/ir.cc +++ b/rs_bindings_from_cc/ir.cc @@ -75,29 +75,32 @@ llvm::json::Value CcType::ToJson() const { return llvm::json::Object{{"Primitive", primitive.spelling}}; }, [&](CcType::PointerType pointer) { + auto pointer_json = llvm::json::Object{ + { + "kind", + [&]() -> llvm::json::Value { + switch (pointer.kind) { + case PointerTypeKind::kLValueRef: + return "LValueRef"; + case PointerTypeKind::kRValueRef: + return "RValueRef"; + case PointerTypeKind::kNullable: + return "Nullable"; + case PointerTypeKind::kNonNull: + return "NonNull"; + case PointerTypeKind::kOwned: + return "Owned"; + } + }(), + }, + {"lifetime", pointer.lifetime}, + {"pointee_type", *pointer.pointee_type}, + }; + if (pointer.is_cref) { + pointer_json.insert({"is_cref", pointer.is_cref}); + } return llvm::json::Object{ - {"Pointer", - llvm::json::Object{ - { - "kind", - [&]() -> llvm::json::Value { - switch (pointer.kind) { - case PointerTypeKind::kLValueRef: - return "LValueRef"; - case PointerTypeKind::kRValueRef: - return "RValueRef"; - case PointerTypeKind::kNullable: - return "Nullable"; - case PointerTypeKind::kNonNull: - return "NonNull"; - case PointerTypeKind::kOwned: - return "Owned"; - } - }(), - }, - {"lifetime", pointer.lifetime}, - {"pointee_type", *pointer.pointee_type}, - }}, + {"Pointer", std::move(pointer_json)}, }; }, [&](const CcType::FuncPointer& func_value) { diff --git a/rs_bindings_from_cc/ir.h b/rs_bindings_from_cc/ir.h index 2bddc13bb..63b667ebf 100644 --- a/rs_bindings_from_cc/ir.h +++ b/rs_bindings_from_cc/ir.h @@ -223,6 +223,8 @@ struct CcType { std::optional lifetime; std::shared_ptr pointee_type; + + bool is_cref = false; }; struct Primitive { diff --git a/rs_bindings_from_cc/ir.rs b/rs_bindings_from_cc/ir.rs index d23bf13fc..d29f88be5 100644 --- a/rs_bindings_from_cc/ir.rs +++ b/rs_bindings_from_cc/ir.rs @@ -272,6 +272,11 @@ pub struct PointerType { pub kind: PointerTypeKind, pub lifetime: Option, pub pointee_type: Rc, + /// Lower as CRef or CMut depending on constness. Depending on the strategy we use, various + /// PointerTypeKinds may be `is_cref`. Note that we don't expect this to be produced from + /// the Clang side. + #[serde(default)] + pub is_cref: bool, } /// Generates an enum type that implements `Deserialize`, which parses the stringified contents of diff --git a/rs_bindings_from_cc/ir_from_cc_test.rs b/rs_bindings_from_cc/ir_from_cc_test.rs index 6e82b8492..9c4ac5735 100644 --- a/rs_bindings_from_cc/ir_from_cc_test.rs +++ b/rs_bindings_from_cc/ir_from_cc_test.rs @@ -4317,6 +4317,7 @@ fn test_assumed_lifetimes_function() { unknown_attr: "", explicit_lifetimes: [], }, + is_cref: false, }), is_const: false, unknown_attr: "", @@ -4357,6 +4358,7 @@ fn test_assumed_lifetimes_function_with_explicit_binding() { unknown_attr: "", explicit_lifetimes: [], }, + is_cref: false, }), is_const: false, unknown_attr: "", @@ -4402,6 +4404,7 @@ fn test_assumed_lifetimes_function_with_explicit_bindings() { unknown_attr: "", explicit_lifetimes: [], }, + is_cref: false, }), is_const: false, unknown_attr: "", @@ -4445,6 +4448,7 @@ fn test_assumed_lifetimes_lifetimebound_free_function() { unknown_attr: "", explicit_lifetimes: [], }, + is_cref: false, }), is_const: false, unknown_attr: "", @@ -4466,6 +4470,7 @@ fn test_assumed_lifetimes_lifetimebound_free_function() { unknown_attr: "", explicit_lifetimes: [], }, + is_cref: false, }), is_const: false, unknown_attr: "", @@ -4508,6 +4513,7 @@ fn test_assumed_lifetimes_lifetime_capture_by_free_function() { unknown_attr: "", explicit_lifetimes: [], }, + is_cref: false, }), is_const: false, unknown_attr: "", @@ -4529,6 +4535,7 @@ fn test_assumed_lifetimes_lifetime_capture_by_free_function() { unknown_attr: "", explicit_lifetimes: [], }, + is_cref: false, }), is_const: false, unknown_attr: "",