[cxx-interop] Fix a regression making std::string unsafe

Currently, we only get warnings for using unsafe types in expressions
but not in the function signature. The tests did not use the std::string
object in the function body. As a result, we regressed and std::string
was considered unsafe.

The reason is that the annotation only mode for calculating escapability
of a type did not do what we intended. std::basic_string is
conditionally escapable if the template argument is escapable. We
considered 'char' to have unknown escapability in annotation only mode.
The annotation only mode was introduced to avoid suddenly importing
certain types as not escapable when they have pointer fields and break
backward compatibility.

The solution is to make annotation only mode to still consider char and
co as escapable types and only fall back to unknown when the inference
otherwise would have deduced non-escapable (for unannotated typed).

Author: Gabor Horvath

Diff for: include/swift/ClangImporter/ClangImporterRequests.h

@@ -537,6 +537,9 @@ enum class CxxEscapability { Escapable, NonEscapable, Unknown };
 struct EscapabilityLookupDescriptor final {
   const clang::Type *type;
   ClangImporter::Implementation *impl;
+  // Only explicitly ~Escapable annotated types are considered ~Escapable.
+  // This is for backward compatibility, so we continue to import aggregates
+  // containing pointers as Escapable types.
   bool annotationOnly = true;
 
   friend llvm::hash_code hash_value(const EscapabilityLookupDescriptor &desc) {

Diff for: lib/ClangImporter/ClangImporter.cpp

@@ -55,6 +55,7 @@
 #include "swift/Strings.h"
 #include "swift/Subsystems.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
@@ -5348,8 +5349,6 @@ ClangTypeEscapability::evaluate(Evaluator &evaluator,
       return hadUnknown ? CxxEscapability::Unknown : CxxEscapability::Escapable;
     }
   }
-  if (desc.annotationOnly)
-    return CxxEscapability::Unknown;
   if (desugared->isArrayType()) {
     auto elemTy = cast<clang::ArrayType>(desugared)
                       ->getElementType()
@@ -5363,7 +5362,8 @@ ClangTypeEscapability::evaluate(Evaluator &evaluator,
   // Base cases
   if (desugared->isAnyPointerType() || desugared->isBlockPointerType() ||
       desugared->isMemberPointerType() || desugared->isReferenceType())
-    return CxxEscapability::NonEscapable;
+    return desc.annotationOnly ? CxxEscapability::Unknown
+                               : CxxEscapability::NonEscapable;
   if (desugared->isScalarType())
     return CxxEscapability::Escapable;
   return CxxEscapability::Unknown;
@@ -8508,7 +8508,7 @@ ExplicitSafety ClangDeclExplicitSafety::evaluate(
   // Enums are always safe.
   if (isa<clang::EnumDecl>(decl))
     return ExplicitSafety::Safe;
-  
+
   // If it's not a record, leave it unspecified.
   auto recordDecl = dyn_cast<clang::RecordDecl>(decl);
   if (!recordDecl)

Diff for: test/Interop/Cxx/class/safe-interop-mode.swift

@@ -91,9 +91,11 @@ func useSafeParams(x: Owner, y: View, z: SafeEscapableAggregate, c: MyContainer)
 }
 
 func useCfType(x: CFArray) {
+  _ = x
 }
 
 func useString(x: std.string) {
+  _ = x
 }
 
 func useVecOfPtr(x: VecOfPtr) {
@@ -102,9 +104,11 @@ func useVecOfPtr(x: VecOfPtr) {
 }
 
 func useVecOfInt(x: VecOfInt) {
+  _ = x
 }
 
 func useSafeTuple(x: SafeTuple) {
+  _ = x
 }
 
 func useUnsafeTuple(x: UnsafeTuple) {