[cxx-interop] Re-enable warnings for unannotated C++ APIs returning SWIFT_SHARED_REFERENCE types under an experimental feature flag #82488
+109
−38
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…S_(UN)RETAINED warning (swiftlang#81411)" This reverts commit 49aea31.
… per source location
fahadnayyar
requested review from
zoecarver,
egorzhdan,
Xazax-hun,
j-hui,
susmonteiro,
hnrklssn,
beccadax,
ian-twilightcoder,
hborla,
slavapestov,
xedin and
DougGregor
as code owners
|
@swift-ci please smoke test |
fahadnayyar
changed the title
Xazax-hun
reviewed
Jun 26, 2025
| /// Emit a warning when a C++ API returns a SWIFT_SHARED_REFERENCE type | ||
| /// without being explicitly annotated with either SWIFT_RETURNS_RETAINED | ||
| /// or SWIFT_RETURNS_UNRETAINED. | ||
| EXPERIMENTAL_FEATURE(WarnUnannotatedReturnOfCxxFrt, false) |
There was a problem hiding this comment.
Why is the second argument false here? Do we want this flag to be available for production compilers?
| clang::SourceLocation spellingLoc; | ||
| if (loc.isMacroID()) { | ||
| clang::SourceLocation expansionLoc = | ||
| sourceMgr.getImmediateExpansionRange(loc).getBegin(); |
There was a problem hiding this comment.
Would the immediate expansion range work as intended for nested macros?
There was a problem hiding this comment.
In what case do we need this trick? And why wouldn't always using the spelling location work?
| @@ -449,4 +449,34 @@ void bRelease(DefaultOwnershipInheritance::BaseTypeNonDefault *v) {}; | |||
| void dRetain(DefaultOwnershipInheritance::DerivedTypeNonDefault *v) {}; | |||
| void dRelease(DefaultOwnershipInheritance::DerivedTypeNonDefault *v) {}; | |||
|
|
|||
| namespace SourceLocationCaching { | |||
There was a problem hiding this comment.
I don't see testing for the macro case.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch re-enables diagnostics for unannotated C++ functions or methods returning
SWIFT_SHARED_REFERENCEtypes. These warnings now fire only once per source location, even in the presence of multiple template instantiations. This avoids diagnostic duplication that was a key source of noise in the compilation of larger codebases.These warnings were previously disabled starting in Swift 6.2 via PR-#81411 due to concerns around false positives and excessive duplication in projects adopting C++ interop. This patch addresses the duplication issue by adding source-location-based caching, which ensures that a warning is emitted only once per source location, even across template instantiations with different types.
However, the false positive issue remains to be investigated and will be addressed in a follow-up patch. Until that happens, the warnings are gated behind a new experimental feature flag:
WarnUnannotatedReturnOfCxxFrt. This feature will be enabled by default only after thorough qualification and testing on large C++ codebases.rdar://154261051