[ClangImporter] Supporting changes towards structs with ARC pointers (swiftlang#59594)

In order to allow supporting `__strong` (and `__weak`) struct fields,
some parts of the ClangImporter needs to understand them. The changes in
this commit allows the type importer to allow the already supported
`__unsafe_unretained` struct fields, but still reject the `__strong` and
`__weak` fields. Later changes will add support for bridging `__strong`
and `__weak` fields.

All the code should be equivalent to the previous code, and since all
the structs with non-trivial copy/destroy are completely discarded, the
code should not even be hit in any case.

The included modifications in the tests check that the error and the
diagnostics note are produced correctly.

Author: drodriguez

lib/ClangImporter/ImportType.cpp

@@ -1267,8 +1267,8 @@ static bool canBridgeTypes(ImportTypeKind importKind) {
   case ImportTypeKind::Variable:
   case ImportTypeKind::AuditedVariable:
   case ImportTypeKind::Enum:
-  case ImportTypeKind::RecordField:
     return false;
+  case ImportTypeKind::RecordField:
   case ImportTypeKind::Result:
   case ImportTypeKind::AuditedResult:
   case ImportTypeKind::Parameter:
@@ -1419,7 +1419,8 @@ static ImportedType adjustTypeForConcreteImport(
     bool allowNSUIntegerAsInt, Bridgeability bridging,
     llvm::function_ref<void(Diagnostic &&)> addImportDiagnostic,
     ImportTypeAttrs attrs, OptionalTypeKind optKind,
-    bool resugarNSErrorPointer) {
+    bool resugarNSErrorPointer,
+    clang::Qualifiers::ObjCLifetime objCLifetime) {
   Type importedType = importResult.AbstractType;
   ImportHint hint = importResult.Hint;
 
@@ -1464,6 +1465,8 @@ static ImportedType adjustTypeForConcreteImport(
     // bridge, do so.
     if (canBridgeTypes(importKind) &&
         importKind != ImportTypeKind::PropertyWithReferenceSemantics &&
+        !(importKind == ImportTypeKind::RecordField &&
+          objCLifetime <= clang::Qualifiers::OCL_ExplicitNone) &&
         !(importKind == ImportTypeKind::Typedef &&
           bridging == Bridgeability::None)) {
       // id and Any can be bridged without Foundation. There would be
@@ -1484,7 +1487,10 @@ static ImportedType adjustTypeForConcreteImport(
     // In some contexts, we bridge them to use the Swift function type
     // representation. This includes typedefs of block types, which use the
     // Swift function type representation.
-    if (!canBridgeTypes(importKind))
+    // FIXME: Do not bridge on RecordFields to keep previous behaviour for
+    // the time being.
+    if (!canBridgeTypes(importKind) ||
+        importKind == ImportTypeKind::RecordField)
       break;
 
     // Determine the function type representation we need.
@@ -1579,15 +1585,26 @@ static ImportedType adjustTypeForConcreteImport(
   assert(importedType);
 
   if (importKind == ImportTypeKind::RecordField &&
-      importedType->isAnyClassReferenceType() &&
       !importedType->isForeignReferenceType()) {
-    // Wrap retainable struct fields in Unmanaged.
-    // FIXME: Eventually we might get C++-like support for strong pointers in
-    // structs, at which point we should really be checking the lifetime
-    // qualifiers.
-    // FIXME: This should apply to blocks as well, but Unmanaged is constrained
-    // to AnyObject.
-    importedType = getUnmanagedType(impl, importedType);
+    switch (objCLifetime) {
+      // Wrap retainable struct fields in Unmanaged.
+      case clang::Qualifiers::OCL_None:
+      case clang::Qualifiers::OCL_ExplicitNone:
+        // FIXME: This should apply to blocks as well, but Unmanaged is constrained
+        // to AnyObject.
+        if (importedType->isAnyClassReferenceType()) {
+          importedType = getUnmanagedType(impl, importedType);
+        }
+        break;
+      // FIXME: Eventually we might get C++-like support for strong pointers in
+      // structs, at which point we should really be checking the lifetime
+      // qualifiers.
+      case clang::Qualifiers::OCL_Strong:
+      case clang::Qualifiers::OCL_Weak:
+        return {Type(), false};
+      case clang::Qualifiers::OCL_Autoreleasing:
+        llvm_unreachable("invalid Objective-C lifetime");
+    }
   }
 
   // Apply attrs.
@@ -1665,6 +1682,8 @@ ImportedType ClangImporter::Implementation::importType(
     }
   }
 
+  clang::Qualifiers::ObjCLifetime objCLifetime = type.getObjCLifetime();
+
   // Perform abstract conversion, ignoring how the type is actually used.
   SwiftTypeConverter converter(
       *this, addImportDiagnosticFn, allowNSUIntegerAsInt, bridging,
@@ -1674,7 +1693,8 @@ ImportedType ClangImporter::Implementation::importType(
   // Now fix up the type based on how we're concretely using it.
   auto adjustedType = adjustTypeForConcreteImport(
       *this, importResult, importKind, allowNSUIntegerAsInt, bridging,
-      addImportDiagnosticFn, attrs, optionality, resugarNSErrorPointer);
+      addImportDiagnosticFn, attrs, optionality, resugarNSErrorPointer,
+      objCLifetime);
 
   return adjustedType;
 }

test/ClangImporter/objc_bridging.swift

@@ -1,4 +1,4 @@
-// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -typecheck -parse-as-library -verify %s
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -typecheck -parse-as-library -verify-additional-file %clang-importer-sdk-path/usr/include/objc_structs.h -verify %s
 
 // REQUIRES: objc_interop
 
@@ -62,6 +62,10 @@ func objcStructs(_ s: StructOfNSStrings, sb: StructOfBlocks) {
   // FIXME: Blocks should also be Unmanaged.
   _ = sb.block as Bool // expected-error {{cannot convert value of type '@convention(block) () -> Void' to type 'Bool' in coercion}}
   sb.block() // okay
+
+  // Structs with non-trivial copy/destroy should not be imported
+  _ = WeaksInAStruct() // expected-error {{cannot find 'WeaksInAStruct' in scope}}
+  _ = StrongsInAStruct() // expected-error {{cannot find 'StrongsInAStruct' in scope}}
 }
 
 func test_repair_does_not_interfere_with_conversions() {

test/Inputs/clang-importer-sdk/usr/include/objc_structs.h

@@ -7,3 +7,11 @@ struct StructOfNSStrings {
 struct StructOfBlocks {
   void (^__unsafe_unretained _Nonnull block)(void);
 };
+
+struct StrongsInAStruct { // expected-note {{record 'StrongsInAStruct' is not trivial to copy/destroy}}
+  __strong NSString *nsstr;
+};
+
+struct WeaksInAStruct { // expected-note {{record 'WeaksInAStruct' is not trivial to copy/destroy}}
+  __weak NSString *nsstr;
+};