Merge pull request #83793 from egorzhdan/egorzhdan/c-frt-validation

[cxx-interop] Validate C foreign reference types

Author: egorzhdan

lib/ClangImporter/ImportDecl.cpp

@@ -2692,6 +2692,25 @@ namespace {
         }
       }
 
+      if (auto classDecl = dyn_cast<ClassDecl>(result)) {
+        validateForeignReferenceType(decl, classDecl);
+
+        auto availability = Impl.SwiftContext.getSwift58Availability();
+        if (!availability.isAlwaysAvailable()) {
+          assert(availability.hasMinimumVersion());
+          auto AvAttr = AvailableAttr::createPlatformVersioned(
+              Impl.SwiftContext, targetPlatform(Impl.SwiftContext.LangOpts),
+              /*Message=*/"", /*Rename=*/"",
+              availability.getRawMinimumVersion(), /*Deprecated=*/{},
+              /*Obsoleted=*/{});
+          classDecl->getAttrs().add(AvAttr);
+        }
+
+        if (cxxRecordDecl && cxxRecordDecl->isEffectivelyFinal())
+          classDecl->getAttrs().add(new (Impl.SwiftContext)
+                                        FinalAttr(/*IsImplicit=*/true));
+      }
+
       // If we need it, add an explicit "deinit" to this type.
       synthesizer.addExplicitDeinitIfRequired(result, decl);
 
@@ -2737,7 +2756,7 @@ namespace {
       }
     }
 
-    void validateForeignReferenceType(const clang::CXXRecordDecl *decl,
+    void validateForeignReferenceType(const clang::RecordDecl *decl,
                                       ClassDecl *classDecl) {
 
       enum class RetainReleaseOperationKind {
@@ -2789,11 +2808,13 @@ namespace {
         // The parameter of the retain/release function should be pointer to the
         // same FRT or a base FRT.
         if (paramDecl != classDecl) {
-          if (const clang::Decl *paramClangDecl = paramDecl->getClangDecl()) {
-            if (const auto *paramTypeDecl =
-                    dyn_cast<clang::CXXRecordDecl>(paramClangDecl)) {
-              if (decl->isDerivedFrom(paramTypeDecl)) {
-                return RetainReleaseOperationKind::valid;
+          if (auto cxxDecl = dyn_cast<clang::CXXRecordDecl>(decl)) {
+            if (const clang::Decl *paramClangDecl = paramDecl->getClangDecl()) {
+              if (const auto *paramTypeDecl =
+                      dyn_cast<clang::CXXRecordDecl>(paramClangDecl)) {
+                if (cxxDecl->isDerivedFrom(paramTypeDecl)) {
+                  return RetainReleaseOperationKind::valid;
+                }
               }
             }
           }
@@ -3140,25 +3161,6 @@ namespace {
 
       validatePrivateFileIDAttributes(decl);
 
-      if (auto classDecl = dyn_cast<ClassDecl>(result)) {
-        validateForeignReferenceType(decl, classDecl);
-
-        auto availability = Impl.SwiftContext.getSwift58Availability();
-        if (!availability.isAlwaysAvailable()) {
-          assert(availability.hasMinimumVersion());
-          auto AvAttr = AvailableAttr::createPlatformVersioned(
-              Impl.SwiftContext, targetPlatform(Impl.SwiftContext.LangOpts),
-              /*Message=*/"", /*Rename=*/"",
-              availability.getRawMinimumVersion(), /*Deprecated=*/{},
-              /*Obsoleted=*/{});
-          classDecl->getAttrs().add(AvAttr);
-        }
-
-        if (decl->isEffectivelyFinal())
-          classDecl->getAttrs().add(new (Impl.SwiftContext)
-                                    FinalAttr(/*IsImplicit=*/true));
-      }
-
       // If this module is declared as a C++ module, try to synthesize
       // conformances to Swift protocols from the Cxx module.
       auto clangModule = Impl.getClangOwningModule(result->getClangNode());

test/Interop/C/struct/Inputs/foreign-reference-invalid.h

@@ -0,0 +1,20 @@
+#include <stdlib.h>
+
+struct __attribute__((swift_attr("import_reference")))
+__attribute__((swift_attr("retain:nonexistent")))
+__attribute__((swift_attr("release:nonexistent"))) NonExistent {
+  int value;
+};
+
+struct __attribute__((swift_attr("import_reference"))) NoRetainRelease {
+  int value;
+};
+
+struct __attribute__((swift_attr("import_reference")))
+__attribute__((swift_attr("retain:badRetain")))
+__attribute__((swift_attr("release:badRelease"))) BadRetainRelease {
+  int value;
+};
+
+float badRetain(struct BadRetainRelease *v);
+void badRelease(struct BadRetainRelease *v, int i);

test/Interop/C/struct/Inputs/module.modulemap

@@ -7,6 +7,10 @@ module ForeignReference {
   header "foreign-reference.h"
 }
 
+module ForeignReferenceInvalid {
+  header "foreign-reference-invalid.h"
+}
+
 module StructAsOptionSet {
   header "struct-as-option-set.h"
 }

test/Interop/C/struct/foreign-reference-invalid-typechecker.swift

@@ -0,0 +1,15 @@
+// RUN: not %target-swift-frontend -typecheck %s -I %S/Inputs -disable-availability-checking -diagnostic-style llvm 2>&1 | %FileCheck %s
+
+import ForeignReferenceInvalid
+
+// CHECK: error: cannot find retain function 'nonexistent' for reference type 'NonExistent'
+// CHECK: error: cannot find release function 'nonexistent' for reference type 'NonExistent'
+public func test(x: NonExistent) { }
+
+// CHECK: error: reference type 'NoRetainRelease' must have 'retain:' Swift attribute
+// CHECK: error: reference type 'NoRetainRelease' must have 'release:' Swift attribute
+public func test(x: NoRetainRelease) { }
+
+// CHECK: error: specified retain function 'badRetain' is invalid; retain function must either return have 'void', the reference count as an integer, or the parameter type
+// CHECK: error: specified release function 'badRelease' is invalid; release function must have exactly one argument of type 'BadRetainRelease'
+public func test(x: BadRetainRelease) { }