Inferring SWIFT_RETURNS_(UN)RETAINED_BY_DEFAULT annotations in C++ inheritance

Author: fahadnayyar

lib/ClangImporter/ImportDecl.cpp

@@ -3618,6 +3618,31 @@ namespace {
       return std::nullopt;
     }
 
+    template <typename T>
+    std::optional<T> matchSwiftAttrConsideringInheritance(
+        const clang::Decl *decl,
+        llvm::ArrayRef<std::pair<llvm::StringRef, T>> patterns) {
+      if (!decl)
+        return std::nullopt;
+
+      if (auto match = matchSwiftAttr<T>(decl, patterns))
+        return match;
+
+      if (const auto *recordDecl = llvm::dyn_cast<clang::CXXRecordDecl>(decl)) {
+        for (const auto &baseSpecifier : recordDecl->bases()) {
+          const clang::CXXRecordDecl *baseDecl =
+              baseSpecifier.getType()->getAsCXXRecordDecl();
+          if (!baseDecl)
+            continue;
+          if (auto match =
+                  matchSwiftAttrConsideringInheritance<T>(baseDecl, patterns))
+            return match;
+        }
+      }
+
+      return std::nullopt;
+    }
+
     /// Emit diagnostics for incorrect usage of SWIFT_RETURNS_RETAINED and
     /// SWIFT_RETURNS_UNRETAINED
     void checkBridgingAttrs(const clang::NamedDecl *decl) {
@@ -3686,7 +3711,7 @@ namespace {
           if (const auto *returnPtrTy = retType->getAs<clang::PointerType>()) {
             if (clang::RecordDecl *returnRecordDecl =
                     returnPtrTy->getPointeeType()->getAsRecordDecl()) {
-              if (auto match = matchSwiftAttr<bool>(
+              if (auto match = matchSwiftAttrConsideringInheritance<bool>(
                       returnRecordDecl,
                       {
                           {"returns_retained_by_default", true},

lib/SIL/IR/SILFunctionType.cpp

@@ -1270,6 +1270,31 @@ matchSwiftAttr(const clang::Decl *decl,
   return std::nullopt;
 }
 
+template <typename T>
+std::optional<T> matchSwiftAttrConsideringInheritance(
+    const clang::Decl *decl,
+    llvm::ArrayRef<std::pair<llvm::StringRef, T>> patterns) {
+  if (!decl)
+    return std::nullopt;
+
+  if (auto match = matchSwiftAttr<T>(decl, patterns))
+    return match;
+
+  if (const auto *recordDecl = llvm::dyn_cast<clang::CXXRecordDecl>(decl)) {
+    for (const auto &baseSpecifier : recordDecl->bases()) {
+      const clang::CXXRecordDecl *baseDecl =
+          baseSpecifier.getType()->getAsCXXRecordDecl();
+      if (!baseDecl)
+        continue;
+      if (auto match =
+              matchSwiftAttrConsideringInheritance<T>(baseDecl, patterns))
+        return match;
+    }
+  }
+
+  return std::nullopt;
+}
+
 class Conventions {
   ConventionsKind kind;
 
@@ -1393,7 +1418,7 @@ class Conventions {
             llvm::dyn_cast<clang::PointerType>(desugaredReturnTy)) {
       if (clang::RecordDecl *clangRecordDecl =
               ptrType->getPointeeType()->getAsRecordDecl())
-        return matchSwiftAttr<ResultConvention>(
+        return matchSwiftAttrConsideringInheritance<ResultConvention>(
             clangRecordDecl,
             {{"returns_unretained_by_default", ResultConvention::Unowned},
              {"returns_retained_by_default", ResultConvention::Owned}});

test/Interop/Cxx/foreign-reference/Inputs/cxx-functions-and-methods-returning-frt.h

@@ -430,4 +430,72 @@ void drelease(
     DefaultOwnershipSuppressUnannotatedAPIWarning::RefTypeDefaultRetained *v) {
 };
 
+namespace DefaultOwnershipInheritance {
+
+struct __attribute__((swift_attr("import_reference")))
+__attribute__((swift_attr("retain:baseRetain")))
+__attribute__((swift_attr("release:baseRelease")))
+__attribute__((swift_attr("returns_retained_by_default"))) BaseType {};
+
+struct __attribute__((swift_attr("import_reference")))
+__attribute__((swift_attr("retain:derivedRetain")))
+__attribute__((swift_attr("release:derivedRelease"))) DerivedType
+    : public BaseType {};
+
+struct __attribute__((swift_attr("import_reference")))
+__attribute__((swift_attr("retain:derivedRetain2")))
+__attribute__((swift_attr("release:derivedRelease2"))) DerivedType2
+    : public DerivedType {};
+
+struct __attribute__((swift_attr("import_reference")))
+__attribute__((swift_attr("retain:derivedRetain3")))
+__attribute__((swift_attr("release:derivedRelease3"))) __attribute__((
+    swift_attr("returns_unretained_by_default"))) DerivedTypeOverrideDefault
+    : public DerivedType {};
+
+BaseType *returnBaseType() { return new BaseType(); }
+DerivedType *returnDerivedType() { return new DerivedType(); }
+DerivedType2 *returnDerivedType2() { return new DerivedType2(); }
+DerivedTypeOverrideDefault *returnDerivedTypeOverrideDefault() {
+  return new DerivedTypeOverrideDefault();
+}
+
+struct __attribute__((swift_attr("import_reference")))
+__attribute__((swift_attr("retain:bRetain")))
+__attribute__((swift_attr("release:bRelease"))) BaseTypeNonDefault {};
+
+struct __attribute__((swift_attr("import_reference")))
+__attribute__((swift_attr("retain:dRetain")))
+__attribute__((swift_attr("release:dRelease"))) DerivedTypeNonDefault
+    : public BaseTypeNonDefault {};
+
+BaseTypeNonDefault *returnBaseTypeNonDefault() { // expected-warning {{'returnBaseTypeNonDefault' should be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED as it is returning a SWIFT_SHARED_REFERENCE}}
+  return new BaseTypeNonDefault();
+}
+DerivedTypeNonDefault *returnDerivedTypeNonDefault() { // expected-warning {{'returnDerivedTypeNonDefault' should be annotated with either SWIFT_RETURNS_RETAINED or SWIFT_RETURNS_UNRETAINED as it is returning a SWIFT_SHARED_REFERENCE}}
+  return new DerivedTypeNonDefault();
+}
+
+} // namespace DefaultOwnershipInheritance
+
+void baseRetain(DefaultOwnershipInheritance::BaseType *v) {};
+void baseRelease(DefaultOwnershipInheritance::BaseType *v) {};
+
+void derivedRetain(DefaultOwnershipInheritance::DerivedType *v) {};
+void derivedRelease(DefaultOwnershipInheritance::DerivedType *v) {};
+
+void derivedRetain2(DefaultOwnershipInheritance::DerivedType2 *v) {};
+void derivedRelease2(DefaultOwnershipInheritance::DerivedType2 *v) {};
+
+void derivedRetain3(
+    DefaultOwnershipInheritance::DerivedTypeOverrideDefault *v) {};
+void derivedRelease3(
+    DefaultOwnershipInheritance::DerivedTypeOverrideDefault *v) {};
+
+void bRetain(DefaultOwnershipInheritance::BaseTypeNonDefault *v) {};
+void bRelease(DefaultOwnershipInheritance::BaseTypeNonDefault *v) {};
+
+void dRetain(DefaultOwnershipInheritance::DerivedTypeNonDefault *v) {};
+void dRelease(DefaultOwnershipInheritance::DerivedTypeNonDefault *v) {};
+
 SWIFT_END_NULLABILITY_ANNOTATIONS

test/Interop/Cxx/foreign-reference/frt-retained-unretained-attributes-error.swift

@@ -34,3 +34,8 @@ let nonFrt = NonFRTStruct()
 let nonFrtLocalVar1 = global_function_returning_templated_retrun_frt_owned(nonFrt)
 let _ = DefaultOwnershipSuppressUnannotatedAPIWarning.returnRefType()
 let _ = DefaultOwnershipSuppressUnannotatedAPIWarning.returnRefTypeDefaultRetained()
+let _ = DefaultOwnershipInheritance.returnBaseType()
+let _ = DefaultOwnershipInheritance.returnDerivedType()
+let _ = DefaultOwnershipInheritance.returnDerivedType2()
+let _ = DefaultOwnershipInheritance.returnBaseTypeNonDefault()
+let _ = DefaultOwnershipInheritance.returnDerivedTypeNonDefault()

test/Interop/Cxx/foreign-reference/frt-retained-unretained-attributes.swift

@@ -266,4 +266,22 @@ func testDefaultOwnershipAnnotation() {
 
   let _ = FunctionAnnotationHasPrecedence.returnRefTypeDefaultRetainedAnnotatedUnRetained()
   // CHECK: function_ref {{.*}}returnRefTypeDefaultRetainedAnnotatedUnRetained{{.*}} : $@convention(c) () -> FunctionAnnotationHasPrecedence.RefTypeDefaultRetained
+
+  let _ = DefaultOwnershipInheritance.returnBaseType()
+  // CHECK: function_ref {{.*}}returnBaseType{{.*}} : $@convention(c) () -> @owned DefaultOwnershipInheritance.BaseType
+
+  let _ = DefaultOwnershipInheritance.returnDerivedType()
+  // CHECK: function_ref {{.*}}returnDerivedType{{.*}} : $@convention(c) () -> @owned DefaultOwnershipInheritance.DerivedType
+
+  let _ = DefaultOwnershipInheritance.returnDerivedType2()
+  // CHECK: function_ref {{.*}}returnDerivedType2{{.*}} : $@convention(c) () -> @owned DefaultOwnershipInheritance.DerivedType2
+
+  let _ = DefaultOwnershipInheritance.returnDerivedTypeOverrideDefault()
+  // CHECK: function_ref {{.*}}returnDerivedTypeOverrideDefault{{.*}} : $@convention(c) () -> DefaultOwnershipInheritance.DerivedTypeOverrideDefault
+
+  let _ = DefaultOwnershipInheritance.returnBaseTypeNonDefault()
+  // CHECK: function_ref {{.*}}returnBaseTypeNonDefault{{.*}} : $@convention(c) () -> DefaultOwnershipInheritance.BaseTypeNonDefault
+
+  let _ = DefaultOwnershipInheritance.returnDerivedTypeNonDefault()
+  // CHECK: function_ref {{.*}}returnDerivedTypeNonDefault{{.*}} : $@convention(c) () -> DefaultOwnershipInheritance.DerivedTypeNonDefault
 }