From 38c23bcdda7e55c0301df8c4b952cc846159c016 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Fri, 31 Oct 2025 15:53:50 -0700 Subject: [PATCH 1/2] [Serialization] Allow near-misses when matching imported Clang declarations Fixes the rest of rdar://137014448. --- lib/Serialization/Deserialization.cpp | 51 ++++++++++++++++++- .../Inputs/c_takes_ptr_nonnull.h | 1 + .../Inputs/c_takes_ptr_nullable.h | 1 + .../Inputs/extern_with_nonnull.swift | 6 +++ .../Inputs/extern_with_nullable.swift | 6 +++ .../SIL/Serialization/Inputs/module.modulemap | 7 +++ .../Serialization/c_header_collision.swift | 19 +++++++ test/SIL/Serialization/extern_collision.swift | 4 +- 8 files changed, 91 insertions(+), 4 deletions(-) create mode 100644 test/SIL/Serialization/Inputs/c_takes_ptr_nonnull.h create mode 100644 test/SIL/Serialization/Inputs/c_takes_ptr_nullable.h create mode 100644 test/SIL/Serialization/Inputs/module.modulemap create mode 100644 test/SIL/Serialization/c_header_collision.swift diff --git a/lib/Serialization/Deserialization.cpp b/lib/Serialization/Deserialization.cpp index 14000a8a793fd..c6fb73f04534c 100644 --- a/lib/Serialization/Deserialization.cpp +++ b/lib/Serialization/Deserialization.cpp @@ -2000,6 +2000,33 @@ 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::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 +2042,10 @@ static void filterValues(Type expectedTy, ModuleDecl *expectedModule, if (expectedTy) canTy = expectedTy->getCanonicalType(); + llvm::TinyPtrVector 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 +2055,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 +2111,20 @@ static void filterValues(Type expectedTy, ModuleDecl *expectedModule, cast(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. diff --git a/test/SIL/Serialization/Inputs/c_takes_ptr_nonnull.h b/test/SIL/Serialization/Inputs/c_takes_ptr_nonnull.h new file mode 100644 index 0000000000000..726d73db6c553 --- /dev/null +++ b/test/SIL/Serialization/Inputs/c_takes_ptr_nonnull.h @@ -0,0 +1 @@ +void takes_a_void_pointer(const void * _Nonnull pointer); diff --git a/test/SIL/Serialization/Inputs/c_takes_ptr_nullable.h b/test/SIL/Serialization/Inputs/c_takes_ptr_nullable.h new file mode 100644 index 0000000000000..b1072c0e73976 --- /dev/null +++ b/test/SIL/Serialization/Inputs/c_takes_ptr_nullable.h @@ -0,0 +1 @@ +void takes_a_void_pointer(const void * _Nullable pointer); diff --git a/test/SIL/Serialization/Inputs/extern_with_nonnull.swift b/test/SIL/Serialization/Inputs/extern_with_nonnull.swift index f96420a498b6e..93f6312b5550e 100644 --- a/test/SIL/Serialization/Inputs/extern_with_nonnull.swift +++ b/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() { diff --git a/test/SIL/Serialization/Inputs/extern_with_nullable.swift b/test/SIL/Serialization/Inputs/extern_with_nullable.swift index 7f70981e306ce..e1500ee4ce524 100644 --- a/test/SIL/Serialization/Inputs/extern_with_nullable.swift +++ b/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() { diff --git a/test/SIL/Serialization/Inputs/module.modulemap b/test/SIL/Serialization/Inputs/module.modulemap new file mode 100644 index 0000000000000..3e238210cf2a4 --- /dev/null +++ b/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" +} diff --git a/test/SIL/Serialization/c_header_collision.swift b/test/SIL/Serialization/c_header_collision.swift new file mode 100644 index 0000000000000..d786183d438f9 --- /dev/null +++ b/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() +} diff --git a/test/SIL/Serialization/extern_collision.swift b/test/SIL/Serialization/extern_collision.swift index c7d13069a2334..fa730e3551ff5 100644 --- a/test/SIL/Serialization/extern_collision.swift +++ b/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 From 1bae4da121220b481e12bca232d3ce9254bfb57c Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Sun, 2 Nov 2025 08:32:50 -0800 Subject: [PATCH 2/2] [Deserialization] Require labels to match when looking for closely-matching clang declarations --- include/swift/AST/Types.h | 2 ++ lib/AST/Type.cpp | 5 +++++ lib/Serialization/Deserialization.cpp | 3 ++- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/include/swift/AST/Types.h b/include/swift/AST/Types.h index fec1440265ca5..1b4a9b2571b09 100644 --- a/include/swift/AST/Types.h +++ b/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; diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp index a9bc44a7abe6c..26de2d02a112d 100644 --- a/lib/AST/Type.cpp +++ b/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. diff --git a/lib/Serialization/Deserialization.cpp b/lib/Serialization/Deserialization.cpp index c6fb73f04534c..a0ceef1c73f36 100644 --- a/lib/Serialization/Deserialization.cpp +++ b/lib/Serialization/Deserialization.cpp @@ -2013,7 +2013,8 @@ namespace { return TypeComparison::Equal; if (nearMatchOk) { - TypeMatchOptions options = TypeMatchFlags::AllowTopLevelOptionalMismatch; + TypeMatchOptions options = TypeMatchFlags::RequireMatchingParameterLabels; + options |= TypeMatchFlags::AllowTopLevelOptionalMismatch; options |= TypeMatchFlags::AllowNonOptionalForIUOParam; options |= TypeMatchFlags::IgnoreNonEscapingForOptionalFunctionParam; options |= TypeMatchFlags::IgnoreFunctionSendability;