[cxx-interop] Do not import template type arguments

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();

lib/ClangImporter/ImportDecl.cpp

@@ -2299,34 +2299,6 @@ namespace {
         }
       }
 
-      // We have to do this after populating ImportedDecls to avoid importing
-      // the same decl multiple times. Also after we imported the bases.
-      if (const auto *ctsd =
-              dyn_cast<clang::ClassTemplateSpecializationDecl>(decl)) {
-        for (auto arg : ctsd->getTemplateArgs().asArray()) {
-          llvm::SmallVector<clang::TemplateArgument, 1> nonPackArgs;
-          if (arg.getKind() == clang::TemplateArgument::Pack) {
-            auto pack = arg.getPackAsArray();
-            nonPackArgs.assign(pack.begin(), pack.end());
-          } else {
-            nonPackArgs.push_back(arg);
-          }
-          for (auto realArg : nonPackArgs) {
-            if (realArg.getKind() != clang::TemplateArgument::Type)
-              continue;
-            auto SwiftType = Impl.importTypeIgnoreIUO(
-                realArg.getAsType(), ImportTypeKind::Abstract,
-                [](Diagnostic &&diag) {}, false, Bridgeability::None,
-                ImportTypeAttrs());
-            if (SwiftType && SwiftType->isUnsafe()) {
-              auto attr = new (Impl.SwiftContext) UnsafeAttr(/*implicit=*/true);
-              result->getAttrs().add(attr);
-              break;
-            }
-          }
-        }
-      }
-
       bool isNonEscapable = false;
       if (evaluateOrDefault(
               Impl.SwiftContext.evaluator,

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}}
+}

test/Interop/Cxx/templates/sfinae-template-type-arguments.swift

@@ -0,0 +1,53 @@
+// RUN: split-file %s %t
+// RUN: %target-clang -c -o /dev/null -Xclang -verify -I %t/Inputs %t/cxx.cpp
+// RUN: %target-swift-frontend -typecheck -verify -cxx-interoperability-mode=default -I %t%{fs-sep}Inputs -verify-additional-file %t%{fs-sep}Inputs%{fs-sep}CxxHeader.h %t%{fs-sep}main.swift
+// RUN: %target-swift-ide-test -print-module -module-to-print=CxxModule -I %t/Inputs -source-filename=x -cxx-interoperability-mode=default | %FileCheck %t/Inputs/CxxHeader.h
+
+//--- Inputs/module.modulemap
+module CxxModule {
+    requires cplusplus
+    header "CxxHeader.h"
+}
+
+//--- Inputs/CxxHeader.h
+#pragma once
+
+struct Empty {};
+template <typename T> struct MissingMember { typename T::Missing member; };
+using SUB = MissingMember<Empty>;
+
+struct Incomplete;
+template <typename T> struct IncompleteField { T member; };
+using INC = IncompleteField<Incomplete>;
+
+template <typename S = SUB, typename I = INC> struct DefaultedTemplateArgs {};
+using DefaultedTemplateArgsInst = DefaultedTemplateArgs<>;
+
+// CHECK: struct DefaultedTemplateArgs<MissingMember<Empty>, IncompleteField<Incomplete>> {
+// CHECK: }
+// CHECK: typealias DefaultedTemplateArgsInst = DefaultedTemplateArgs<MissingMember<Empty>, IncompleteField<Incomplete>>
+
+template <typename S, typename I> struct ThrowsAwayTemplateArgs {};
+using ThrowsAwayTemplateArgsInst = ThrowsAwayTemplateArgs<SUB, INC>;
+
+// CHECK: struct ThrowsAwayTemplateArgs<MissingMember<Empty>, IncompleteField<Incomplete>> {
+// CHECK: }
+// CHECK: typealias ThrowsAwayTemplateArgsInst = ThrowsAwayTemplateArgs<MissingMember<Empty>, IncompleteField<Incomplete>>
+
+//--- cxx.cpp
+// expected-no-diagnostics
+#include <CxxHeader.h>
+void make(void) {
+  DefaultedTemplateArgs<>   dta{};
+  DefaultedTemplateArgsInst dtai{};
+
+  ThrowsAwayTemplateArgs<SUB, INC>  tata{};
+  ThrowsAwayTemplateArgsInst        tatai{};
+}
+
+//--- main.swift
+import CxxModule
+func make() {
+  let _: DefaultedTemplateArgsInst = .init()
+  let _: ThrowsAwayTemplateArgsInst = .init()
+}