[C++] Fix double pointers to foreign ref types

When a C++ type `ForeignTy` is imported as a foreign reference type, Swift should strip a level of indirection from parameters of that type, so e.g. `ForeignTy *` and `ForeignTy &` should be imported to Swift as just `ForeignTy`.  However, it should only strip *one* level of indirection. importType() was instead stripping *all* levels, so things like `ForeignTy **` and `ForeignTy *&` were *also* being incorrectly imported as `ForeignTy`. Narrow this behavior so it only applies to a single level of indirection and add a few test cases to pin down the specifics.

Fixes rdar://123905345.

Author: beccadax

lib/ClangImporter/ImportType.cpp

@@ -201,6 +201,13 @@ namespace {
                                     : ImportHint::OtherPointer};
   }
 
+  static bool
+  isDirectUseOfForeignReferenceType(clang::QualType clangPointeeType,
+                                    Type swiftPointeeType) {
+     return swiftPointeeType && swiftPointeeType->isForeignReferenceType() &&
+               !clangPointeeType->isPointerType();
+   }
+
   class SwiftTypeConverter :
     public clang::TypeVisitor<SwiftTypeConverter, ImportResult>
   {
@@ -494,9 +501,10 @@ namespace {
           pointeeQualType, ImportTypeKind::Value, addImportDiagnostic,
           AllowNSUIntegerAsInt, Bridgeability::None, ImportTypeAttrs());
 
-      // If this is imported as a reference type, ignore the pointer.
-      if (pointeeType && pointeeType->isForeignReferenceType())
-         return {pointeeType, ImportHint::OtherPointer};
+      // If this is imported as a reference type, ignore the innermost pointer.
+      // (`T *` becomes `T`, but `T **` becomes `UnsafeMutablePointer<T>`.)
+      if (isDirectUseOfForeignReferenceType(pointeeQualType, pointeeType))
+        return {pointeeType, ImportHint::OtherPointer};
 
       // If the pointed-to type is unrepresentable in Swift, or its C
       // alignment is greater than the maximum Swift alignment, import as
@@ -592,7 +600,7 @@ namespace {
       if (!pointeeType)
         return Type();
 
-      if (pointeeType->isForeignReferenceType())
+      if (isDirectUseOfForeignReferenceType(pointeeQualType, pointeeType))
         return {pointeeType, ImportHint::None};
 
       if (pointeeQualType->isFunctionType()) {
@@ -2531,6 +2539,15 @@ ClangImporter::Implementation::importParameterType(
     swiftParamTy = importedType.getType();
   }
 
+  // `isInOut` is set above if we stripped off a mutable `&` before importing
+  // the type. Normally, we want to use an `inout` parameter in this situation.
+  // However, if the parameter belongs to a foreign reference type *and* the
+  // reference we stripped out was directly to that type (rather than to a
+  // pointer to that type), the foreign reference type should "eat" the
+  // indirection of the `&`, so we *don't* want to use an `inout` parameter.
+  if (isInOut && isDirectUseOfForeignReferenceType(paramTy, swiftParamTy))
+    isInOut = false;
+
   return ImportParameterTypeResult{swiftParamTy, isInOut,
                                    isParamTypeImplicitlyUnwrapped};
 }
@@ -2611,9 +2628,8 @@ static ParamDecl *getParameterInfo(ClangImporter::Implementation *impl,
 
   // Foreign references are already references so they don't need to be passed
   // as inout.
-  paramInfo->setSpecifier(isInOut && !swiftParamTy->isForeignReferenceType()
-                              ? ParamSpecifier::InOut
-                              : ParamSpecifier::Default);
+  paramInfo->setSpecifier(isInOut ? ParamSpecifier::InOut
+                                  : ParamSpecifier::Default);
   paramInfo->setInterfaceType(swiftParamTy);
   impl->recordImplicitUnwrapForDecl(paramInfo, isParamTypeImplicitlyUnwrapped);
 

test/Interop/Cxx/ergonomics/swift-bridging-annotations.swift

@@ -27,6 +27,19 @@ public protocol Proto {
 #if BRIDGING_HEADER_TEST
 func f() -> SharedObject { return SharedObject.create() }
 
+func g() {
+  var logger: LoggerSingleton?
+  var loggerPtr: UnsafeMutablePointer<LoggerSingleton?>?
+  var loggerPtrPtr: UnsafeMutablePointer<UnsafeMutablePointer<LoggerSingleton?>?>?
+
+  takeLoggersByPointer(logger, &logger, &loggerPtr)
+  takeLoggersByPointer(logger, loggerPtr, loggerPtrPtr)
+  takeLoggersByPointer(nil, nil, nil)
+  
+  takeLoggersByReference(logger!, &logger, &loggerPtr)
+  takeLoggersByReference(logger!, &loggerPtr!.pointee, &loggerPtrPtr!.pointee)
+}
+
 func releaseSharedObject(_: SharedObject) { }
 #endif
 
@@ -71,6 +84,12 @@ public:
     static LoggerSingleton *getInstance();
 };
 
+void takeLoggersByPointer(LoggerSingleton *ptr, LoggerSingleton **ptr_ptr, LoggerSingleton ***ptr_ptr_ptr);
+void takeLoggersByReference(LoggerSingleton &ref, LoggerSingleton *&ref_ptr, LoggerSingleton **&ref_ptr_ptr);
+
+void takeLoggersByConstPointer(const LoggerSingleton **pointee0, LoggerSingleton const **pointee1, LoggerSingleton *const *pointer);
+void takeLoggersByConstReference(const LoggerSingleton *&pointee0, LoggerSingleton const *&pointee1, LoggerSingleton *const &pointer);
+
 class SWIFT_UNSAFE_REFERENCE UnsafeNonCopyable {
 public:
     UnsafeNonCopyable(UnsafeNonCopyable &) = delete;
@@ -110,6 +129,26 @@ private:
 // CHECK:   class func getInstance() -> LoggerSingleton!
 // CHECK: }
 
+// CHECK-LABEL: func takeLoggersByPointer(
+// CHECK-SAME: _ ptr: LoggerSingleton!,
+// CHECK-SAME: _ ptr_ptr: UnsafeMutablePointer<LoggerSingleton?>!,
+// CHECK-SAME: _ ptr_ptr_ptr: UnsafeMutablePointer<UnsafeMutablePointer<LoggerSingleton?>?>!)
+
+// CHECK-LABEL: func takeLoggersByReference(
+// CHECK-SAME: _ ref: LoggerSingleton,
+// CHECK-SAME: _ ref_ptr: inout LoggerSingleton!,
+// CHECK-SAME: _ ref_ptr_ptr: inout UnsafeMutablePointer<LoggerSingleton?>!)
+
+// CHECK-LABEL: func takeLoggersByConstPointer(
+// CHECK-SAME: _ pointee0: UnsafeMutablePointer<LoggerSingleton?>!,
+// CHECK-SAME: _ pointee1: UnsafeMutablePointer<LoggerSingleton?>!,
+// CHECK-SAME: _ pointer: UnsafePointer<LoggerSingleton?>!)
+
+// CHECK-LABEL: func takeLoggersByConstReference(
+// CHECK-SAME: _ pointee0: inout LoggerSingleton!,
+// CHECK-SAME: _ pointee1: inout LoggerSingleton!,
+// CHECK-SAME: _ pointer: LoggerSingleton!)
+
 // CHECK: class UnsafeNonCopyable {
 // CHECK: }
 // CHECK: func returnsPointerToUnsafeReference() -> UnsafeNonCopyable!