Merge pull request #85262 from DougGregor/clang-deserialization-near-miss

[Serialization] Allow near-misses when matching imported Clang declarations

Author: DougGregor

include/swift/AST/Types.h

@@ -372,6 +372,8 @@ enum class TypeMatchFlags {
   IgnoreSendability = 1 << 7,
   /// Ignore global actor isolation attributes on functions when matching types.
   IgnoreFunctionGlobalActorIsolation = 1 << 8,
+  /// Require parameter labels to match.
+  RequireMatchingParameterLabels = 1 << 9,
 };
 using TypeMatchOptions = OptionSet<TypeMatchFlags>;
 

lib/AST/Type.cpp

@@ -3604,6 +3604,11 @@ static bool matches(CanType t1, CanType t2, TypeMatchOptions matchMode,
                      OptionalUnwrapping::None)) {
           return false;
         }
+
+        if (matchMode.contains(TypeMatchFlags::RequireMatchingParameterLabels)&&
+            fn1Params[i].getLabel() != fn2Params[i].getLabel()) {
+          return false;
+        }
       }
 
       // Results are covariant.

lib/Serialization/Deserialization.cpp

@@ -2000,6 +2000,34 @@ static bool isReExportedToModule(const ValueDecl *value,
   return exportedName == expectedModule->getName().str();
 }
 
+namespace {
+  /// The result of a type comparison.
+  enum class TypeComparison {
+    NotEqual,
+    Equal,
+    NearMatch,
+  };
+
+  TypeComparison compareTypes(CanType type1, CanType type2, bool nearMatchOk) {
+    if (type1->isEqual(type2))
+      return TypeComparison::Equal;
+
+    if (nearMatchOk) {
+      TypeMatchOptions options = TypeMatchFlags::RequireMatchingParameterLabels;
+      options |= TypeMatchFlags::AllowTopLevelOptionalMismatch;
+      options |= TypeMatchFlags::AllowNonOptionalForIUOParam;
+      options |= TypeMatchFlags::IgnoreNonEscapingForOptionalFunctionParam;
+      options |= TypeMatchFlags::IgnoreFunctionSendability;
+      options |= TypeMatchFlags::IgnoreSendability;
+      options |= TypeMatchFlags::IgnoreFunctionGlobalActorIsolation;
+      if (type1->matches(type2, options))
+        return TypeComparison::NearMatch;
+    }
+
+    return TypeComparison::NotEqual;
+  }
+}
+
 /// Remove values from \p values that don't match the expected type or module.
 ///
 /// Any of \p expectedTy, \p expectedModule, or \p expectedGenericSig can be
@@ -2015,8 +2043,10 @@ static void filterValues(Type expectedTy, ModuleDecl *expectedModule,
   if (expectedTy)
     canTy = expectedTy->getCanonicalType();
 
+  llvm::TinyPtrVector<ValueDecl *> clangNearMatches;
+
   auto newEnd = std::remove_if(values.begin(), values.end(),
-                               [=](ValueDecl *value) {
+                               [=, &clangNearMatches](ValueDecl *value) {
     // Ignore anything that was parsed (vs. deserialized), because a serialized
     // module cannot refer to it.
     if (value->getDeclContext()->getParentSourceFile())
@@ -2026,7 +2056,14 @@ static void filterValues(Type expectedTy, ModuleDecl *expectedModule,
       return true;
 
     // If we're expecting a type, make sure this decl has the expected type.
-    if (canTy && !value->getInterfaceType()->isEqual(canTy))
+    TypeComparison typesMatch = TypeComparison::Equal;
+    if (canTy) {
+      typesMatch = compareTypes(canTy,
+                                value->getInterfaceType()->getCanonicalType(),
+                                importedFromClang);
+    }
+
+    if (typesMatch == TypeComparison::NotEqual)
       return true;
 
     if (value->isStatic() != isStatic)
@@ -2075,9 +2112,20 @@ static void filterValues(Type expectedTy, ModuleDecl *expectedModule,
           cast<ConstructorDecl>(value)->getInitKind() != *ctorInit)
         return true;
     }
+
+    // Record near matches.
+    if (typesMatch == TypeComparison::NearMatch) {
+      clangNearMatches.push_back(value);
+      return true;
+    }
+
+    ASSERT(typesMatch == TypeComparison::Equal);
     return false;
   });
   values.erase(newEnd, values.end());
+
+  if (values.empty())
+    values.append(clangNearMatches.begin(), clangNearMatches.end());
 }
 
 /// Look for nested types in all files of \p extensionModule except from the \p thisFile.

test/SIL/Serialization/Inputs/c_takes_ptr_nonnull.h

@@ -0,0 +1 @@
+void takes_a_void_pointer(const void * _Nonnull pointer);

test/SIL/Serialization/Inputs/c_takes_ptr_nullable.h

@@ -0,0 +1 @@
+void takes_a_void_pointer(const void * _Nullable pointer);

test/SIL/Serialization/Inputs/extern_with_nonnull.swift

@@ -1,5 +1,11 @@
+#if USE_EXTERN
 @_extern(c, "takes_a_void_pointer")
 public func takes_a_void_pointer(_ pointer: UnsafeRawPointer)
+#elseif USE_C_MODULE
+import CTakesPtrNonNull
+#else
+#error("Not configured")
+#endif
 
 @_alwaysEmitIntoClient
 public func callWithNonNull() {

test/SIL/Serialization/Inputs/extern_with_nullable.swift

@@ -1,5 +1,11 @@
+#if USE_EXTERN
 @_extern(c, "takes_a_void_pointer")
 public func takes_a_void_pointer(_ pointer: UnsafeRawPointer?)
+#elseif USE_C_MODULE
+import CTakesPtrNullable
+#else
+#error("Not configured")
+#endif
 
 @_alwaysEmitIntoClient
 public func callWithNullable() {

test/SIL/Serialization/Inputs/module.modulemap

@@ -0,0 +1,7 @@
+module CTakesPtrNullable {
+  header "c_takes_ptr_nullable.h"
+}
+
+module CTakesPtrNonNull {
+  header "c_takes_ptr_nonnull.h"
+}

test/SIL/Serialization/c_header_collision.swift

@@ -0,0 +1,19 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module -DUSE_C_MODULE -I %S/Inputs -o %t %S/Inputs/extern_with_nullable.swift -module-name c_with_nullable -enable-experimental-feature Extern
+// RUN: %target-swift-frontend -emit-module -DUSE_C_MODULE -I %S/Inputs -o %t %S/Inputs/extern_with_nonnull.swift -module-name c_with_nonnull -enable-experimental-feature Extern
+// RUN: %target-swift-frontend -emit-sil -o %t -I %t -primary-file %s -module-name main -O -enable-experimental-feature Extern
+
+// RUN: %target-swift-frontend -emit-ir -o %t -I %t -primary-file %s -module-name main -O -enable-experimental-feature Extern
+
+// Don't crash or otherwise fail when inlining multiple functions that reference
+// C declarations of the same name but different types at the SIL level.
+
+// REQUIRES: swift_feature_Extern
+
+import c_with_nullable
+import c_with_nonnull
+
+public func main() {
+  callWithNullable()
+  callWithNonNull()
+}

test/SIL/Serialization/extern_collision.swift

@@ -1,6 +1,6 @@
 // RUN: %empty-directory(%t)
-// RUN: %target-swift-frontend -emit-module -enable-experimental-feature Extern -o %t %S/Inputs/extern_with_nullable.swift
-// RUN: %target-swift-frontend -emit-module -enable-experimental-feature Extern -o %t %S/Inputs/extern_with_nonnull.swift
+// RUN: %target-swift-frontend -emit-module -DUSE_EXTERN -enable-experimental-feature Extern -o %t %S/Inputs/extern_with_nullable.swift
+// RUN: %target-swift-frontend -emit-module -DUSE_EXTERN -enable-experimental-feature Extern -o %t %S/Inputs/extern_with_nonnull.swift
 // RUN: %target-swift-frontend -emit-sil -o %t -I %t -primary-file %s -module-name main -O
 
 // REQUIRES: swift_feature_Extern