[cxx-interop] Check template argument safety in ClangDeclExplicitSafety

Checking this upon import may overlook annotations that explicitly
mark a type as safe (or unsafe). Instead, we consolidate this logic in
the ClangDeclExplicitSafety request.

rdar://163196609

Author: j-hui

lib/ClangImporter/ClangImporter.cpp

@@ -8777,14 +8777,14 @@ static bool hasUnsafeType(Evaluator &evaluator, clang::QualType clangType) {
     // 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 (auto *recordDecl = pointeeType->getAsRecordDecl()) {
       if (hasImportAsRefAttr(recordDecl))
         return false;
     }
-    
+
     // All other pointers are considered unsafe.
     return true;
   }
@@ -8793,19 +8793,25 @@ static bool hasUnsafeType(Evaluator &evaluator, clang::QualType clangType) {
   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;        
-    }
+    auto req = ClangDeclExplicitSafety({recordDecl, false});
+    if (evaluator.hasActiveRequest(req))
+      // Normally, using hasActiveRequest() to avoid cycles is an anti-pattern
+      // because cycles should be treated as errors. However, cycles are allowed
+      // in C++ template, e.g.:
+      //
+      //   template <typename>   class Foo { ... }; // throws away template arg
+      //   template <typename T> class Bar : Foo<Bar<T>> { ... };
+      //
+      // A common use case of cyclic templates is the CRTP pattern.
+      //
+      // We need to avoid request cycles, so if there is already an active
+      // request for this type, just assume it is not explicitly unsafe for now
+      // (i.e., as if it were unspecified).
+      return false;
+    return evaluateOrDefault(evaluator, req, ExplicitSafety::Unspecified) ==
+           ExplicitSafety::Unsafe;
   }
-    
+
   // Everything else is safe.
   return false;
 }
@@ -8845,6 +8851,29 @@ ClangDeclExplicitSafety::evaluate(Evaluator &evaluator,
           ClangTypeEscapability({recordDecl->getTypeForDecl(), nullptr}),
           CxxEscapability::Unknown) != CxxEscapability::Unknown)
     return ExplicitSafety::Safe;
+
+  // A template class is unsafe if any of its type arguments are unsafe.
+  // Note that this does not rely on the record being defined.
+  if (const auto *ctsd =
+          dyn_cast<clang::ClassTemplateSpecializationDecl>(recordDecl)) {
+    for (auto arg : ctsd->getTemplateArgs().asArray()) {
+      switch (arg.getKind()) {
+      case clang::TemplateArgument::Type:
+        if (hasUnsafeType(evaluator, arg.getAsType()))
+          return ExplicitSafety::Unsafe;
+        break;
+      case clang::TemplateArgument::Pack:
+        for (auto pkArg : arg.getPackAsArray()) {
+          if (pkArg.getKind() == clang::TemplateArgument::Type &&
+              hasUnsafeType(evaluator, pkArg.getAsType()))
+            return ExplicitSafety::Unsafe;
+        }
+        break;
+      default:
+        continue;
+      }
+    }
+  }
 
   // If we don't have a definition, leave it unspecified.
   recordDecl = recordDecl->getDefinition();

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

@@ -83,6 +83,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>;
+
 struct HoldsShared {
   SharedObject* obj;
 
@@ -184,3 +195,21 @@ func useSharedReference(frt: DerivedFromSharedObject, h: HoldsShared) {
   let _ = frt
   let _ = h.getObj()
 }
+
+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}}
+}