From f2771584dff50b1c42faec23c12d5ffa0e5e7994 Mon Sep 17 00:00:00 2001
From: Egor Zhdan <e_zhdan@apple.com>
Date: Fri, 26 May 2023 12:21:16 +0100
Subject: [PATCH] [cxx-interop] Do not treat `std::pair<UnsafeType, T>` as safe

rdar://109529750
---
 lib/ClangImporter/ClangImporter.cpp             | 17 +++++++++++++----
 test/Interop/Cxx/stdlib/Inputs/std-pair.h       | 14 ++++++++++++++
 .../Cxx/stdlib/use-std-pair-typechecker.swift   |  8 ++++++++
 3 files changed, 35 insertions(+), 4 deletions(-)
 create mode 100644 test/Interop/Cxx/stdlib/use-std-pair-typechecker.swift

diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp
index af44d5f46fae2..9cb7fabc715df 100644
--- a/lib/ClangImporter/ClangImporter.cpp
+++ b/lib/ClangImporter/ClangImporter.cpp
@@ -6574,6 +6574,17 @@ static bool hasDestroyTypeOperations(const clang::CXXRecordDecl *decl) {
   return false;
 }
 
+static bool hasCustomCopyOrMoveConstructor(const clang::CXXRecordDecl *decl) {
+  // std::pair and std::tuple might have copy and move constructors, but that
+  // doesn't mean they are safe to use from Swift, e.g. std::pair<UnsafeType, T>
+  if (decl->isInStdNamespace() &&
+      (decl->getName() == "pair" || decl->getName() == "tuple")) {
+    return false;
+  }
+  return decl->hasUserDeclaredCopyConstructor() ||
+         decl->hasUserDeclaredMoveConstructor();
+}
+
 static bool isSwiftClassType(const clang::CXXRecordDecl *decl) {
   // Swift type must be annotated with external_source_symbol attribute.
   auto essAttr = decl->getAttr<clang::ExternalSourceSymbolAttr>();
@@ -6641,8 +6652,7 @@ CxxRecordSemantics::evaluate(Evaluator &evaluator,
     return CxxRecordSemanticsKind::Iterator;
   }
   
-  if (!cxxDecl->hasUserDeclaredCopyConstructor() &&
-      !cxxDecl->hasUserDeclaredMoveConstructor() &&
+  if (!hasCustomCopyOrMoveConstructor(cxxDecl) &&
       hasPointerInSubobjects(cxxDecl)) {
     return CxxRecordSemanticsKind::UnsafePointerMember;
   }
@@ -6730,8 +6740,7 @@ bool IsSafeUseOfCxxDecl::evaluate(Evaluator &evaluator,
           return true;
         }
 
-        if (!cxxRecordReturnType->hasUserDeclaredCopyConstructor() &&
-            !cxxRecordReturnType->hasUserDeclaredMoveConstructor() &&
+        if (!hasCustomCopyOrMoveConstructor(cxxRecordReturnType) &&
             !hasOwnedValueAttr(cxxRecordReturnType) &&
             hasPointerInSubobjects(cxxRecordReturnType)) {
           return false;
diff --git a/test/Interop/Cxx/stdlib/Inputs/std-pair.h b/test/Interop/Cxx/stdlib/Inputs/std-pair.h
index 2e501f416d9cf..890cedf67953e 100644
--- a/test/Interop/Cxx/stdlib/Inputs/std-pair.h
+++ b/test/Interop/Cxx/stdlib/Inputs/std-pair.h
@@ -23,3 +23,17 @@ using PairStructInt = std::pair<StructInPair, int>;
 inline PairStructInt getPairStructInt(int x) {
     return { { x * 2, -x}, x };
 }
+
+struct UnsafeStruct {
+    int *ptr;
+};
+
+struct __attribute__((swift_attr("import_iterator"))) Iterator {};
+
+using PairUnsafeStructInt = std::pair<UnsafeStruct, int>;
+using PairIteratorInt = std::pair<Iterator, int>;
+
+struct HasMethodThatReturnsUnsafePair {
+    PairUnsafeStructInt getUnsafePair() const { return {}; }
+    PairIteratorInt getIteratorPair() const { return {}; }
+};
diff --git a/test/Interop/Cxx/stdlib/use-std-pair-typechecker.swift b/test/Interop/Cxx/stdlib/use-std-pair-typechecker.swift
new file mode 100644
index 0000000000000..637662afbc271
--- /dev/null
+++ b/test/Interop/Cxx/stdlib/use-std-pair-typechecker.swift
@@ -0,0 +1,8 @@
+// RUN: %target-typecheck-verify-swift -I %S/Inputs -enable-experimental-cxx-interop
+// REQUIRES: OS=macosx || OS=linux-gnu
+
+import StdPair
+
+let u = HasMethodThatReturnsUnsafePair()
+u.getUnsafePair() // expected-error {{value of type 'HasMethodThatReturnsUnsafePair' has no member 'getUnsafePair'}}
+u.getIteratorPair() // expected-error {{value of type 'HasMethodThatReturnsUnsafePair' has no member 'getIteratorPair'}}