From 2bc6ed0768f9c987466f8659ef7a34a9af92812e Mon Sep 17 00:00:00 2001 From: fahadnayyar Date: Mon, 30 Sep 2024 17:15:59 -0700 Subject: [PATCH] [cxx-interop] Warning unannotated C++ APIs returning SWIFT_SHARED_REFERENCE types --- .../swift/AST/DiagnosticsClangImporter.def | 12 ++++- lib/ClangImporter/ImportDecl.cpp | 44 ++++++++++++++++--- .../cxx-functions-and-methods-returning-frt.h | 8 ++++ ...retained-unretained-attributes-error.swift | 6 +++ 4 files changed, 61 insertions(+), 9 deletions(-) diff --git a/include/swift/AST/DiagnosticsClangImporter.def b/include/swift/AST/DiagnosticsClangImporter.def index f118710b6c7c2..d6f5df6d9bcff 100644 --- a/include/swift/AST/DiagnosticsClangImporter.def +++ b/include/swift/AST/DiagnosticsClangImporter.def @@ -258,8 +258,16 @@ ERROR(failed_base_method_call_synthesis,none, "failed to synthesize call to the base method %0 of type %0", (ValueDecl *, ValueDecl *)) -ERROR(both_returns_retained_returns_unretained,none, - "%0 cannot be annotated with both swift_attr('returns_retained') and swift_attr('returns_unretained') attributes", (const clang::NamedDecl*)) +ERROR(both_returns_retained_returns_unretained, none, + "%0 cannot be annotated with both swift_attr('returns_retained') and " + "swift_attr('returns_unretained') attributes", + (const clang::NamedDecl *)) + +WARNING(no_returns_retained_returns_unretained, none, + "%0 is returning a 'SWIFT_SHARED_REFERENCE' type but is not annotated " + "with either swift_attr('returns_retained') or " + "swift_attr('returns_unretained') attributes", + (const clang::NamedDecl *)) NOTE(unsupported_builtin_type, none, "built-in type '%0' not supported", (StringRef)) NOTE(record_field_not_imported, none, "field %0 unavailable (cannot import)", (const clang::NamedDecl*)) diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp index 6154ad15b2dbb..cc64964d6f5f1 100644 --- a/lib/ClangImporter/ImportDecl.cpp +++ b/lib/ClangImporter/ImportDecl.cpp @@ -2684,7 +2684,7 @@ namespace { Impl.diagnoseTopLevelValue( DeclName(Impl.SwiftContext.getIdentifier(releaseOperation.name))); } - }else if (releaseOperation.kind == + } else if (releaseOperation.kind == CustomRefCountingOperationResult::tooManyFound) { HeaderLoc loc(decl->getLocation()); Impl.diagnose(loc, @@ -3356,9 +3356,9 @@ namespace { // swift_attr("returns_unretained") then emit an error in the swift // compiler. Note: this error is not emitted in the clang compiler because // these attributes are used only for swift interop. + bool returnsRetainedAttrIsPresent = false; + bool returnsUnretainedAttrIsPresent = false; if (decl->hasAttrs()) { - bool returnsRetainedAttrIsPresent = false; - bool returnsUnretainedAttrIsPresent = false; for (const auto *attr : decl->getAttrs()) { if (const auto *swiftAttr = dyn_cast(attr)) { if (swiftAttr->getAttribute() == "returns_unretained") { @@ -3368,11 +3368,18 @@ namespace { } } } + } + + HeaderLoc loc(decl->getLocation()); + if (returnsRetainedAttrIsPresent && returnsUnretainedAttrIsPresent) { + Impl.diagnose(loc, diag::both_returns_retained_returns_unretained, + decl); + } - if (returnsRetainedAttrIsPresent && returnsUnretainedAttrIsPresent) { - HeaderLoc loc(decl->getLocation()); - Impl.diagnose(loc, diag::both_returns_retained_returns_unretained, - decl); + if ((!returnsRetainedAttrIsPresent) && (!returnsUnretainedAttrIsPresent)) { + if (isForeignReferenceType(decl->getReturnType())) { + Impl.diagnose(loc, diag::no_returns_retained_returns_unretained, + decl); } } @@ -3380,6 +3387,29 @@ namespace { std::nullopt); } + static bool isForeignReferenceType(const clang::QualType type) { + if (!type->isPointerType()) + return false; + + auto pointeeType = + dyn_cast(type->getPointeeType().getCanonicalType()); + if (pointeeType == nullptr) + return false; + + return hasImportAsRefAttr(pointeeType->getDecl()); + } + + static bool hasImportAsRefAttr(const clang::RecordDecl *decl) { + return decl->hasAttrs() && llvm::any_of(decl->getAttrs(), [](auto *attr) { + if (auto swiftAttr = dyn_cast(attr)) + return swiftAttr->getAttribute() == "import_reference" || + // TODO: Remove this once libSwift hosttools no longer + // requires it. + swiftAttr->getAttribute() == "import_as_ref"; + return false; + }); + } + /// Handles special functions such as subscripts and dereference operators. bool processSpecialImportedFunc(FuncDecl *func, ImportedName importedName, diff --git a/test/Interop/Cxx/foreign-reference/Inputs/cxx-functions-and-methods-returning-frt.h b/test/Interop/Cxx/foreign-reference/Inputs/cxx-functions-and-methods-returning-frt.h index 71a003aebbf9b..deec63f241e9e 100644 --- a/test/Interop/Cxx/foreign-reference/Inputs/cxx-functions-and-methods-returning-frt.h +++ b/test/Interop/Cxx/foreign-reference/Inputs/cxx-functions-and-methods-returning-frt.h @@ -158,6 +158,14 @@ struct __attribute__((swift_attr("returns_unretained"))); }; +// C++ APIs returning FRTs but not having annotations +struct + StructWithoutReturnsAnnotations { + static FRTStruct *_Nonnull StaticMethodReturningFRT(); +}; + +FRTStruct *_Nonnull global_function_returning_FRT_WithoutReturnsAnnotations(); + struct NonFRTStruct {}; // Global/free C++ functions returning non-FRT diff --git a/test/Interop/Cxx/foreign-reference/frt-retained-unretained-attributes-error.swift b/test/Interop/Cxx/foreign-reference/frt-retained-unretained-attributes-error.swift index 5fa18cdd90dea..c90e98763a33e 100644 --- a/test/Interop/Cxx/foreign-reference/frt-retained-unretained-attributes-error.swift +++ b/test/Interop/Cxx/foreign-reference/frt-retained-unretained-attributes-error.swift @@ -8,3 +8,9 @@ let frtLocalVar1 = global_function_returning_FRT_with_both_attrs_returns_retaine let frtLocalVar2 = StructWithStaticMethodsReturningFRTWithBothAttributesReturnsRetainedAndReturnsUnretained.StaticMethodReturningFRT() // CHECK: error: 'StaticMethodReturningFRT' cannot be annotated with both swift_attr('returns_retained') and swift_attr('returns_unretained') attributes + +let frtLocalVar3 = global_function_returning_FRT_WithoutReturnsAnnotations() +// CHECK: warning: 'global_function_returning_FRT_WithoutReturnsAnnotations' is returning a 'SWIFT_SHARED_REFERENCE' type but is not annotated with either swift_attr('returns_retained') or swift_attr('returns_unretained') attributes + +let frtLocalVar4 = StructWithoutReturnsAnnotations.StaticMethodReturningFRT() +// CHECK: warning: 'StaticMethodReturningFRT' is returning a 'SWIFT_SHARED_REFERENCE' type but is not annotated with either swift_attr('returns_retained') or swift_attr('returns_unretained') attributes