[cxx-interop] Add diagnostics and validation for retain/release function; require all foreign reference types to provide retain release functions or be immortal.

Author: zoecarver

include/swift/AST/DiagnosticsClangImporter.def

@@ -170,6 +170,27 @@ NOTE(dont_use_iterator_api, none, "C++ method '%0' that returns an unsafe "
                                   "APIs instead",
      (StringRef))
 
+ERROR(reference_type_must_have_retain_attr,none,
+      "reference type '%0' must have 'retain:' swift attribute.", (StringRef))
+ERROR(reference_type_must_have_release_attr,none,
+      "reference type '%0' must have 'release:' swift attribute.", (StringRef))
+ERROR(foreign_reference_types_cannot_find_retain,none,
+      "cannot find retain function '%0' for reference type '%1'.", (StringRef, StringRef))
+ERROR(foreign_reference_types_cannot_find_release,none,
+      "cannot find release function '%0' for reference type '%1'.", (StringRef, StringRef))
+ERROR(too_many_reference_type_retain_operations,none,
+      "too many functions with name '%0'. There must be exactly one retain "
+      "function for reference type '%1'.", (StringRef, StringRef))
+ERROR(too_many_reference_type_release_operations,none,
+      "too many functions with name '%0'. There must be exactly one release "
+      "function for reference type '%1'.", (StringRef, StringRef))
+ERROR(foreign_reference_types_invalid_retain,none,
+      "specified retain function '%0' is invalid. Retain must have exactly one "
+      "argument of type '%1'", (StringRef, StringRef))
+ERROR(foreign_reference_types_invalid_release,none,
+      "specified release function '%0' is invalid. Release must have exactly "
+      "one argument of type '%1'", (StringRef, StringRef))
+
 NOTE(unsupported_builtin_type, none, "built-in type '%0' not supported", (StringRef))
 NOTE(record_field_not_imported, none, "field %0 not imported", (const clang::NamedDecl*))
 NOTE(invoked_func_not_imported, none, "function %0 not imported", (const clang::NamedDecl*))

include/swift/AST/DiagnosticsIRGen.def

@@ -59,19 +59,5 @@ ERROR(temporary_allocation_alignment_not_positive,none,
 ERROR(temporary_allocation_alignment_not_power_of_2,none,
       "alignment value must be a power of two", ())
 
-ERROR(foreign_reference_types_cannot_find_retain,none,
-      "cannot find retain function '%0'.", (StringRef))
-
-ERROR(foreign_reference_types_cannot_find_release,none,
-      "cannot find release function '%0'.", (StringRef))
-
-ERROR(foreign_reference_types_invalid_retain,none,
-      "specified retain function '%0' is invalid. Retain must have exactly one "
-      "argument of type '%1'", (StringRef, StringRef))
-
-ERROR(foreign_reference_types_invalid_release,none,
-      "specified release function '%0' is invalid. Release must have exactly "
-      "one argument of type '%1'", (StringRef, StringRef))
-
 #define UNDEFINE_DIAGNOSTIC_MACROS
 #include "DefineDiagnosticMacros.h"

include/swift/ClangImporter/ClangImporterRequests.h

@@ -285,6 +285,67 @@ class IsSafeUseOfCxxDecl
   bool evaluate(Evaluator &evaluator, SafeUseOfCxxDeclDescriptor desc) const;
 };
 
+enum class CustomRefCountingOperationKind { retain, release };
+
+struct CustomRefCountingOperationDescriptor final {
+  const ClassDecl *decl;
+  CustomRefCountingOperationKind kind;
+
+  CustomRefCountingOperationDescriptor(const ClassDecl *decl,
+                                       CustomRefCountingOperationKind kind)
+      : decl(decl), kind(kind) {}
+
+  friend llvm::hash_code hash_value(const CustomRefCountingOperationDescriptor &desc) {
+    return llvm::hash_combine(desc.decl, desc.kind);
+  }
+
+  friend bool operator==(const CustomRefCountingOperationDescriptor &lhs,
+                         const CustomRefCountingOperationDescriptor &rhs) {
+    return lhs.decl == rhs.decl && lhs.kind == rhs.kind;
+  }
+
+  friend bool operator!=(const CustomRefCountingOperationDescriptor &lhs,
+                         const CustomRefCountingOperationDescriptor &rhs) {
+    return !(lhs == rhs);
+  }
+};
+
+void simple_display(llvm::raw_ostream &out, CustomRefCountingOperationDescriptor desc);
+SourceLoc extractNearestSourceLoc(CustomRefCountingOperationDescriptor desc);
+
+struct CustomRefCountingOperationResult {
+  enum CustomRefCountingOperationResultKind {
+    noAttribute, immortal, notFound, tooManyFound, foundOperation
+  };
+
+  CustomRefCountingOperationResultKind kind;
+  ValueDecl *operation;
+  std::string name;
+};
+
+class CustomRefCountingOperation
+    : public SimpleRequest<CustomRefCountingOperation,
+                           CustomRefCountingOperationResult(
+                               CustomRefCountingOperationDescriptor),
+                           RequestFlags::Cached> {
+public:
+  using SimpleRequest::SimpleRequest;
+
+  // Caching
+  bool isCached() const { return true; }
+
+  // Source location
+  SourceLoc getNearestLoc() const { return SourceLoc(); };
+
+private:
+  friend SimpleRequest;
+
+  // Evaluation.
+  CustomRefCountingOperationResult evaluate(Evaluator &evaluator,
+                      CustomRefCountingOperationDescriptor desc) const;
+};
+
+
 #define SWIFT_TYPEID_ZONE ClangImporter
 #define SWIFT_TYPEID_HEADER "swift/ClangImporter/ClangImporterTypeIDZone.def"
 #include "swift/Basic/DefineTypeIDZone.h"

include/swift/ClangImporter/ClangImporterTypeIDZone.def

@@ -30,3 +30,6 @@ SWIFT_REQUEST(ClangImporter, CxxRecordSemantics,
 SWIFT_REQUEST(ClangImporter, IsSafeUseOfCxxDecl,
               bool(SafeUseOfCxxRecordDescriptor), Cached,
               NoLocationInfo)
+SWIFT_REQUEST(ClangImporter, CustomRefCountingOperation,
+              CustomRefCountingOperationResult(CustomRefCountingOperationDescriptor), Cached,
+              NoLocationInfo)

lib/AST/Decl.cpp

@@ -49,6 +49,7 @@
 #include "swift/AST/SwiftNameTranslation.h"
 #include "swift/Basic/Defer.h"
 #include "swift/ClangImporter/ClangModule.h"
+#include "swift/ClangImporter/ClangImporterRequests.h"
 #include "swift/Parse/Lexer.h" // FIXME: Bad dependency
 #include "clang/Lex/MacroInfo.h"
 #include "llvm/ADT/SmallPtrSet.h"
@@ -5228,25 +5229,9 @@ bool ClassDecl::isForeignReferenceType() const {
 }
 
 bool ClassDecl::hasRefCountingAnnotations() const {
-  auto decl = dyn_cast_or_null<clang::RecordDecl>(getClangDecl());
-  if (!decl)
-    return false;
-
-  bool hasRetain = decl->hasAttrs() && llvm::any_of(decl->getAttrs(),
-         [](auto *attr) {
-           if (auto swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr))
-             return swiftAttr->getAttribute().startswith("retain:");
-           return false;
-         });
-
-  bool hasRelease = decl->hasAttrs() && llvm::any_of(decl->getAttrs(),
-         [](auto *attr) {
-           if (auto swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr))
-             return swiftAttr->getAttribute().startswith("release:");
-           return false;
-         });
-
-  return hasRetain && hasRelease;
+  return evaluateOrDefault(
+      getASTContext().evaluator,
+      CustomRefCountingOperation({this, CustomRefCountingOperationKind::release}), {}).kind != CustomRefCountingOperationResult::immortal;
 }
 
 ReferenceCounting ClassDecl::getObjectModel() const {

lib/ClangImporter/ClangImporter.cpp

@@ -6073,4 +6073,56 @@ void swift::simple_display(llvm::raw_ostream &out,
 
 SourceLoc swift::extractNearestSourceLoc(SafeUseOfCxxDeclDescriptor desc) {
   return SourceLoc();
+}
+
+CustomRefCountingOperationResult CustomRefCountingOperation::evaluate(
+    Evaluator &evaluator, CustomRefCountingOperationDescriptor desc) const {
+  auto swiftDecl = desc.decl;
+  auto operation = desc.kind;
+  auto &ctx = swiftDecl->getASTContext();
+
+  std::string operationStr = operation == CustomRefCountingOperationKind::retain ? "retain:" : "release:";
+
+  auto decl = cast<clang::RecordDecl>(swiftDecl->getClangDecl());
+  if (!decl->hasAttrs())
+    return {CustomRefCountingOperationResult::noAttribute, nullptr, ""};
+
+  auto retainFnAttr = llvm::find_if(decl->getAttrs(),
+                                    [&operationStr](auto *attr) {
+                                      if (auto swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr))
+                                        return swiftAttr->getAttribute().startswith(operationStr);
+                                      return false;
+                                    });
+  if (retainFnAttr == decl->getAttrs().end()) {
+    return {CustomRefCountingOperationResult::noAttribute, nullptr, ""};
+  }
+
+  auto name = cast<clang::SwiftAttrAttr>(*retainFnAttr)->getAttribute().drop_front(StringRef(operationStr).size()).str();
+
+  if (name == "immortal")
+    return {CustomRefCountingOperationResult::immortal, nullptr, name};
+
+  llvm::SmallVector<ValueDecl *, 1> results;
+  ctx.lookupInModule(swiftDecl->getParentModule(), name, results);
+
+  if (results.size() == 1)
+    return {CustomRefCountingOperationResult::foundOperation, results.front(), name};
+
+  if (results.empty())
+    return {CustomRefCountingOperationResult::notFound, nullptr, name};
+
+  return {CustomRefCountingOperationResult::tooManyFound, nullptr, name};
+}
+
+void swift::simple_display(llvm::raw_ostream &out,
+                           CustomRefCountingOperationDescriptor desc) {
+  out << "Finding custom (foreign reference) reference counting operation '"
+      << (desc.kind == CustomRefCountingOperationKind::retain ? "retain" : "release")
+      << "for '"
+      << desc.decl->getNameStr()
+      << "'.\n";
+}
+
+SourceLoc swift::extractNearestSourceLoc(CustomRefCountingOperationDescriptor desc) {
+  return SourceLoc();
 }

lib/ClangImporter/ImportDecl.cpp

@@ -2424,6 +2424,70 @@ namespace {
       return result;
     }
 
+    void validateForeignReferenceType(const clang::CXXRecordDecl *decl,
+                                      ClassDecl *classDecl) {
+      auto isValidOperation = [&](ValueDecl *operation) -> bool {
+        auto operationFn = dyn_cast<FuncDecl>(operation);
+        if (!operationFn)
+          return false;
+
+        if (!operationFn->getResultInterfaceType()->isVoid())
+          return false;
+
+        if (operationFn->getParameters()->size() != 1)
+          return false;
+
+        if (operationFn->getParameters()->get(0)->getInterfaceType()->isEqual(classDecl->getInterfaceType()))
+          return false;
+
+        return true;
+      };
+
+      auto retainOperation = evaluateOrDefault(
+          Impl.SwiftContext.evaluator,
+          CustomRefCountingOperation({classDecl, CustomRefCountingOperationKind::retain}), {});
+      if (retainOperation.kind == CustomRefCountingOperationResult::noAttribute) {
+        HeaderLoc loc(decl->getLocation());
+        Impl.diagnose(loc, diag::reference_type_must_have_retain_attr, decl->getNameAsString());
+      } else if (retainOperation.kind == CustomRefCountingOperationResult::notFound) {
+        HeaderLoc loc(decl->getLocation());
+        Impl.diagnose(loc, diag::foreign_reference_types_cannot_find_retain, retainOperation.name, decl->getNameAsString());
+      } else if (retainOperation.kind == CustomRefCountingOperationResult::tooManyFound) {
+        HeaderLoc loc(decl->getLocation());
+        Impl.diagnose(loc, diag::too_many_reference_type_retain_operations, retainOperation.name, decl->getNameAsString());
+      } else if (retainOperation.kind == CustomRefCountingOperationResult::foundOperation) {
+        if (!isValidOperation(retainOperation.operation)) {
+          HeaderLoc loc(decl->getLocation());
+          Impl.diagnose(loc, diag::foreign_reference_types_invalid_retain, retainOperation.name, decl->getNameAsString());
+        }
+      } else {
+        // Nothing to do.
+        assert(retainOperation.kind == CustomRefCountingOperationResult::immortal);
+      }
+
+      auto releaseOperation = evaluateOrDefault(
+          Impl.SwiftContext.evaluator,
+          CustomRefCountingOperation({classDecl, CustomRefCountingOperationKind::release}), {});
+      if (releaseOperation.kind == CustomRefCountingOperationResult::noAttribute) {
+        HeaderLoc loc(decl->getLocation());
+        Impl.diagnose(loc, diag::reference_type_must_have_release_attr, decl->getNameAsString());
+      } else if (releaseOperation.kind == CustomRefCountingOperationResult::notFound) {
+        HeaderLoc loc(decl->getLocation());
+        Impl.diagnose(loc, diag::foreign_reference_types_cannot_find_release, releaseOperation.name, decl->getNameAsString());
+      } else if (releaseOperation.kind == CustomRefCountingOperationResult::tooManyFound) {
+        HeaderLoc loc(decl->getLocation());
+        Impl.diagnose(loc, diag::too_many_reference_type_release_operations, releaseOperation.name, decl->getNameAsString());
+      } else if (releaseOperation.kind == CustomRefCountingOperationResult::foundOperation) {
+        if (!isValidOperation(releaseOperation.operation)) {
+          HeaderLoc loc(decl->getLocation());
+          Impl.diagnose(loc, diag::foreign_reference_types_invalid_release, releaseOperation.name, decl->getNameAsString());
+        }
+      } else {
+        // Nothing to do.
+        assert(releaseOperation.kind == CustomRefCountingOperationResult::immortal);
+      }
+    }
+
     Decl *VisitCXXRecordDecl(const clang::CXXRecordDecl *decl) {
       // This can be called from lldb without C++ interop being enabled: There
       // may be C++ declarations in imported modules, but the interface for
@@ -2504,6 +2568,9 @@ namespace {
 
       auto result = VisitRecordDecl(decl);
 
+      if (auto classDecl = dyn_cast_or_null<ClassDecl>(result))
+        validateForeignReferenceType(decl, classDecl);
+
       // If this module is declared as a C++ module, try to synthesize
       // conformances to Swift protocols from the Cxx module.
       auto clangModule = decl->getOwningModule();

lib/IRGen/ClassTypeInfo.h

@@ -20,6 +20,8 @@
 #include "ClassLayout.h"
 #include "HeapTypeInfo.h"
 
+#include "swift/ClangImporter/ClangImporterRequests.h"
+
 namespace swift {
 namespace irgen {
 
@@ -62,8 +64,9 @@ class ClassTypeInfo : public HeapTypeInfo<ClassTypeInfo> {
   void emitScalarRelease(IRGenFunction &IGF, llvm::Value *value,
                          Atomicity atomicity) const override {
     if (getReferenceCounting() == ReferenceCounting::Custom) {
-      auto releaseFn = findForeignReferenceTypeRefCountingOperation(getClass(),
-          ForeignReferenceTypeRefCountingOperation::release);
+      auto releaseFn = evaluateOrDefault(
+                           getClass()->getASTContext().evaluator,
+                           CustomRefCountingOperation({getClass(), CustomRefCountingOperationKind::release}), {}).operation;
       IGF.emitForeignReferenceTypeLifetimeOperation(releaseFn, value);
       return;
     }
@@ -74,9 +77,10 @@ class ClassTypeInfo : public HeapTypeInfo<ClassTypeInfo> {
   void emitScalarRetain(IRGenFunction &IGF, llvm::Value *value,
                         Atomicity atomicity) const override {
     if (getReferenceCounting() == ReferenceCounting::Custom) {
-      auto releaseFn = findForeignReferenceTypeRefCountingOperation(getClass(),
-          ForeignReferenceTypeRefCountingOperation::retain);
-      IGF.emitForeignReferenceTypeLifetimeOperation(releaseFn, value);
+      auto retainFn = evaluateOrDefault(
+                          getClass()->getASTContext().evaluator,
+                          CustomRefCountingOperation({getClass(), CustomRefCountingOperationKind::retain}), {}).operation;
+      IGF.emitForeignReferenceTypeLifetimeOperation(retainFn, value);
       return;
     }
 
@@ -89,9 +93,10 @@ class ClassTypeInfo : public HeapTypeInfo<ClassTypeInfo> {
                     Atomicity atomicity) const override {
     if (getReferenceCounting() == ReferenceCounting::Custom) {
       llvm::Value *value = e.claimNext();
-      auto releaseFn = findForeignReferenceTypeRefCountingOperation(getClass(),
-          ForeignReferenceTypeRefCountingOperation::retain);
-      IGF.emitForeignReferenceTypeLifetimeOperation(releaseFn, value);
+      auto retainFn = evaluateOrDefault(
+                           getClass()->getASTContext().evaluator,
+                           CustomRefCountingOperation({getClass(), CustomRefCountingOperationKind::retain}), {}).operation;
+      IGF.emitForeignReferenceTypeLifetimeOperation(retainFn, value);
       return;
     }
 
@@ -102,8 +107,9 @@ class ClassTypeInfo : public HeapTypeInfo<ClassTypeInfo> {
                      Atomicity atomicity) const override {
     if (getReferenceCounting() == ReferenceCounting::Custom) {
       llvm::Value *value = e.claimNext();
-      auto releaseFn = findForeignReferenceTypeRefCountingOperation(getClass(),
-          ForeignReferenceTypeRefCountingOperation::release);
+      auto releaseFn = evaluateOrDefault(
+                           getClass()->getASTContext().evaluator,
+                           CustomRefCountingOperation({getClass(), CustomRefCountingOperationKind::release}), {}).operation;
       IGF.emitForeignReferenceTypeLifetimeOperation(releaseFn, value);
       return;
     }

lib/IRGen/HeapTypeInfo.h

@@ -31,33 +31,6 @@
 namespace swift {
 namespace irgen {
 
-enum class ForeignReferenceTypeRefCountingOperation { retain, release };
-
-static ValueDecl *findForeignReferenceTypeRefCountingOperation(ClassDecl *swiftDecl,
-                                                               ForeignReferenceTypeRefCountingOperation operation) {
-  std::string operationStr = operation == ForeignReferenceTypeRefCountingOperation::retain ? "retain:" : "release:";
-
-  auto decl = cast<clang::RecordDecl>(swiftDecl->getClangDecl());
-  assert(decl->hasAttrs());
-
-  auto retainFnAttr = llvm::find_if(decl->getAttrs(),
-                                    [&operationStr](auto *attr) {
-                                      if (auto swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr))
-                                        return swiftAttr->getAttribute().startswith(operationStr);
-                                      return false;
-                                    });
-  assert(retainFnAttr != decl->getAttrs().end());
-
-  auto name = cast<clang::SwiftAttrAttr>(*retainFnAttr)->getAttribute().drop_front(StringRef(operationStr).size());
-  auto &ctx = swiftDecl->getASTContext();
-
-  llvm::SmallVector<ValueDecl *, 1> results;
-  ctx.lookupInModule(swiftDecl->getParentModule(), name, results);
-
-  assert(results.size() == 1);
-  return results.front();
-}
-
 /// \group HeapTypeInfo
 
 /// The kind of 'isa' encoding a heap object uses to reference its heap