[ClangImporter] Don't drop "Ref" from a CF types if it creates a conf…

…lict.

https://bugs.swift.org/browse/SR-545

lib/ClangImporter/ClangImporter.cpp

@@ -2278,60 +2278,62 @@ auto ClangImporter::Implementation::importFullName(
       baseName = baseName.substr(removePrefix.size());
   }
 
+  auto hasConflict = [&](const clang::IdentifierInfo *proposedName) -> bool {
+    // Test to see if there is a value with the same name as 'proposedName'
+    // in the same module as the decl
+    // FIXME: This will miss macros.
+    auto clangModule = getClangSubmoduleForDecl(D);
+    if (clangModule.hasValue() && clangModule.getValue())
+      clangModule = clangModule.getValue()->getTopLevelModule();
+
+    auto isInSameModule = [&](const clang::Decl *D) -> bool {
+      auto declModule = getClangSubmoduleForDecl(D);
+      if (!declModule.hasValue())
+        return false;
+      // Handle the bridging header case. This is pretty nasty since things
+      // can get added to it *later*, but there's not much we can do.
+      if (!declModule.getValue())
+        return *clangModule == nullptr;
+      return *clangModule == declModule.getValue()->getTopLevelModule();
+    };
+
+    // Allow this lookup to find hidden names.  We don't want the
+    // decision about whether to rename the decl to depend on
+    // what exactly the user has imported.  Indeed, if we're being
+    // asked to resolve a serialization cross-reference, the user
+    // may not have imported this module at all, which means a
+    // normal lookup wouldn't even find the decl!
+    //
+    // Meanwhile, we don't need to worry about finding unwanted
+    // hidden declarations from different modules because we do a
+    // module check before deciding that there's a conflict.
+    clang::LookupResult lookupResult(clangSema, proposedName,
+                                     clang::SourceLocation(),
+                                     clang::Sema::LookupOrdinaryName);
+    lookupResult.setAllowHidden(true);
+    lookupResult.suppressDiagnostics();
+
+    if (clangSema.LookupName(lookupResult, /*scope=*/nullptr)) {
+      if (std::any_of(lookupResult.begin(), lookupResult.end(), isInSameModule))
+        return true;
+    }
+
+    lookupResult.clear(clang::Sema::LookupTagName);
+    if (clangSema.LookupName(lookupResult, /*scope=*/nullptr)) {
+      if (std::any_of(lookupResult.begin(), lookupResult.end(), isInSameModule))
+        return true;
+    }
+
+    return false;
+  };
+
   // Objective-C protocols may have the suffix "Protocol" appended if
   // the non-suffixed name would conflict with another entity in the
   // same top-level module.
   SmallString<16> baseNameWithProtocolSuffix;
   if (auto objcProto = dyn_cast<clang::ObjCProtocolDecl>(D)) {
     if (objcProto->hasDefinition()) {
-      // Test to see if there is a value with the same name as the protocol
-      // in the same module.
-      // FIXME: This will miss macros.
-      auto clangModule = getClangSubmoduleForDecl(objcProto);
-      if (clangModule.hasValue() && clangModule.getValue())
-        clangModule = clangModule.getValue()->getTopLevelModule();
-
-      auto isInSameModule = [&](const clang::Decl *D) -> bool {
-        auto declModule = getClangSubmoduleForDecl(D);
-        if (!declModule.hasValue())
-          return false;
-        // Handle the bridging header case. This is pretty nasty since things
-        // can get added to it *later*, but there's not much we can do.
-        if (!declModule.getValue())
-          return *clangModule == nullptr;
-        return *clangModule == declModule.getValue()->getTopLevelModule();
-      };
-
-      // Allow this lookup to find hidden names.  We don't want the
-      // decision about whether to rename the protocol to depend on
-      // what exactly the user has imported.  Indeed, if we're being
-      // asked to resolve a serialization cross-reference, the user
-      // may not have imported this module at all, which means a
-      // normal lookup wouldn't even find the protocol!
-      //
-      // Meanwhile, we don't need to worry about finding unwanted
-      // hidden declarations from different modules because we do a
-      // module check before deciding that there's a conflict.
-      bool hasConflict = false;
-      clang::LookupResult lookupResult(clangSema, D->getDeclName(),
-                                       clang::SourceLocation(),
-                                       clang::Sema::LookupOrdinaryName);
-      lookupResult.setAllowHidden(true);
-      lookupResult.suppressDiagnostics();
-
-      if (clangSema.LookupName(lookupResult, /*scope=*/nullptr)) {
-        hasConflict = std::any_of(lookupResult.begin(), lookupResult.end(),
-                                  isInSameModule);
-      }
-      if (!hasConflict) {
-        lookupResult.clear(clang::Sema::LookupTagName);
-        if (clangSema.LookupName(lookupResult, /*scope=*/nullptr)) {
-          hasConflict = std::any_of(lookupResult.begin(), lookupResult.end(),
-                                    isInSameModule);
-        }
-      }
-
-      if (hasConflict) {
+      if (hasConflict(objcProto->getIdentifier())) {
         baseNameWithProtocolSuffix = baseName;
         baseNameWithProtocolSuffix += SWIFT_PROTOCOL_SUFFIX;
         baseName = baseNameWithProtocolSuffix;
@@ -2341,29 +2343,38 @@ auto ClangImporter::Implementation::importFullName(
 
   // Typedef declarations might be CF types that will drop the "Ref"
   // suffix.
+  clang::ASTContext &clangCtx = clangSema.Context;
   bool aliasIsFunction = false;
   bool aliasIsInitializer = false;
   StringRef aliasBaseName;
   SmallVector<StringRef, 4> aliasArgumentNames;
   if (auto typedefNameDecl = dyn_cast<clang::TypedefNameDecl>(D)) {
     auto swiftName = getCFTypeName(typedefNameDecl, &aliasBaseName);
     if (!swiftName.empty()) {
-      baseName = swiftName;
+      if (swiftName != baseName) {
+        assert(aliasBaseName == baseName);
+        if (!hasConflict(&clangCtx.Idents.get(swiftName)))
+          baseName = swiftName;
+        else
+          aliasBaseName = "";
+      } else if (!aliasBaseName.empty()) {
+        if (hasConflict(&clangCtx.Idents.get(aliasBaseName)))
+          aliasBaseName = "";
+      }
     }
   }
 
   // Local function to determine whether the given declaration is subject to
   // a swift_private attribute.
-  auto clangSemaPtr = &clangSema;
-  auto hasSwiftPrivate = [clangSemaPtr](const clang::NamedDecl *D) {
+  auto hasSwiftPrivate = [&clangSema](const clang::NamedDecl *D) {
     if (D->hasAttr<clang::SwiftPrivateAttr>())
       return true;
 
     // Enum constants that are not imported as members should be considered
     // private if the parent enum is marked private.
     if (auto *ECD = dyn_cast<clang::EnumConstantDecl>(D)) {
       auto *ED = cast<clang::EnumDecl>(ECD->getDeclContext());
-      switch (classifyEnum(clangSemaPtr->getPreprocessor(), ED)) {
+      switch (classifyEnum(clangSema.getPreprocessor(), ED)) {
         case EnumKind::Constants:
         case EnumKind::Unknown:
           if (ED->hasAttr<clang::SwiftPrivateAttr>())
@@ -2383,7 +2394,6 @@ auto ClangImporter::Implementation::importFullName(
   };
 
   // Omit needless words.
-  clang::ASTContext &clangCtx = clangSema.Context;
   StringScratchSpace omitNeedlessWordsScratch;
   if (OmitNeedlessWords) {
     // Objective-C properties.

test/ClangModules/Inputs/custom-modules/CFAndObjC.h

@@ -0,0 +1,7 @@
+typedef const struct __attribute__((objc_bridge(id))) __MyProblematicObject *MyProblematicObjectRef;
+
+@interface MyProblematicObject
+@end
+
+typedef float MyProblematicAlias;
+typedef MyProblematicObjectRef MyProblematicAliasRef;

test/ClangModules/Inputs/custom-modules/module.map

@@ -7,6 +7,11 @@ module AvailabilityExtras {
   export *
 }
 
+module CFAndObjC {
+  header "CFAndObjC.h"
+  export *
+}
+
 module ClangModuleUser {
   header "ClangModuleUser.h"
   export *

test/ClangModules/cf.swift

@@ -3,6 +3,7 @@
 // REQUIRES: objc_interop
 
 import CoreCooling
+import CFAndObjC
 
 func assertUnmanaged<T: AnyObject>(t: Unmanaged<T>) {}
 func assertManaged<T: AnyObject>(t: T) {}
@@ -119,3 +120,21 @@ func testOutParametersBad() {
   let item: CCItem?
   CCRefrigeratorGetItemUnaudited(0, 0, item) // expected-error {{cannot convert value of type 'Int' to expected argument type 'CCRefrigerator!'}}
 }
+
+func nameCollisions() {
+  var objc: MyProblematicObject?
+  var cf: MyProblematicObjectRef?
+  cf = objc // expected-error {{cannot assign value of type 'MyProblematicObject?' to type 'MyProblematicObjectRef?'}}
+  objc = cf // expected-error {{cannot assign value of type 'MyProblematicObjectRef?' to type 'MyProblematicObject?'}}
+
+  var cfAlias: MyProblematicAliasRef?
+  cfAlias = cf // okay
+  cf = cfAlias // okay
+
+  var otherAlias: MyProblematicAlias?
+  otherAlias = cfAlias // expected-error {{cannot assign value of type 'MyProblematicAliasRef?' to type 'MyProblematicAlias?'}}
+  cfAlias = otherAlias // expected-error {{cannot assign value of type 'MyProblematicAlias?' to type 'MyProblematicAliasRef?'}}
+
+  func isOptionalFloat(inout _: Optional<Float>) {}
+  isOptionalFloat(&otherAlias) // okay
+}