From c73f528b82747a74bb93c3f28bc6651a21a253df Mon Sep 17 00:00:00 2001 From: Egor Zhdan Date: Mon, 18 Aug 2025 19:18:18 +0100 Subject: [PATCH] [cxx-interop] Validate C foreign reference types Swift validates the retain/release operations for foreign reference types to check for obvious errors, e.g. a wrong parameter type or return type. That logic was only running for C++ foreign reference types. This patch enables it for C foreign reference types as well. rdar://158609723 --- lib/ClangImporter/ImportDecl.cpp | 52 ++++++++++--------- .../struct/Inputs/foreign-reference-invalid.h | 20 +++++++ test/Interop/C/struct/Inputs/module.modulemap | 4 ++ ...oreign-reference-invalid-typechecker.swift | 15 ++++++ 4 files changed, 66 insertions(+), 25 deletions(-) create mode 100644 test/Interop/C/struct/Inputs/foreign-reference-invalid.h create mode 100644 test/Interop/C/struct/foreign-reference-invalid-typechecker.swift diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp index 096c8d7315876..a6d1e347b30a9 100644 --- a/lib/ClangImporter/ImportDecl.cpp +++ b/lib/ClangImporter/ImportDecl.cpp @@ -2692,6 +2692,25 @@ namespace { } } + if (auto classDecl = dyn_cast(result)) { + validateForeignReferenceType(decl, classDecl); + + auto availability = Impl.SwiftContext.getSwift58Availability(); + if (!availability.isAlwaysAvailable()) { + assert(availability.hasMinimumVersion()); + auto AvAttr = AvailableAttr::createPlatformVersioned( + Impl.SwiftContext, targetPlatform(Impl.SwiftContext.LangOpts), + /*Message=*/"", /*Rename=*/"", + availability.getRawMinimumVersion(), /*Deprecated=*/{}, + /*Obsoleted=*/{}); + classDecl->getAttrs().add(AvAttr); + } + + if (cxxRecordDecl && cxxRecordDecl->isEffectivelyFinal()) + classDecl->getAttrs().add(new (Impl.SwiftContext) + FinalAttr(/*IsImplicit=*/true)); + } + // If we need it, add an explicit "deinit" to this type. synthesizer.addExplicitDeinitIfRequired(result, decl); @@ -2737,7 +2756,7 @@ namespace { } } - void validateForeignReferenceType(const clang::CXXRecordDecl *decl, + void validateForeignReferenceType(const clang::RecordDecl *decl, ClassDecl *classDecl) { enum class RetainReleaseOperationKind { @@ -2789,11 +2808,13 @@ namespace { // The parameter of the retain/release function should be pointer to the // same FRT or a base FRT. if (paramDecl != classDecl) { - if (const clang::Decl *paramClangDecl = paramDecl->getClangDecl()) { - if (const auto *paramTypeDecl = - dyn_cast(paramClangDecl)) { - if (decl->isDerivedFrom(paramTypeDecl)) { - return RetainReleaseOperationKind::valid; + if (auto cxxDecl = dyn_cast(decl)) { + if (const clang::Decl *paramClangDecl = paramDecl->getClangDecl()) { + if (const auto *paramTypeDecl = + dyn_cast(paramClangDecl)) { + if (cxxDecl->isDerivedFrom(paramTypeDecl)) { + return RetainReleaseOperationKind::valid; + } } } } @@ -3140,25 +3161,6 @@ namespace { validatePrivateFileIDAttributes(decl); - if (auto classDecl = dyn_cast(result)) { - validateForeignReferenceType(decl, classDecl); - - auto availability = Impl.SwiftContext.getSwift58Availability(); - if (!availability.isAlwaysAvailable()) { - assert(availability.hasMinimumVersion()); - auto AvAttr = AvailableAttr::createPlatformVersioned( - Impl.SwiftContext, targetPlatform(Impl.SwiftContext.LangOpts), - /*Message=*/"", /*Rename=*/"", - availability.getRawMinimumVersion(), /*Deprecated=*/{}, - /*Obsoleted=*/{}); - classDecl->getAttrs().add(AvAttr); - } - - if (decl->isEffectivelyFinal()) - classDecl->getAttrs().add(new (Impl.SwiftContext) - FinalAttr(/*IsImplicit=*/true)); - } - // If this module is declared as a C++ module, try to synthesize // conformances to Swift protocols from the Cxx module. auto clangModule = Impl.getClangOwningModule(result->getClangNode()); diff --git a/test/Interop/C/struct/Inputs/foreign-reference-invalid.h b/test/Interop/C/struct/Inputs/foreign-reference-invalid.h new file mode 100644 index 0000000000000..39a6b4fbc48ac --- /dev/null +++ b/test/Interop/C/struct/Inputs/foreign-reference-invalid.h @@ -0,0 +1,20 @@ +#include + +struct __attribute__((swift_attr("import_reference"))) +__attribute__((swift_attr("retain:nonexistent"))) +__attribute__((swift_attr("release:nonexistent"))) NonExistent { + int value; +}; + +struct __attribute__((swift_attr("import_reference"))) NoRetainRelease { + int value; +}; + +struct __attribute__((swift_attr("import_reference"))) +__attribute__((swift_attr("retain:badRetain"))) +__attribute__((swift_attr("release:badRelease"))) BadRetainRelease { + int value; +}; + +float badRetain(struct BadRetainRelease *v); +void badRelease(struct BadRetainRelease *v, int i); diff --git a/test/Interop/C/struct/Inputs/module.modulemap b/test/Interop/C/struct/Inputs/module.modulemap index 82698ffd42c87..339adb3139a14 100644 --- a/test/Interop/C/struct/Inputs/module.modulemap +++ b/test/Interop/C/struct/Inputs/module.modulemap @@ -7,6 +7,10 @@ module ForeignReference { header "foreign-reference.h" } +module ForeignReferenceInvalid { + header "foreign-reference-invalid.h" +} + module StructAsOptionSet { header "struct-as-option-set.h" } diff --git a/test/Interop/C/struct/foreign-reference-invalid-typechecker.swift b/test/Interop/C/struct/foreign-reference-invalid-typechecker.swift new file mode 100644 index 0000000000000..9717c614ed4b0 --- /dev/null +++ b/test/Interop/C/struct/foreign-reference-invalid-typechecker.swift @@ -0,0 +1,15 @@ +// RUN: not %target-swift-frontend -typecheck %s -I %S/Inputs -disable-availability-checking -diagnostic-style llvm 2>&1 | %FileCheck %s + +import ForeignReferenceInvalid + +// CHECK: error: cannot find retain function 'nonexistent' for reference type 'NonExistent' +// CHECK: error: cannot find release function 'nonexistent' for reference type 'NonExistent' +public func test(x: NonExistent) { } + +// CHECK: error: reference type 'NoRetainRelease' must have 'retain:' Swift attribute +// CHECK: error: reference type 'NoRetainRelease' must have 'release:' Swift attribute +public func test(x: NoRetainRelease) { } + +// CHECK: error: specified retain function 'badRetain' is invalid; retain function must either return have 'void', the reference count as an integer, or the parameter type +// CHECK: error: specified release function 'badRelease' is invalid; release function must have exactly one argument of type 'BadRetainRelease' +public func test(x: BadRetainRelease) { }