[cxx-interop] Mark class templates with unsafe type arguments as unsafe

Doing so preserves ClangImporter's behavior first introduced in
f12b48a, but do so without relying on
importing the type argument (since that can lead to template
over-instantiation).

This patch also promotes the logic of hasUnsafeType() to a standalone
SimpleRequest, to provide a QualType-typed entry point for evaluating
the safety of a Clang entity.

Author: j-hui

include/swift/ClangImporter/ClangImporterRequests.h

@@ -624,6 +624,51 @@ class CxxValueSemantics
 void simple_display(llvm::raw_ostream &out, CxxValueSemanticsDescriptor desc);
 SourceLoc extractNearestSourceLoc(CxxValueSemanticsDescriptor desc);
 
+struct ClangTypeExplicitSafetyDescriptor final {
+  clang::QualType type;
+
+  ClangTypeExplicitSafetyDescriptor(clang::QualType type) : type(type) {}
+
+  friend llvm::hash_code
+  hash_value(const ClangTypeExplicitSafetyDescriptor &desc) {
+    return llvm::hash_combine(desc.type.getTypePtrOrNull());
+  }
+
+  friend bool operator==(const ClangTypeExplicitSafetyDescriptor &lhs,
+                         const ClangTypeExplicitSafetyDescriptor &rhs) {
+    return lhs.type == rhs.type;
+  }
+
+  friend bool operator!=(const ClangTypeExplicitSafetyDescriptor &lhs,
+                         const ClangTypeExplicitSafetyDescriptor &rhs) {
+    return !(lhs == rhs);
+  }
+};
+
+void simple_display(llvm::raw_ostream &out,
+                    ClangTypeExplicitSafetyDescriptor desc);
+SourceLoc extractNearestSourceLoc(ClangTypeExplicitSafetyDescriptor desc);
+
+/// Determine the safety of the given Clang declaration.
+class ClangTypeExplicitSafety
+    : public SimpleRequest<ClangTypeExplicitSafety,
+                           ExplicitSafety(ClangTypeExplicitSafetyDescriptor),
+                           RequestFlags::Cached> {
+public:
+  using SimpleRequest::SimpleRequest;
+
+  // Source location
+  SourceLoc getNearestLoc() const { return SourceLoc(); };
+  bool isCached() const { return true; }
+
+private:
+  friend SimpleRequest;
+
+  // Evaluation.
+  ExplicitSafety evaluate(Evaluator &evaluator,
+                          ClangTypeExplicitSafetyDescriptor desc) const;
+};
+
 struct CxxDeclExplicitSafetyDescriptor final {
   const clang::Decl *decl;
   bool isClass;

include/swift/ClangImporter/ClangImporterTypeIDZone.def

@@ -48,6 +48,9 @@ SWIFT_REQUEST(ClangImporter, ClangTypeEscapability,
 SWIFT_REQUEST(ClangImporter, CxxValueSemantics,
               CxxValueSemanticsKind(CxxValueSemanticsDescriptor), Cached,
               NoLocationInfo)
+SWIFT_REQUEST(ClangImporter, ClangTypeExplicitSafety,
+              ExplicitSafety(ClangTypeExplicitSafetyDescriptor), Cached,
+              NoLocationInfo)
 SWIFT_REQUEST(ClangImporter, ClangDeclExplicitSafety,
               ExplicitSafety(CxxDeclExplicitSafetyDescriptor), Cached,
               NoLocationInfo)

lib/ClangImporter/ClangImporter.cpp

@@ -8640,6 +8640,51 @@ SourceLoc swift::extractNearestSourceLoc(SafeUseOfCxxDeclDescriptor desc) {
   return SourceLoc();
 }
 
+void swift::simple_display(llvm::raw_ostream &out,
+                           ClangTypeExplicitSafetyDescriptor desc) {
+  out << "Checking if type '" << desc.type.getAsString()
+      << "' is explicitly safe.\n";
+}
+
+SourceLoc swift::extractNearestSourceLoc(ClangTypeExplicitSafetyDescriptor desc) {
+  return SourceLoc();
+}
+
+ExplicitSafety ClangTypeExplicitSafety::evaluate(
+    Evaluator &evaluator, ClangTypeExplicitSafetyDescriptor desc) const {
+  auto clangType = desc.type;
+
+  // Handle pointers.
+  auto pointeeType = clangType->getPointeeType();
+  if (!pointeeType.isNull()) {
+    // Function pointers are okay.
+    if (pointeeType->isFunctionType())
+      return ExplicitSafety::Safe;
+
+    // Pointers to record types are okay if they come in as foreign reference
+    // types.
+    if (auto *recordDecl = pointeeType->getAsRecordDecl()) {
+      if (hasImportAsRefAttr(recordDecl))
+        return ExplicitSafety::Safe;
+    }
+
+    // All other pointers are considered unsafe.
+    return ExplicitSafety::Unsafe;
+  }
+
+  // Handle records recursively.
+  if (auto recordDecl = clangType->getAsTagDecl()) {
+    // If we reached this point the types is not imported as a shared reference,
+    // so we don't need to check the bases whether they are shared references.
+    return evaluateOrDefault(evaluator,
+                             ClangDeclExplicitSafety({recordDecl, false}),
+                             ExplicitSafety::Unspecified);
+  }
+
+  // Everything else is safe.
+  return ExplicitSafety::Safe;
+}
+
 void swift::simple_display(llvm::raw_ostream &out,
                            CxxDeclExplicitSafetyDescriptor desc) {
   out << "Checking if '";
@@ -8711,43 +8756,11 @@ CustomRefCountingOperationResult CustomRefCountingOperation::evaluate(
 
 /// Check whether the given Clang type involves an unsafe type.
 static bool hasUnsafeType(Evaluator &evaluator, clang::QualType clangType) {
-  // Handle pointers.
-  auto pointeeType = clangType->getPointeeType();
-  if (!pointeeType.isNull()) {
-    // Function pointers are okay.
-    if (pointeeType->isFunctionType())
-      return false;
-    
-    // Pointers to record types are okay if they come in as foreign reference
-    // types.
-    if (auto recordDecl = pointeeType->getAsRecordDecl()) {
-      if (hasImportAsRefAttr(recordDecl))
-        return false;
-    }
-    
-    // All other pointers are considered unsafe.
-    return true;
-  }
 
-  // Handle records recursively.
-  if (auto recordDecl = clangType->getAsTagDecl()) {
-    // If we reached this point the types is not imported as a shared reference,
-    // so we don't need to check the bases whether they are shared references.
-    auto safety = evaluateOrDefault(
-        evaluator, ClangDeclExplicitSafety({recordDecl, false}),
-        ExplicitSafety::Unspecified);
-    switch (safety) {
-      case ExplicitSafety::Unsafe:
-        return true;
-        
-      case ExplicitSafety::Safe:
-      case ExplicitSafety::Unspecified:
-        return false;        
-    }
-  }
-    
-  // Everything else is safe.
-  return false;
+  auto safety =
+      evaluateOrDefault(evaluator, ClangTypeExplicitSafety({clangType}),
+                        ExplicitSafety::Unspecified);
+  return safety == ExplicitSafety::Unsafe;
 }
 
 ExplicitSafety

lib/ClangImporter/ImportDecl.cpp

@@ -68,6 +68,7 @@
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/StmtVisitor.h"
+#include "clang/AST/TemplateBase.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Basic/TargetInfo.h"
@@ -2298,6 +2299,40 @@ namespace {
         }
       }
 
+      if (const auto *ctsd =
+              dyn_cast<clang::ClassTemplateSpecializationDecl>(decl)) {
+        for (auto arg : ctsd->getTemplateArgs().asArray()) {
+          auto done = false;
+          auto checkUnsafe = [&](clang::TemplateArgument tyArg) {
+            if (tyArg.getKind() != clang::TemplateArgument::Type)
+              return;
+
+            auto safety =
+                evaluateOrDefault(Impl.SwiftContext.evaluator,
+                                  ClangTypeExplicitSafety({tyArg.getAsType()}),
+                                  ExplicitSafety::Unspecified);
+
+            if (safety == ExplicitSafety::Unsafe) {
+              result->getAttrs().add(new (Impl.SwiftContext)
+                                         UnsafeAttr(/*implicit=*/true));
+              done = true;
+            }
+          };
+
+          if (arg.getKind() == clang::TemplateArgument::Pack) {
+            for (auto pkArg : arg.getPackAsArray()) {
+              checkUnsafe(pkArg);
+              if (done)
+                break;
+            }
+          } else {
+            checkUnsafe(arg);
+          }
+          if (done)
+            break;
+        }
+      }
+
       bool isNonEscapable = false;
       if (evaluateOrDefault(
               Impl.SwiftContext.evaluator,

test/Interop/Cxx/class/safe-interop-mode.swift

@@ -78,6 +78,17 @@ struct OwnedData {
   void takeSharedObject(SharedObject *) const;
 };
 
+// A class template that throws away its type argument.
+//
+// If this template is instantiated with an unsafe type, it should be considered
+// unsafe even if that type is never used.
+template <typename> struct TTake {};
+
+using TTakeInt = TTake<int>;
+using TTakePtr = TTake<int *>;
+using TTakeSafeTuple = TTake<SafeTuple>;
+using TTakeUnsafeTuple = TTake<UnsafeTuple>;
+
 //--- test.swift
 
 import Test
@@ -167,3 +178,21 @@ func useSharedReference(frt: SharedObject, x: OwnedData) {
 func useSharedReference(frt: DerivedFromSharedObject) {
   let _ = frt
 }
+
+func useTTakeInt(x: TTakeInt) {
+  _ = x
+}
+
+func useTTakePtr(x: TTakePtr) {
+  // expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}}
+  _ = x // expected-note{{reference to parameter 'x' involves unsafe type}}
+}
+
+func useTTakeSafeTuple(x: TTakeSafeTuple) {
+  _ = x
+}
+
+func useTTakeUnsafeTuple(x: TTakeUnsafeTuple) {
+  // expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}}
+  _ = x // expected-note{{reference to parameter 'x' involves unsafe type}}
+}