From ab123929f6a301eddeb4a1fcdcdea903cecc8c3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Horv=C3=A1th?= Date: Tue, 15 Jul 2025 17:07:53 +0100 Subject: [PATCH 1/2] [6.2][cxx-interop] Types exposed from ObjC modules should be behind a macro Explanation: We the generated reverse interop headers to be valid C++, so every declaration coming from an Obj-C module should be behind an ifdef. Unfortunately, we do not always have this information but we do know that our frameworks contain Obj-C code. So this PR makes sure every entity coming from our frameworks are behind ifdef. Issues: rdar://152836730 Original PRs: #83002 Risk: Low, the change is narrow and straightforward. Testing: Added a compiler test. Reviewers: @egorzhdan --- include/swift/AST/SwiftNameTranslation.h | 3 ++ lib/AST/SwiftNameTranslation.cpp | 37 +++++++++++++++++++ lib/PrintAsClang/PrintClangFunction.cpp | 3 +- lib/PrintAsClang/PrintClangValueType.cpp | 19 ++++++++-- ...n-type-not-exposed-by-default-to-cxx.swift | 6 +++ 5 files changed, 63 insertions(+), 5 deletions(-) diff --git a/include/swift/AST/SwiftNameTranslation.h b/include/swift/AST/SwiftNameTranslation.h index 3e3c880643197..2f7054fe00c4a 100644 --- a/include/swift/AST/SwiftNameTranslation.h +++ b/include/swift/AST/SwiftNameTranslation.h @@ -112,6 +112,9 @@ inline bool isExposableToCxx( return !getDeclRepresentation(VD, isZeroSized).isUnsupported(); } +bool isObjCxxOnly(const ValueDecl *VD); +bool isObjCxxOnly(const clang::Decl *D); + /// Returns true if the given value decl D is visible to C++ of its /// own accord (i.e. without considering its context) bool isVisibleToCxx(const ValueDecl *VD, AccessLevel minRequiredAccess, diff --git a/lib/AST/SwiftNameTranslation.cpp b/lib/AST/SwiftNameTranslation.cpp index 41e29f5cceeee..52b329ca1781f 100644 --- a/lib/AST/SwiftNameTranslation.cpp +++ b/lib/AST/SwiftNameTranslation.cpp @@ -28,6 +28,7 @@ #include "swift/Basic/StringExtras.h" #include "clang/AST/DeclObjC.h" +#include "clang/Basic/Module.h" #include "llvm/ADT/SmallString.h" #include @@ -238,6 +239,39 @@ swift::cxx_translation::getNameForCxx(const ValueDecl *VD, return VD->getBaseIdentifier().str(); } +namespace { +struct ObjCTypeWalker : TypeWalker { + bool hadObjCType = false; + Action walkToTypePre(Type ty) override { + if (auto *nominal = ty->getNominalOrBoundGenericNominal()) { + if (auto clangDecl = nominal->getClangDecl()) { + if (cxx_translation::isObjCxxOnly(clangDecl)) { + hadObjCType = true; + return Action::Stop; + } + } + } + return Action::Continue; + } +}; +} // namespace + +bool swift::cxx_translation::isObjCxxOnly(const ValueDecl *VD) { + ObjCTypeWalker walker; + VD->getInterfaceType().walk(walker); + return walker.hadObjCType; +} + +bool swift::cxx_translation::isObjCxxOnly(const clang::Decl *D) { + // By default, we import all modules in Obj-C++ mode, so there is no robust + // way to tell if something is coming from an Obj-C module. The best + // approximation is to check if something is a framework module as most of + // those are written in Obj-C. Ideally, we want to add `requires ObjC` to all + // ObjC modules and respect that here to make this completely precise. + return D->getASTContext().getLangOpts().ObjC && + D->getOwningModule()->isPartOfFramework(); +} + swift::cxx_translation::DeclRepresentation swift::cxx_translation::getDeclRepresentation( const ValueDecl *VD, @@ -324,6 +358,9 @@ swift::cxx_translation::getDeclRepresentation( return {Unsupported, UnrepresentableGenericRequirements}; } + if (isObjCxxOnly(VD)) + return {ObjCxxOnly, std::nullopt}; + return {Representable, std::nullopt}; } diff --git a/lib/PrintAsClang/PrintClangFunction.cpp b/lib/PrintAsClang/PrintClangFunction.cpp index 3ae06026bb43f..1bd546494efbf 100644 --- a/lib/PrintAsClang/PrintClangFunction.cpp +++ b/lib/PrintAsClang/PrintClangFunction.cpp @@ -812,7 +812,8 @@ ClangRepresentation DeclAndTypeClangFunctionPrinter::printFunctionSignature( ClangSyntaxPrinter(FD->getASTContext(), functionSignatureOS).printInlineForThunk(); ClangRepresentation resultingRepresentation = - ClangRepresentation::representable; + cxx_translation::isObjCxxOnly(FD) ? ClangRepresentation::objcxxonly + : ClangRepresentation::representable; // Print out the return type. if (FD->hasThrows() && outputLang == OutputLanguageMode::Cxx) diff --git a/lib/PrintAsClang/PrintClangValueType.cpp b/lib/PrintAsClang/PrintClangValueType.cpp index 7e05ead0613fc..facbd0bc47fc3 100644 --- a/lib/PrintAsClang/PrintClangValueType.cpp +++ b/lib/PrintAsClang/PrintClangValueType.cpp @@ -22,6 +22,7 @@ #include "swift/ClangImporter/ClangImporter.h" #include "swift/IRGen/IRABIDetailsProvider.h" #include "swift/IRGen/Linking.h" +#include "clang/Basic/Module.h" #include "llvm/ADT/STLExtras.h" #include "llvm/Support/raw_ostream.h" @@ -642,6 +643,12 @@ void ClangValueTypePrinter::printTypeGenericTraits( }); } + bool objCxxOnly = false; + if (const auto *clangDecl = typeDecl->getClangDecl()) { + if (cxx_translation::isObjCxxOnly(clangDecl)) + objCxxOnly = true; + } + // FIXME: avoid popping out of the module's namespace here. os << "} // end namespace \n\n"; os << "namespace swift SWIFT_PRIVATE_ATTR {\n"; @@ -649,13 +656,15 @@ void ClangValueTypePrinter::printTypeGenericTraits( bool addPointer = typeDecl->isObjC() || (classDecl && classDecl->isForeignReferenceType()); + if (objCxxOnly) + os << "#if defined(__OBJC__)\n"; os << "#pragma clang diagnostic push\n"; os << "#pragma clang diagnostic ignored \"-Wc++17-extensions\"\n"; - if (typeDecl->hasClangNode()) { + if (const auto *clangDecl = typeDecl->getClangDecl()) { // FIXME: share the code. os << "template<>\n"; os << "inline const constexpr bool isUsableInGenericContext<"; - printer.printClangTypeReference(typeDecl->getClangDecl()); + printer.printClangTypeReference(clangDecl); if (addPointer) os << "*"; os << "> = true;\n"; @@ -665,8 +674,8 @@ void ClangValueTypePrinter::printTypeGenericTraits( os << "struct"; declAndTypePrinter.printAvailability(os, typeDecl); os << " TypeMetadataTrait<"; - if (typeDecl->hasClangNode()) { - printer.printClangTypeReference(typeDecl->getClangDecl()); + if (const auto *clangDecl = typeDecl->getClangDecl()) { + printer.printClangTypeReference(clangDecl); if (addPointer) os << "*"; } else { @@ -746,6 +755,8 @@ void ClangValueTypePrinter::printTypeGenericTraits( } os << "} // namespace\n"; os << "#pragma clang diagnostic pop\n"; + if (objCxxOnly) + os << "#endif // #if defined(__OBJC__)\n"; os << "} // namespace swift\n"; os << "\n"; printer.printModuleNamespaceStart(*moduleContext); diff --git a/test/Interop/SwiftToCxx/stdlib/foundation-type-not-exposed-by-default-to-cxx.swift b/test/Interop/SwiftToCxx/stdlib/foundation-type-not-exposed-by-default-to-cxx.swift index cf8ef20b2014c..26fbae249059c 100644 --- a/test/Interop/SwiftToCxx/stdlib/foundation-type-not-exposed-by-default-to-cxx.swift +++ b/test/Interop/SwiftToCxx/stdlib/foundation-type-not-exposed-by-default-to-cxx.swift @@ -3,6 +3,8 @@ // RUN: %target-swift-frontend %s -module-name UseFoundation -enable-experimental-cxx-interop -typecheck -verify -emit-clang-header-path %t/UseFoundation.h // RUN: %FileCheck %s < %t/UseFoundation.h +// RUN: %check-interop-cxx-header-in-clang(%t/UseFoundation.h -DSWIFT_CXX_INTEROP_HIDE_STL_OVERLAY) + // REQUIRES: objc_interop import Foundation @@ -12,4 +14,8 @@ public enum UseFoundationEnum { case B } +public func f() -> Decimal { + Decimal() +} + // CHECK: class UseFoundationEnum { } SWIFT_UNAVAILABLE_MSG("Swift enum 'UseFoundationEnum' cannot be represented in C++"); From f48da454384b1fdd0c7a058ceeb07cfb399dd353 Mon Sep 17 00:00:00 2001 From: Gabor Horvath Date: Thu, 17 Jul 2025 16:18:32 +0100 Subject: [PATCH 2/2] [cxx-interop] Configure requires ObjC from frontend option We sometimes don't have the information in the modulemaps whether a module requires ObjC or not. This info is useful for reverse interop. This PR introduces a frontend flag to have a comma separated list of modules that we should import as if they had "requires ObjC" in their modulemaps. --- include/swift/AST/SwiftNameTranslation.h | 3 +- include/swift/Basic/LangOptions.h | 4 +++ lib/AST/SwiftNameTranslation.cpp | 31 +++++++++++++------ lib/Frontend/CompilerInvocation.cpp | 8 ++++- lib/PrintAsClang/PrintClangValueType.cpp | 3 +- ...n-type-not-exposed-by-default-to-cxx.swift | 2 +- .../Misc/verify-swift-feature-testing.test-sh | 2 ++ 7 files changed, 40 insertions(+), 13 deletions(-) diff --git a/include/swift/AST/SwiftNameTranslation.h b/include/swift/AST/SwiftNameTranslation.h index 2f7054fe00c4a..1ae3f691fccd9 100644 --- a/include/swift/AST/SwiftNameTranslation.h +++ b/include/swift/AST/SwiftNameTranslation.h @@ -13,6 +13,7 @@ #ifndef SWIFT_NAME_TRANSLATION_H #define SWIFT_NAME_TRANSLATION_H +#include "swift/AST/ASTContext.h" #include "swift/AST/AttrKind.h" #include "swift/AST/Decl.h" #include "swift/AST/DiagnosticEngine.h" @@ -113,7 +114,7 @@ inline bool isExposableToCxx( } bool isObjCxxOnly(const ValueDecl *VD); -bool isObjCxxOnly(const clang::Decl *D); +bool isObjCxxOnly(const clang::Decl *D, const ASTContext &ctx); /// Returns true if the given value decl D is visible to C++ of its /// own accord (i.e. without considering its context) diff --git a/include/swift/Basic/LangOptions.h b/include/swift/Basic/LangOptions.h index a288bbcb0b625..1e29cc15363d0 100644 --- a/include/swift/Basic/LangOptions.h +++ b/include/swift/Basic/LangOptions.h @@ -632,6 +632,10 @@ namespace swift { /// All block list configuration files to be honored in this compilation. std::vector BlocklistConfigFilePaths; + /// List of top level modules to be considered as if they had require ObjC + /// in their module map. + llvm::SmallVector ModulesRequiringObjC; + /// Whether to ignore checks that a module is resilient during /// type-checking, SIL verification, and IR emission, bool BypassResilienceChecks = false; diff --git a/lib/AST/SwiftNameTranslation.cpp b/lib/AST/SwiftNameTranslation.cpp index 52b329ca1781f..730faf9d6984c 100644 --- a/lib/AST/SwiftNameTranslation.cpp +++ b/lib/AST/SwiftNameTranslation.cpp @@ -29,6 +29,7 @@ #include "clang/AST/DeclObjC.h" #include "clang/Basic/Module.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallString.h" #include @@ -242,10 +243,12 @@ swift::cxx_translation::getNameForCxx(const ValueDecl *VD, namespace { struct ObjCTypeWalker : TypeWalker { bool hadObjCType = false; + const ASTContext &ctx; + ObjCTypeWalker(const ASTContext &ctx) : ctx(ctx) {} Action walkToTypePre(Type ty) override { if (auto *nominal = ty->getNominalOrBoundGenericNominal()) { if (auto clangDecl = nominal->getClangDecl()) { - if (cxx_translation::isObjCxxOnly(clangDecl)) { + if (cxx_translation::isObjCxxOnly(clangDecl, ctx)) { hadObjCType = true; return Action::Stop; } @@ -257,19 +260,29 @@ struct ObjCTypeWalker : TypeWalker { } // namespace bool swift::cxx_translation::isObjCxxOnly(const ValueDecl *VD) { - ObjCTypeWalker walker; + ObjCTypeWalker walker{VD->getASTContext()}; VD->getInterfaceType().walk(walker); return walker.hadObjCType; } -bool swift::cxx_translation::isObjCxxOnly(const clang::Decl *D) { +bool swift::cxx_translation::isObjCxxOnly(const clang::Decl *D, + const ASTContext &ctx) { // By default, we import all modules in Obj-C++ mode, so there is no robust - // way to tell if something is coming from an Obj-C module. The best - // approximation is to check if something is a framework module as most of - // those are written in Obj-C. Ideally, we want to add `requires ObjC` to all - // ObjC modules and respect that here to make this completely precise. - return D->getASTContext().getLangOpts().ObjC && - D->getOwningModule()->isPartOfFramework(); + // way to tell if something is coming from an Obj-C module. Use the + // requirements and the language options to check if we should actually + // consider the module to have ObjC constructs. + const auto &langOpts = D->getASTContext().getLangOpts(); + auto clangModule = D->getOwningModule()->getTopLevelModule(); + bool requiresObjC = false; + for (auto req : clangModule->Requirements) + if (req.RequiredState && req.FeatureName == "objc") + requiresObjC = true; + return langOpts.ObjC && + (requiresObjC || + llvm::any_of(ctx.LangOpts.ModulesRequiringObjC, + [clangModule](StringRef moduleName) { + return clangModule->getFullModuleName() == moduleName; + })); } swift::cxx_translation::DeclRepresentation diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp index 141e4d2b9f0ab..7a9e2270fdb63 100644 --- a/lib/Frontend/CompilerInvocation.cpp +++ b/lib/Frontend/CompilerInvocation.cpp @@ -847,7 +847,8 @@ static bool ParseEnabledFeatureArgs(LangOptions &Opts, ArgList &Args, // Collect some special case pseudo-features which should be processed // separately. if (argValue.starts_with("StrictConcurrency") || - argValue.starts_with("AvailabilityMacro=")) { + argValue.starts_with("AvailabilityMacro=") || + argValue.starts_with("RequiresObjC=")) { if (isEnableFeatureFlag) psuedoFeatures.push_back(argValue); continue; @@ -974,6 +975,11 @@ static bool ParseEnabledFeatureArgs(LangOptions &Opts, ArgList &Args, Opts.AvailabilityMacros.push_back(availability.str()); continue; } + + if (featureName->starts_with("RequiresObjC")) { + auto modules = featureName->split("=").second; + modules.split(Opts.ModulesRequiringObjC, ","); + } } // Map historical flags over to experimental features. We do this for all diff --git a/lib/PrintAsClang/PrintClangValueType.cpp b/lib/PrintAsClang/PrintClangValueType.cpp index facbd0bc47fc3..2dec50f2d4235 100644 --- a/lib/PrintAsClang/PrintClangValueType.cpp +++ b/lib/PrintAsClang/PrintClangValueType.cpp @@ -16,6 +16,7 @@ #include "OutputLanguageMode.h" #include "PrimitiveTypeMapping.h" #include "SwiftToClangInteropContext.h" +#include "swift/AST/ASTContext.h" #include "swift/AST/Decl.h" #include "swift/AST/SwiftNameTranslation.h" #include "swift/AST/Type.h" @@ -645,7 +646,7 @@ void ClangValueTypePrinter::printTypeGenericTraits( bool objCxxOnly = false; if (const auto *clangDecl = typeDecl->getClangDecl()) { - if (cxx_translation::isObjCxxOnly(clangDecl)) + if (cxx_translation::isObjCxxOnly(clangDecl, typeDecl->getASTContext())) objCxxOnly = true; } diff --git a/test/Interop/SwiftToCxx/stdlib/foundation-type-not-exposed-by-default-to-cxx.swift b/test/Interop/SwiftToCxx/stdlib/foundation-type-not-exposed-by-default-to-cxx.swift index 26fbae249059c..b1779211c630c 100644 --- a/test/Interop/SwiftToCxx/stdlib/foundation-type-not-exposed-by-default-to-cxx.swift +++ b/test/Interop/SwiftToCxx/stdlib/foundation-type-not-exposed-by-default-to-cxx.swift @@ -1,6 +1,6 @@ // RUN: %empty-directory(%t) -// RUN: %target-swift-frontend %s -module-name UseFoundation -enable-experimental-cxx-interop -typecheck -verify -emit-clang-header-path %t/UseFoundation.h +// RUN: %target-swift-frontend %s -module-name UseFoundation -enable-experimental-cxx-interop -typecheck -verify -emit-clang-header-path %t/UseFoundation.h -enable-experimental-feature RequiresObjC=CoreGraphics,Foundation // RUN: %FileCheck %s < %t/UseFoundation.h // RUN: %check-interop-cxx-header-in-clang(%t/UseFoundation.h -DSWIFT_CXX_INTEROP_HIDE_STL_OVERLAY) diff --git a/test/Misc/verify-swift-feature-testing.test-sh b/test/Misc/verify-swift-feature-testing.test-sh index 41bc56793409e..bc3ad2c941fd7 100755 --- a/test/Misc/verify-swift-feature-testing.test-sh +++ b/test/Misc/verify-swift-feature-testing.test-sh @@ -21,6 +21,8 @@ EXCEPTIONAL_FILES = [ pathlib.Path("test/ModuleInterface/swift-export-as.swift"), # Uses the pseudo-feature AvailabilityMacro= pathlib.Path("test/Availability/availability_define.swift"), + # Uses the pseudo-feature RequiresObjC= + pathlib.Path("test/Interop/SwiftToCxx/stdlib/foundation-type-not-exposed-by-default-to-cxx.swift"), # Tests behavior when you try to use a feature without enabling it pathlib.Path("test/attr/feature_requirement.swift"), # Tests completion with features both enabled and disabled