Merge pull request #84866 from j-hui/do-not-import-template-type-arguments

[cxx-interop] Do not import template type arguments

Author: j-hui

include/swift/ClangImporter/ClangImporterRequests.h

@@ -624,6 +624,53 @@ class CxxValueSemantics
 void simple_display(llvm::raw_ostream &out, CxxValueSemanticsDescriptor desc);
 SourceLoc extractNearestSourceLoc(CxxValueSemanticsDescriptor desc);
 
+struct ClangTypeExplicitSafetyDescriptor final {
+  clang::CanQualType type;
+
+  ClangTypeExplicitSafetyDescriptor(clang::CanQualType type) : type(type) {}
+  ClangTypeExplicitSafetyDescriptor(clang::QualType type)
+      : type(type->getCanonicalTypeUnqualified()) {}
+
+  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) {
+  auto qt = static_cast<clang::QualType>(desc.type);
+  out << "Checking if type '" << qt.getAsString() << "' is explicitly safe.\n";
+}
+
+SourceLoc swift::extractNearestSourceLoc(ClangTypeExplicitSafetyDescriptor desc) {
+  return SourceLoc();
+}
+
+ExplicitSafety ClangTypeExplicitSafety::evaluate(
+    Evaluator &evaluator, ClangTypeExplicitSafetyDescriptor desc) const {
+  auto clangType = static_cast<clang::QualType>(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,49 +8756,16 @@ 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
 ClangDeclExplicitSafety::evaluate(Evaluator &evaluator,
                                   CxxDeclExplicitSafetyDescriptor desc) const {
-  // FIXME: Somewhat duplicative with importAsUnsafe.
   // FIXME: Also similar to hasPointerInSubobjects
   // FIXME: should probably also subsume IsSafeUseOfCxxDecl
 

lib/ClangImporter/ImportDecl.cpp

@@ -69,6 +69,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"
@@ -2299,31 +2300,37 @@ 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;
+          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) {
-            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;
+            for (auto pkArg : arg.getPackAsArray()) {
+              checkUnsafe(pkArg);
+              if (done)
+                break;
             }
+          } else {
+            checkUnsafe(arg);
           }
+          if (done)
+            break;
         }
       }
 

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

@@ -82,6 +82,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
@@ -175,3 +186,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}}
+}

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