Skip to content

Conversation

fahadnayyar
Copy link
Contributor

@fahadnayyar fahadnayyar commented Jul 14, 2025

This patch improves the warning for C++ APIs returning SWIFT_SHARED_REFERENCE types but not annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED in the following ways:

  1. The warning for missing SWIFT_RETURNS_(UN)RETAINED annotations is now emitted on Swift use sites, rather than while importing the API (func/method decls).
  • This logic is now implemented as a Misl Diagnostic in function diagnoseCxxFunctionCalls in file lib/Sema/MiscDiagnostics.cpp.
  • The warning is now triggered only when the API is actually used, which reduces noise in large C++ headers.

rdar://150800115

@fahadnayyar fahadnayyar self-assigned this Jul 14, 2025
@fahadnayyar fahadnayyar added the c++ interop Feature: Interoperability with C++ label Jul 14, 2025
@fahadnayyar fahadnayyar force-pushed the fnayyar/unannotated-returns-unretained-warn-false-positives-fix branch from 23b360e to 96c60e7 Compare July 15, 2025 10:07
@fahadnayyar fahadnayyar marked this pull request as ready for review July 15, 2025 10:16
@Xazax-hun

This comment was marked as resolved.

@Xazax-hun

This comment was marked as resolved.

@fahadnayyar

This comment was marked as resolved.

@fahadnayyar

This comment was marked as resolved.

j-hui

This comment was marked as resolved.

egorzhdan

This comment was marked as resolved.

@fahadnayyar

This comment was marked as resolved.

@fahadnayyar

This comment was marked as resolved.

@fahadnayyar fahadnayyar force-pushed the fnayyar/unannotated-returns-unretained-warn-false-positives-fix branch from 6c52fa5 to 1024a33 Compare July 23, 2025 20:12
j-hui

This comment was marked as resolved.

@fahadnayyar fahadnayyar marked this pull request as draft August 4, 2025 01:57
@fahadnayyar

This comment was marked as resolved.

Copy link
Contributor

@Xazax-hun Xazax-hun left a comment

Choose a reason for hiding this comment

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

This looks really clean now, thanks!

@fahadnayyar fahadnayyar force-pushed the fnayyar/unannotated-returns-unretained-warn-false-positives-fix branch from 5adb4f5 to 1389b87 Compare August 6, 2025 23:15
@fahadnayyar fahadnayyar marked this pull request as ready for review August 19, 2025 15:14
@fahadnayyar fahadnayyar marked this pull request as draft August 19, 2025 20:33
Copy link
Contributor

@Xazax-hun Xazax-hun left a comment

Choose a reason for hiding this comment

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

I have one nit inline, once that is addressed the PR looks good to me.

@fahadnayyar fahadnayyar force-pushed the fnayyar/unannotated-returns-unretained-warn-false-positives-fix branch 2 times, most recently from 36af6cc to 2bf0bb0 Compare August 21, 2025 23:08
@fahadnayyar fahadnayyar changed the title Diagnose unannotated C++ APIs returning SWIFT_SHARED_REFERENCE types at their Swift usage sites Diagnose unannotated C++ APIs returning SWIFT_SHARED_REFERENCE at Swift call sites Aug 21, 2025
@fahadnayyar fahadnayyar marked this pull request as ready for review August 21, 2025 23:20
…D_REFERENCE without ownership annotations

Move the “unannotated SWIFT_SHARED_REFERENCE return” diagnostic from
ClangImporter to Sema use sites (call expressions). This reduces the
noise from large headers and only warns when the API is actually
used.

- New warning: warn_unannotated_cxx_func_returning_frt
  “cannot infer the ownership of the returned value, annotate %0 with
   either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED”
  plus a note on the declaration site.
- Gated by -enable-experimental-feature WarnUnannotatedReturnOfCxxFrt.

- Implementation:
  * Add diagnoseCxxFunctionCalls() walker in MiscDiagnostics.
  * Add helpers: ReturnOwnershipInfo, hasImmortalAttrs, isReturningFRT().
  * Remove the old import-time GROUPED_WARNING and update tests.

rdar://150800115
@fahadnayyar fahadnayyar force-pushed the fnayyar/unannotated-returns-unretained-warn-false-positives-fix branch from 2bf0bb0 to 5df006b Compare August 22, 2025 23:15
@fahadnayyar
Copy link
Contributor Author

@swift-ci please smoke test

@fahadnayyar
Copy link
Contributor Author

@swift-ci please smoke test windows platform

@fahadnayyar fahadnayyar merged commit 720406f into swiftlang:main Aug 25, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ interop Feature: Interoperability with C++
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants