Skip to content

Commit 77c219a

Browse files
nunoplopesnunoplopes
authored
Remove Ptr::to_strong() and add Ptr::decay() (#410)
To decay a pointer, we would previously translate first to a strong reference and then call `as_pointer`. This patch avoids that by 1) decaying pointers whenever possible and by using `decay` as a more efficient way to decay pointers (does only a weak clone).
1 parent d0194c3 commit 77c219a

27 files changed

Lines changed: 380 additions & 363 deletions

‎.github/workflows/format.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,5 +39,5 @@ jobs:
3939
run: git diff --exit-code
4040

4141
- name: Check markdown formatting
42-
run: npx prettier@3.6.2 --check "src/**/*.md"
42+
run: npx prettier@3.9.8 --check "src/**/*.md"
4343
working-directory: docs

‎CMakeLists.txt‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,9 @@ add_custom_target("format"
9696
--manifest-path ${PROJECT_SOURCE_DIR}/libcc2rs-macros/Cargo.toml -- -Dwarnings
9797
COMMAND ${RUFF} --silent check --fix ${PYTHON_SOURCES}
9898
COMMAND ${RUFF} --silent format ${PYTHON_SOURCES}
99+
COMMAND npx prettier@3.9.8 --write ${PROJECT_SOURCE_DIR}/docs/src/**/*.md
99100
DEPENDS "${RUST_STAMP_FILE}"
100-
COMMENT "Running clang-format, cargo fmt, cargo clippy, and ruff on all source files"
101+
COMMENT "Running clang-format, cargo fmt, cargo clippy, ruff, and prettier on all source files"
101102
VERBATIM
102103
)
103104

@@ -152,6 +153,13 @@ set(RULES_LIB "${PROJECT_SOURCE_DIR}/rules/target/release/librules.rlib")
152153
file(GLOB_RECURSE rule_preprocessor_sources
153154
${PROJECT_SOURCE_DIR}/rule-preprocessor/src/*.rs)
154155

156+
file(GLOB_RECURSE libcc2rs_sources CONFIGURE_DEPENDS
157+
${PROJECT_SOURCE_DIR}/libcc2rs/src/*.rs
158+
${PROJECT_SOURCE_DIR}/libcc2rs-macros/src/*.rs)
159+
list(APPEND libcc2rs_sources
160+
${PROJECT_SOURCE_DIR}/libcc2rs/Cargo.toml
161+
${PROJECT_SOURCE_DIR}/libcc2rs-macros/Cargo.toml)
162+
155163
execute_process(
156164
COMMAND ${CMAKE_COMMAND} -E env RUSTUP_TOOLCHAIN=${RUST_NIGHTLY_VERSION}
157165
rustc --print sysroot
@@ -165,6 +173,7 @@ add_custom_command(
165173
--manifest-path "${PROJECT_SOURCE_DIR}/rules/Cargo.toml"
166174
DEPENDS "${PROJECT_SOURCE_DIR}/rules/build.rs"
167175
"${PROJECT_SOURCE_DIR}/rules/Cargo.toml"
176+
${libcc2rs_sources}
168177
"${RUST_STAMP_FILE}"
169178
DEPFILE "${PROJECT_SOURCE_DIR}/rules/target/release/librules.d"
170179
VERBATIM
@@ -177,6 +186,7 @@ add_custom_command(
177186
cargo +${RUST_NIGHTLY_VERSION} build --release
178187
--manifest-path "${PROJECT_SOURCE_DIR}/rule-preprocessor/Cargo.toml"
179188
DEPENDS ${rule_preprocessor_sources}
189+
${libcc2rs_sources}
180190
"${RUST_STAMP_FILE}"
181191
VERBATIM
182192
)

‎cpp2rust/converter/converter.cpp‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ std::string Converter::ConvertFreshPointer(clang::Expr *expr) {
244244
return str;
245245
}
246246

247-
std::string Converter::ConvertFreshObject(clang::Expr *expr) {
247+
std::string Converter::ConvertFreshObject(clang::Expr *expr, std::string_view) {
248248
return ConvertFreshPointer(expr);
249249
}
250250

@@ -4858,9 +4858,10 @@ std::string Converter::ConvertPlaceholder(clang::Expr *expr, clang::Expr *arg,
48584858
}
48594859

48604860
if (ph_ctx.needs_pointer_receiver()) {
4861-
return std::format(
4862-
"({} as {})", ConvertFreshObject(arg),
4863-
Mapper::GetParamType(GetCalleeOrExpr(expr), ph_ctx.arg_idx));
4861+
auto param_type =
4862+
Mapper::GetParamType(GetCalleeOrExpr(expr), ph_ctx.arg_idx);
4863+
return std::format("({} as {})", ConvertFreshObject(arg, param_type),
4864+
param_type);
48644865
}
48654866

48664867
if (ph_ctx.needs_object_receiver()) {

‎cpp2rust/converter/converter.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1016,7 +1016,10 @@ class Converter : public clang::RecursiveASTVisitor<Converter> {
10161016
ConvertFreshRValue(clang::Expr *expr,
10171017
std::optional<clang::QualType> implicit_convert_to = {});
10181018
virtual std::string ConvertFreshPointer(clang::Expr *expr);
1019-
virtual std::string ConvertFreshObject(clang::Expr *expr);
1019+
// target_ptr_type, when known (e.g. a translation rule's parameter type),
1020+
// is the Rust pointer type the result will be used as.
1021+
virtual std::string ConvertFreshObject(clang::Expr *expr,
1022+
std::string_view target_ptr_type = {});
10201023
std::string ConvertPointer(clang::Expr *expr, int line = __builtin_LINE());
10211024

10221025
/// Materialize a temporary for a prvalue bound to a reference parameter.

‎cpp2rust/converter/models/converter_refcount.cpp‎

Lines changed: 76 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -183,9 +183,24 @@ bool ConverterRefCount::VisitIncompleteArrayType(
183183
}
184184

185185
bool ConverterRefCount::VisitReferenceType(clang::ReferenceType *type) {
186+
auto pointee_type = type->getPointeeType();
187+
if (pointee_type->isArrayType()) {
188+
// A reference to an array decays straight to a pointer to its first
189+
// element, the same way a by-value array parameter would, instead of
190+
// going through a pointer to the whole boxed array.
191+
auto element_type = pointee_type->getAsArrayTypeUnsafe()->getElementType();
192+
PushConversionKind push1(*this, ConversionKind::Ptr,
193+
!element_type->isArrayType());
194+
PushConversionKind push2(*this, ConversionKind::FullRefCount,
195+
element_type->isArrayType());
196+
StrCat("Ptr<");
197+
Convert(element_type);
198+
StrCat(token::kGt);
199+
return false;
200+
}
186201
PushConversionKind push(*this, ConversionKind::Pointee);
187202
StrCat("Ptr<");
188-
Convert(type->getPointeeType());
203+
Convert(pointee_type);
189204
StrCat(token::kGt);
190205
return false;
191206
}
@@ -321,18 +336,43 @@ std::string ConverterRefCount::ConvertFreshLValue(clang::Expr *expr) {
321336
return std::format("({}).clone()", std::move(str));
322337
}
323338

324-
std::string ConverterRefCount::ConvertObject(clang::Expr *expr) {
339+
std::string ConverterRefCount::ConvertObject(clang::Expr *expr,
340+
ObjectShape shape) {
325341
PushExprKind push(*this, ExprKind::Object);
342+
auto saved_shape = std::exchange(object_shape_, shape);
326343
auto str = ToString(expr);
327-
if (expr->getType()->isPointerType()) {
328-
computed_expr_type_ = ComputedExprType::FreshPointer;
329-
return std::format("{}.to_strong().as_pointer()", std::move(str));
344+
object_shape_ = saved_shape;
345+
if (shape == ObjectShape::Element && expr->getType()->isPointerType()) {
346+
auto pointee = expr->getType()->getPointeeType();
347+
if (IsBoxedType(pointee) || pointee->isArrayType()) {
348+
computed_expr_type_ = ComputedExprType::FreshPointer;
349+
return std::format("{}.decay()", std::move(str));
350+
}
330351
}
331352
return str;
332353
}
333354

334-
std::string ConverterRefCount::ConvertFreshObject(clang::Expr *expr) {
335-
auto str = ConvertObject(expr);
355+
std::string
356+
ConverterRefCount::ConvertFreshObject(clang::Expr *expr,
357+
std::string_view target_ptr_type) {
358+
auto shape = ObjectShape::Whole;
359+
if (!target_ptr_type.empty()) {
360+
auto type = expr->getType().getNonReferenceType();
361+
auto pointee = type->isPointerType() ? type->getPointeeType() : type;
362+
if (IsBoxedType(pointee) || pointee->isArrayType()) {
363+
auto normalize = [](std::string s) {
364+
std::erase(s, ' ');
365+
for (size_t pos; (pos = s.find("::<")) != std::string::npos;)
366+
s.erase(pos, 2);
367+
return s;
368+
};
369+
if (normalize(std::string(target_ptr_type)) ==
370+
normalize(ConvertPtrType(pointee))) {
371+
shape = ObjectShape::Element;
372+
}
373+
}
374+
}
375+
auto str = ConvertObject(expr, shape);
336376
if (isFresh()) {
337377
return str;
338378
}
@@ -878,15 +918,10 @@ bool ConverterRefCount::VisitDeclRefExpr(clang::DeclRefExpr *expr) {
878918
return false;
879919
}
880920

881-
// std::vector<T>& gets converted to Ptr<vec<T>>
882-
// So we need to make a pointer to the vector itself
883-
if (isObject()) {
884-
if (IsBoxedType(ref->getPointeeType()) ||
885-
ref->getPointeeType()->isArrayType()) {
886-
StrCat(str, ".to_strong().as_pointer()");
887-
computed_expr_type_ = ComputedExprType::FreshPointer;
888-
return false;
889-
}
921+
if (isObject() && WantsElementPtr() && IsBoxedType(ref->getPointeeType())) {
922+
StrCat(str, ".decay()");
923+
computed_expr_type_ = ComputedExprType::FreshPointer;
924+
return false;
890925
}
891926

892927
// references are not boxed
@@ -1174,8 +1209,10 @@ bool ConverterRefCount::VisitCallExpr(clang::CallExpr *expr) {
11741209
return false;
11751210
}
11761211

1177-
if (isObject()) {
1178-
StrCat(std::format("{}.to_strong().as_pointer()", std::move(str)));
1212+
if (isObject() && WantsElementPtr() && ref &&
1213+
IsBoxedType(ref->getPointeeType())) {
1214+
StrCat(std::format("{}.decay()", std::move(str)));
1215+
computed_expr_type_ = ComputedExprType::FreshPointer;
11791216
return false;
11801217
}
11811218

@@ -1281,7 +1318,7 @@ bool ConverterRefCount::VisitImplicitCastExpr(clang::ImplicitCastExpr *expr) {
12811318

12821319
if (pointee_type && abstract_structs_.contains(GetID(pointee_type))) {
12831320
PushConversionKind push(*this, ConversionKind::Unboxed);
1284-
StrCat(std::format("({}.to_strong() as Value<{}>).as_pointer_dyn()",
1321+
StrCat(std::format("{}.to_dyn::<{}>(|w| w)",
12851322
ToString(sub_expr->IgnoreCasts()),
12861323
ConvertPointeeType(expr->getType())));
12871324
computed_expr_type_ = ComputedExprType::FreshPointer;
@@ -1309,10 +1346,10 @@ bool ConverterRefCount::VisitImplicitCastExpr(clang::ImplicitCastExpr *expr) {
13091346
// smart enough to pick the right specialization
13101347
PushConversionKind push(*this, ConversionKind::Unboxed);
13111348
PushParen paren(*this);
1312-
StrCat(IsReferenceType(sub_expr) ? ConvertObject(sub_expr)
1313-
: ConvertPointer(sub_expr),
1314-
keyword::kAs, ToString(expr->getType()));
1315-
computed_expr_type_ = ComputedExprType::FreshPointer;
1349+
StrCat(ConvertPointer(sub_expr));
1350+
if (!IsReferenceType(sub_expr)) {
1351+
StrCat(keyword::kAs, ToString(expr->getType()));
1352+
}
13161353
return false;
13171354
}
13181355
}
@@ -1899,7 +1936,8 @@ bool ConverterRefCount::VisitCXXForRangeStmtVector(
18991936
StrCat("'loop_:");
19001937
StrCat(keyword::kFor,
19011938
stmt->getLoopVariable()->getType().isConstQualified() ? "" : "mut",
1902-
loop_var_name, keyword::kIn, ConvertObject(stmt->getRangeInit()));
1939+
loop_var_name, keyword::kIn,
1940+
ConvertObject(stmt->getRangeInit(), ObjectShape::Element));
19031941
StrCat(keyword::kAs, ConvertPtrType(stmt->getRangeInit()->getType()));
19041942

19051943
PushBrace brace(*this);
@@ -1941,7 +1979,8 @@ bool ConverterRefCount::VisitCXXForRangeStmtString(
19411979
StrCat("'loop_:");
19421980
StrCat(keyword::kFor,
19431981
stmt->getLoopVariable()->getType().isConstQualified() ? "" : "mut",
1944-
loop_var_name, keyword::kIn, ConvertObject(stmt->getRangeInit()));
1982+
loop_var_name, keyword::kIn,
1983+
ConvertObject(stmt->getRangeInit(), ObjectShape::Element));
19451984
StrCat(".to_string_iterator() as StringIterator<",
19461985
ToString(loop_var->getType().getNonReferenceType()), '>');
19471986

@@ -2196,8 +2235,7 @@ std::string ConverterRefCount::ConvertVarInitValue(clang::QualType qual_type,
21962235
ctx_.getAsArrayType(
21972236
expr->IgnoreParens()->IgnoreImplicit()->getType())
21982237
->getElementType());
2199-
return std::format("Ptr::<Box<[{}]>>::from_string_literal_array({})",
2200-
code_unit,
2238+
return std::format("Ptr::<{}>::from_string_literal({})", code_unit,
22012239
ToString(expr->IgnoreParens()->IgnoreImplicit()));
22022240
}
22032241
return std::format("({} as {})", ConvertFreshPointer(expr),
@@ -2409,11 +2447,12 @@ bool ConverterRefCount::ConvertCXXOperatorCallExpr(
24092447

24102448
if (isLValue()) {
24112449
PushConversionKind push_ck(*this, ConversionKind::Unboxed);
2412-
pending_deref_.set(std::format("({} as {}).offset({})",
2413-
ConvertObject(expr->getArg(0)),
2414-
ConvertPtrType(expr->getArg(0)->getType()),
2415-
ConvertSubscriptIndex(expr->getArg(1))),
2416-
/*fresh=*/true, expr);
2450+
pending_deref_.set(
2451+
std::format("({} as {}).offset({})",
2452+
ConvertObject(expr->getArg(0), ObjectShape::Element),
2453+
ConvertPtrType(expr->getArg(0)->getType()),
2454+
ConvertSubscriptIndex(expr->getArg(1))),
2455+
/*fresh=*/true, expr);
24172456
break;
24182457
}
24192458

@@ -2430,7 +2469,7 @@ bool ConverterRefCount::ConvertCXXOperatorCallExpr(
24302469

24312470
PushConversionKind push(*this, ConversionKind::Unboxed);
24322471
StrCat(std::format("({} as {}).offset({})",
2433-
ConvertObject(expr->getArg(0)),
2472+
ConvertObject(expr->getArg(0), ObjectShape::Element),
24342473
ConvertPtrType(expr->getArg(0)->getType()),
24352474
ConvertSubscriptIndex(expr->getArg(1))));
24362475

@@ -2611,11 +2650,10 @@ void ConverterRefCount::ConvertDeref(clang::Expr *expr) {
26112650
}
26122651
}
26132652

2614-
if (isObject()) {
2615-
if (IsBoxedType(pointee_type) || pointee_type->isArrayType()) {
2616-
StrCat(".to_strong().as_pointer()");
2617-
computed_expr_type_ = ComputedExprType::FreshPointer;
2618-
}
2653+
if (isObject() && WantsElementPtr() &&
2654+
(IsBoxedType(pointee_type) || pointee_type->isArrayType())) {
2655+
StrCat(".decay()");
2656+
computed_expr_type_ = ComputedExprType::FreshPointer;
26192657
}
26202658
}
26212659

‎cpp2rust/converter/models/converter_refcount.h‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,8 +282,16 @@ class ConverterRefCount final : public Converter {
282282
std::string GetInnerType(clang::QualType type);
283283

284284
std::string ConvertFreshLValue(clang::Expr *expr);
285-
std::string ConvertObject(clang::Expr *expr);
286-
std::string ConvertFreshObject(clang::Expr *expr) override;
285+
// What an Object-kind conversion of a boxed container/array should yield:
286+
// a pointer to the whole object (Ptr<Vec<T>>), or one to its first
287+
// element (Ptr<T>).
288+
enum class ObjectShape { Whole, Element };
289+
std::string ConvertObject(clang::Expr *expr,
290+
ObjectShape shape = ObjectShape::Whole);
291+
std::string
292+
ConvertFreshObject(clang::Expr *expr,
293+
std::string_view target_ptr_type = {}) override;
294+
bool WantsElementPtr() const { return object_shape_ == ObjectShape::Element; }
287295
std::string
288296
ConvertFresh(clang::Expr *expr,
289297
std::optional<clang::QualType> implicit_convert_to = {});
@@ -369,6 +377,7 @@ class ConverterRefCount final : public Converter {
369377
std::string BoxValue(std::string &&str) const;
370378

371379
std::vector<ConversionKind> conversion_kind_;
380+
ObjectShape object_shape_ = ObjectShape::Whole;
372381

373382
// Set by pointer-related visit methods (ConvertDeref,
374383
// ConvertPointerSubscript, etc.) when converting an LValue that goes through

‎docs/src/codegen/types/casts.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,8 @@ allocation as bytes. Three helpers from the runtime cover the cases:
122122
Two casts do not use these helpers. An array decaying to a pointer is spelled
123123
`arr.as_pointer() as Ptr<T>`, where the `as` only names the pointer type. An
124124
upcast from a derived class to an abstract base becomes
125-
`(p.to_strong() as Value<dyn Base>).as_pointer_dyn()`, which is the ordinary
126-
Rust unsizing coercion applied to the owning cell (see
125+
`p.to_dyn::<dyn Base>(|w| w)`, which is the ordinary Rust unsizing coercion
126+
applied to the pointer's weak reference (see
127127
[Virtual Classes](../../runtime/ptr-dyn.md)). Casts between pointers and
128128
integers use the [integer cast](../../runtime/rc.md#integer-casts) API of `Ptr`.
129129

‎docs/src/runtime/ptr-dyn.md‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,17 @@ bound. The runtime provides a dedicated `PtrDyn<dyn T>` type, declared with
88
for dynamic dispatch.
99

1010
A `PtrDyn` is created at the point where C++ converts a derived pointer to a
11-
base pointer. `to_strong` upgrades the `Ptr<Derived>` into its `Value<Derived>`,
12-
Rust's unsized coercion turns that into a `Value<dyn Base>`, and
13-
`as_pointer_dyn` takes the weak reference back out.
11+
base pointer. `Ptr::to_dyn` takes the weak reference out of the `Ptr<Derived>`
12+
and applies Rust's unsized coercion to it, turning a `Weak<RefCell<Derived>>`
13+
into a `Weak<RefCell<dyn Base>>`, without ever upgrading it. The coercion itself
14+
is written by the code generator as the closure `|w| w`, whose return type
15+
selects the target trait object.
1416

1517
> [!NOTE]
1618
>
17-
> This coercion is why `Value` is a type alias for `Rc<RefCell<T>>` rather than
18-
> a struct of its own: `Rc` already implements it, and a new type could only opt
19-
> in through the nightly-only `CoerceUnsized` trait.
19+
> This coercion is why the pointee cell is a plain `RefCell` behind `Weak`
20+
> rather than a struct of the runtime's own: `Weak` already implements it, and a
21+
> new type could only opt in through the nightly-only `CoerceUnsized` trait.
2022
2123
The conversion looks like this:
2224

@@ -32,7 +34,7 @@ int r = b->f();
3234
```rust
3335
let d: Value<Derived> = Rc::new(RefCell::new(<Derived>::default()));
3436
let b: Value<PtrDyn<dyn Base>> = Rc::new(RefCell::new(
35-
((d.as_pointer()).to_strong() as Value<dyn Base>).as_pointer_dyn(),
37+
(d.as_pointer()).to_dyn::<dyn Base>(|w| w),
3638
));
3739
let r: Value<i32> = Rc::new(RefCell::new(
3840
({ (*(*b.borrow()).upgrade().deref()).f() }),
@@ -51,7 +53,9 @@ the call dispatches through the trait's vtable.
5153
> virtual call, so a method that deletes its own object panics on `delete`.
5254
5355
`PtrDyn` is far smaller than `Ptr`: it is either null or a weak reference to a
54-
single object. It has no arithmetic, no comparison, no array kinds, and no byte
55-
view. Because `to_strong` is only defined for single-value pointers, a base
56-
pointer into an array of polymorphic objects (a `Derived arr[N]` walked through
57-
a `Base *`) cannot be formed.
56+
single object, on the stack or on the heap. `Ptr::to_dyn` keeps that
57+
distinction, so a base pointer made from a `new`ed object can be `delete`d and
58+
one made from a local cannot. It has no arithmetic, no comparison, no array
59+
kinds, and no byte view. Because `to_dyn` is only defined for single-value
60+
pointers, a base pointer into an array of polymorphic objects (a
61+
`Derived arr[N]` walked through a `Base *`) cannot be formed.

0 commit comments

Comments
 (0)