[cxx-interop] Enabling WarnUnannotatedReturnOfCxxFrt on by default and add it to a diagnostic group #84759
Conversation
fahadnayyar
force-pushed
the
fnayyar/misl-diag-group-default-on
branch
3 times, most recently
from
afaec26 to
20005bb
Compare
fahadnayyar
requested review from
DougGregor,
Xazax-hun,
egorzhdan,
hborla,
hnrklssn,
j-hui,
slavapestov,
susmonteiro,
xedin and
zoecarver
as code owners
|
@swift-ci please smoke test |
include/swift/Basic/Features.def
Outdated
| /// without being explicitly annotated with either SWIFT_RETURNS_RETAINED | ||
| /// or SWIFT_RETURNS_UNRETAINED. | ||
| EXPERIMENTAL_FEATURE(WarnUnannotatedReturnOfCxxFrt, true) | ||
| EXPERIMENTAL_FEATURE(SuppressWarnUnannotatedReturnOfCxxFrt, true) |
There was a problem hiding this comment.
Instead of inverting the feature flag, should we enable the experimental feature by default with a way to opt out? That might be more inline with what happens to other experimental features when they become language features.
There was a problem hiding this comment.
I am now making WarnUnannotatedReturnOfCxxFrt from an EXPERIMENTAL_FEATURE to a BASELINE_LANGUAGE_FEATURE. Users can opt-out of these warnings by using the CxxForeignReferenceType diagnostic group via the -Werror/-Wwarning flags. Does this approach seem reasonable?
There was a problem hiding this comment.
I think that would make sense to me, yes!
…d add it to a diagnostic group This change makes the warning for unannotated C++ functions returning foreign reference types (FRT) enabled by default, improving memory safety for Swift/C++ interop users. Also added CxxForeignReferenceType diagnostic group for better control
fahadnayyar
force-pushed
the
fnayyar/misl-diag-group-default-on
branch
from
20005bb to
31d7816
Compare
fahadnayyar
changed the title
| BASELINE_LANGUAGE_FEATURE(BuiltinTaskRunInline, 0, "Builtin.taskRunInline") | ||
| BASELINE_LANGUAGE_FEATURE(BuiltinUnprotectedAddressOf, 0, "Builtin.unprotectedAddressOf") | ||
| BASELINE_LANGUAGE_FEATURE(NewCxxMethodSafetyHeuristics, 0, "Only import C++ methods that return pointers (projections) on owned types as unsafe") | ||
| BASELINE_LANGUAGE_FEATURE(WarnUnannotatedReturnOfCxxFrt, 0, "Warn about unannotated C++ functions returning foreign reference types") |
There was a problem hiding this comment.
I don't think we need a feature for this at all, because nobody needs to do #if hasFeature(WarnUnannotatedReturnOfCxxFrt) or opt in to it
| GROUP(AvailabilityUnrecognizedName, "availability-unrecognized-name") | ||
| GROUP(ClangDeclarationImport, "clang-declaration-import") | ||
| GROUP(ConformanceIsolation, "conformance-isolation") | ||
| GROUP(CxxForeignReferenceType, "cxx-foreign-reference-type") |
There was a problem hiding this comment.
Please remove the "cxx" from these. It's not a C++-specific feature.
| @@ -0,0 +1,44 @@ | |||
| # C++ Foreign Reference Type warnings (CxxForeignReferenceType) | |||
There was a problem hiding this comment.
As I noted earlier, SWIFT_SHARED_REFERENCE isn't a C++-specific feature. There are C types that get imported as shared references, too. Please remove the C++-specific bits; generally speaking, replacing C++ with C in this markdown file looks sufficient.
|
|
||
| ## Overview | ||
|
|
||
| When importing C++ types marked with `SWIFT_SHARED_REFERENCE` into Swift, the compiler needs to understand the ownership semantics of functions returning these types. Without explicit annotations, the compiler cannot determine whether the returned object should be retained or not, which can lead to memory management issues. |
There was a problem hiding this comment.
Is it worth saying that these types are imported as classes in Swift, and showing an example declaration like
typedef struct SWIFT_SHARED_REFERENCE(MyFRT_retain, MyFRT_release) MyFRT;
| @@ -0,0 +1,44 @@ | |||
| # C++ Foreign Reference Type warnings (CxxForeignReferenceType) | |||
|
|
|||
| Warnings related to C++ APIs returning foreign reference types that lack proper ownership annotations. | |||
There was a problem hiding this comment.
Thank you for the descriptive documentation group. It's very helpful to our users.
4 participants
Enabling the warning on unannotated C++ functions returning foreign reference types (
SWIFT_SHARED_REFERENCE) on by default to improve memory safety when calling such APIs from SwiftChanges
WarnUnannotatedReturnOfCxxFrtfrom anEXPERIMENTAL_FEATUREto aBASELINE_LANGUAGE_FEATURECxxForeignReferenceTypefor granular control via-Werror/-Wwarninguserdocs/diagnostics/cxx-foreign-reference-type.md