Skip to content

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

Open
nunoplopes with Copilot wants to merge 5 commits into
masterfrom
copilot/fix-spurious-clone-for-fn-ptrs
Open

Avoid spurious clone emission for (void)-discarded function pointer casts in refcount model#289
nunoplopes with Copilot wants to merge 5 commits into
masterfrom
copilot/fix-spurious-clone-for-fn-ptrs

Conversation

Copilot AI commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

(void)-discarded expressions involving casted function pointers were translated with an unnecessary .clone() in refcount output (e.g. (void)(static_cast<int (*)()>(&f))). This change removes that extra clone while preserving discard semantics.

  • Refcount converter: void-cast emission

    • Updated ConverterRefCount::VisitExplicitCastExpr to keep existing clone behavior for non-copyable discarded expressions, but skip clone when the discarded subexpression type is a function pointer.
    • This is scoped to (void) cast handling only.
  • Golden output update (unit regression coverage)

    • Updated tests/unit/out/refcount/void_cast.rs expected output to remove clone emission for discarded function-pointer expressions and discarded casted function-pointer expressions.
  • Before/after behavior

    // before
    ((FnPtr::<fn() -> i32>::new(bump_and_return_4)).cast::<fn() -> i32>(None)).clone();
    
    // after
    ((FnPtr::<fn() -> i32>::new(bump_and_return_4)).cast::<fn() -> i32>(None));

Copilot AI review requested due to automatic review settings July 25, 2026 17:58

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Copilot AI linked an issue Jul 25, 2026 that may be closed by this pull request
Copilot AI review requested due to automatic review settings July 25, 2026 18:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Copilot AI changed the title [WIP] Fix spurious clone for cast'ed function pointers Avoid spurious clone emission for (void)-discarded function pointer casts in refcount model Jul 25, 2026
Copilot AI requested a review from nunoplopes July 25, 2026 18:05
Copilot AI review requested due to automatic review settings July 25, 2026 18:41

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Copilot AI review requested due to automatic review settings July 25, 2026 20:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Copilot AI review requested due to automatic review settings July 25, 2026 20:34

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

@nunoplopes
nunoplopes marked this pull request as ready for review July 25, 2026 20:37
// 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) &&

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()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Spurious clone for cast'ed fn ptrs

4 participants