Skip to content

Avoid spurious clone emission for (void)-discarded function pointer casts in refcount model #289

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Copilot wants to merge 5 commits into from
Closed

Avoid spurious clone emission for (void)-discarded function pointer casts in refcount model #289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b29a3cf
Initial plan
Copilot Jul 25, 2026
6aebf02
Avoid cloning function pointers in void casts
Copilot Jul 25, 2026
9c227b6
Fix failing CI: update va_arg_fn_ptr.rs golden output for fn-ptr void…
Copilot Jul 25, 2026
4786462
Fix fn-ptr void-cast clone: skip only for fresh expressions, not vari…
Copilot Jul 25, 2026
1fa7b9a
fix: apply clang-format to converter_refcount.cpp
Copilot Jul 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion cpp2rust/converter/models/converter_refcount.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1322,7 +1322,15 @@ bool ConverterRefCount::VisitExplicitCastExpr(clang::ExplicitCastExpr *expr) {
if (expr->getTypeAsWritten()->isVoidType()) {
PushExprKind push(*this, ExprKind::Void);
Convert(expr->getSubExpr());
if (!TypeIsCopyable(expr->getSubExpr()->getType())) {
// Skip .clone() for fresh fn-ptr values (cast/constructor results).
// Variable references to fn-ptrs still need .clone() because FnPtr is
// non-Copy and (*var.borrow()) would otherwise attempt a move.
auto *inner = expr->getSubExpr()->IgnoreParenImpCasts();
bool is_fn_ptr_var_ref = clang::isa<clang::DeclRefExpr>(inner) &&

@lucic71 lucic71 Jul 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this entire check is focusing only on DeclRefExpr, while missing other AST shapes

The correct solution is probably to do computed_expr_type_ = ComputedExprType::FreshPointer in ConvertFunctionToFunctionPointer

Then the check in VisitExplicitCastExpr becomes: !TypeIsCopyable(sub->getType()) && !isFresh()

@lucic71 lucic71 Jul 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #290

inner->getType()->isFunctionPointerType();
if (!TypeIsCopyable(expr->getSubExpr()->getType()) &&
(!expr->getSubExpr()->getType()->isFunctionPointerType() ||
is_fn_ptr_var_ref)) {
StrCat(".clone()");
}
return false;
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/out/refcount/void_cast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,9 @@ fn main_0() -> i32 {
assert!(((*chosen.borrow()) == 123));
bump_and_return_4.clone();
assert!(((*side_effect_counter_3.with(Value::clone).borrow()) == 2));
(FnPtr::<fn() -> i32>::new(bump_and_return_4)).clone();
(FnPtr::<fn() -> i32>::new(bump_and_return_4));
assert!(((*side_effect_counter_3.with(Value::clone).borrow()) == 2));
((FnPtr::<fn() -> i32>::new(bump_and_return_4)).cast::<fn() -> i32>(None)).clone();
((FnPtr::<fn() -> i32>::new(bump_and_return_4)).cast::<fn() -> i32>(None));
assert!(((*side_effect_counter_3.with(Value::clone).borrow()) == 2));
let storage: Value<i32> = Rc::new(RefCell::new(11));
let p: Value<Ptr<i32>> = Rc::new(RefCell::new((storage.as_pointer())));
Expand Down
Loading