From 6a3defdfb0ef65503b08ee7aa06044a55801b954 Mon Sep 17 00:00:00 2001 From: Egor Zhdan Date: Fri, 15 Sep 2023 13:57:25 +0100 Subject: [PATCH 1/3] Revert "Revert "[cxx-interop] Use up-to-date definition of `CF_OPTIONS` in tests"" This reverts commit e1a9be2203bf351256a71f43ee6916f3dbc01b62. --- .../enum/Inputs/c-enums-withOptions-omit.h | 60 ++++++++++++------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/test/Interop/Cxx/enum/Inputs/c-enums-withOptions-omit.h b/test/Interop/Cxx/enum/Inputs/c-enums-withOptions-omit.h index daeced9d2377f..81b407ed60e48 100644 --- a/test/Interop/Cxx/enum/Inputs/c-enums-withOptions-omit.h +++ b/test/Interop/Cxx/enum/Inputs/c-enums-withOptions-omit.h @@ -1,39 +1,57 @@ -// Enum usage that is bitwise-able and assignable in C++, aka how CF_OPTIONS -// does things. -typedef int __attribute__((availability(swift, unavailable))) NSEnumerationOptions; -enum : NSEnumerationOptions { NSEnumerationConcurrent, NSEnumerationReverse }; +typedef unsigned NSUInteger; + +#define __CF_OPTIONS_ATTRIBUTES __attribute__((flag_enum,enum_extensibility(open))) +#if (__cplusplus) +#define CF_OPTIONS(_type, _name) __attribute__((availability(swift,unavailable))) _type _name; enum __CF_OPTIONS_ATTRIBUTES : _name +#else +#define CF_OPTIONS(_type, _name) enum __CF_OPTIONS_ATTRIBUTES _name : _type _name; enum _name : _type +#endif + +typedef CF_OPTIONS(NSUInteger, NSEnumerationOptions) { + NSEnumerationConcurrent = (1UL << 0), + NSEnumerationReverse = (1UL << 1), +}; @interface NSSet - (void)enumerateObjectsWithOptions:(NSEnumerationOptions)opts ; @end -typedef int __attribute__((availability(swift, unavailable))) NSOrderedCollectionDifferenceCalculationOptions; -enum : NSOrderedCollectionDifferenceCalculationOptions { +typedef CF_OPTIONS(NSUInteger, NSOrderedCollectionDifferenceCalculationOptions) { NSOrderedCollectionDifferenceCalculationOptions1, NSOrderedCollectionDifferenceCalculationOptions2 }; -typedef int __attribute__((availability(swift, unavailable))) NSCalendarUnit; -enum : NSCalendarUnit { NSCalendarUnit1, NSCalendarUnit2 }; +typedef CF_OPTIONS(NSUInteger, NSCalendarUnit) { + NSCalendarUnit1, + NSCalendarUnit2 +}; -typedef int __attribute__((availability(swift, unavailable))) NSSearchPathDomainMask; -enum : NSSearchPathDomainMask { NSSearchPathDomainMask1, NSSearchPathDomainMask2 }; +typedef CF_OPTIONS(NSUInteger, NSSearchPathDomainMask) { + NSSearchPathDomainMask1, + NSSearchPathDomainMask2 +}; -typedef int __attribute__((availability(swift, unavailable))) NSControlCharacterAction; -enum : NSControlCharacterAction { NSControlCharacterAction1, NSControlCharacterAction2 }; +typedef CF_OPTIONS(NSUInteger, NSControlCharacterAction) { + NSControlCharacterAction1, + NSControlCharacterAction2 +}; -typedef int __attribute__((availability(swift, unavailable))) UIControlState; -enum : UIControlState { UIControlState1, UIControlState2 }; +typedef CF_OPTIONS(NSUInteger, UIControlState) { + UIControlState1, + UIControlState2 +}; -typedef int __attribute__((availability(swift, unavailable))) UITableViewCellStateMask; -enum : UITableViewCellStateMask { UITableViewCellStateMask1, UITableViewCellStateMask2 }; +typedef CF_OPTIONS(NSUInteger, UITableViewCellStateMask) { + UITableViewCellStateMask1, + UITableViewCellStateMask2 +}; -typedef int __attribute__((availability(swift, unavailable))) UIControlEvents; -enum : UIControlEvents { UIControlEvents1, UIControlEvents2 }; +typedef CF_OPTIONS(NSUInteger, UIControlEvents) { + UIControlEvents1, + UIControlEvents2 +}; -typedef int __attribute__((availability(swift, unavailable))) -UITableViewScrollPosition; -enum : UITableViewScrollPosition { +typedef CF_OPTIONS(NSUInteger, UITableViewScrollPosition) { UITableViewScrollPosition1, UITableViewScrollPosition2 }; From f7c2257f51b55a9025f98f732f91373e764dcdab Mon Sep 17 00:00:00 2001 From: Egor Zhdan Date: Fri, 15 Sep 2023 13:57:59 +0100 Subject: [PATCH 2/3] Revert "Revert "[cxx-interop] Import custom `NS_OPTIONS` correctly"" This reverts commit e1d70f06e7dd4d3bae3c46a84ba8eaece0227fbc. --- lib/ClangImporter/ImportType.cpp | 61 ++++++++----------- .../objc-correctness/Inputs/customNSOptions.h | 6 ++ .../objc-correctness/Inputs/module.modulemap | 4 ++ .../objc-correctness/custom-nsoptions.swift | 7 +++ 4 files changed, 41 insertions(+), 37 deletions(-) create mode 100644 test/Interop/Cxx/objc-correctness/Inputs/customNSOptions.h create mode 100644 test/Interop/Cxx/objc-correctness/custom-nsoptions.swift diff --git a/lib/ClangImporter/ImportType.cpp b/lib/ClangImporter/ImportType.cpp index fe915dd30a9dc..b8540d3e12607 100644 --- a/lib/ClangImporter/ImportType.cpp +++ b/lib/ClangImporter/ImportType.cpp @@ -2646,44 +2646,31 @@ ArgumentAttrs ClangImporter::Implementation::inferDefaultArgument( } } else if (const clang::TypedefType *typedefType = type->getAs()) { - // Get the AvailabilityAttr that would be set from CF/NS_OPTIONS - if (importer::isUnavailableInSwift(typedefType->getDecl(), nullptr, true)) { - // If we've taken this branch it means we have an enum type, and it is - // likely an integer or NSInteger that is being used by NS/CF_OPTIONS to - // behave like a C enum in the presence of C++. - auto enumName = typedefType->getDecl()->getName(); - ArgumentAttrs argumentAttrs(DefaultArgumentKind::None, true, enumName); - auto camelCaseWords = camel_case::getWords(enumName); - for (auto it = camelCaseWords.rbegin(); it != camelCaseWords.rend(); - ++it) { - auto word = *it; - auto next = std::next(it); - if (camel_case::sameWordIgnoreFirstCase(word, "options")) { - argumentAttrs.argumentKind = DefaultArgumentKind::EmptyArray; - return argumentAttrs; + clang::TypedefNameDecl *typedefDecl = typedefType->getDecl(); + // Find the next decl in the same context. If this typedef is a part of an + // NS/CF_OPTIONS declaration, the next decl will be an enum. + auto declsInContext = typedefDecl->getDeclContext()->decls(); + auto declIter = llvm::find(declsInContext, typedefDecl); + if (declIter != declsInContext.end()) + declIter++; + if (declIter != declsInContext.end()) { + if (auto enumDecl = dyn_cast(*declIter)) { + if (auto cfOptionsTy = + nameImporter.getContext() + .getClangModuleLoader() + ->getTypeDefForCXXCFOptionsDefinition(enumDecl)) { + if (cfOptionsTy->getDecl() == typedefDecl) { + auto enumName = typedefDecl->getName(); + ArgumentAttrs argumentAttrs(DefaultArgumentKind::None, true, + enumName); + for (auto word : llvm::reverse(camel_case::getWords(enumName))) { + if (camel_case::sameWordIgnoreFirstCase(word, "options")) { + argumentAttrs.argumentKind = DefaultArgumentKind::EmptyArray; + } + } + return argumentAttrs; + } } - if (camel_case::sameWordIgnoreFirstCase(word, "units")) - return argumentAttrs; - if (camel_case::sameWordIgnoreFirstCase(word, "domain")) - return argumentAttrs; - if (camel_case::sameWordIgnoreFirstCase(word, "action")) - return argumentAttrs; - if (camel_case::sameWordIgnoreFirstCase(word, "event")) - return argumentAttrs; - if (camel_case::sameWordIgnoreFirstCase(word, "events") && - next != camelCaseWords.rend() && - camel_case::sameWordIgnoreFirstCase(*next, "control")) - return argumentAttrs; - if (camel_case::sameWordIgnoreFirstCase(word, "state")) - return argumentAttrs; - if (camel_case::sameWordIgnoreFirstCase(word, "unit")) - return argumentAttrs; - if (camel_case::sameWordIgnoreFirstCase(word, "position") && - next != camelCaseWords.rend() && - camel_case::sameWordIgnoreFirstCase(*next, "scroll")) - return argumentAttrs; - if (camel_case::sameWordIgnoreFirstCase(word, "edge")) - return argumentAttrs; } } } diff --git a/test/Interop/Cxx/objc-correctness/Inputs/customNSOptions.h b/test/Interop/Cxx/objc-correctness/Inputs/customNSOptions.h new file mode 100644 index 0000000000000..b666f4d713027 --- /dev/null +++ b/test/Interop/Cxx/objc-correctness/Inputs/customNSOptions.h @@ -0,0 +1,6 @@ +#include "CFOptions.h" + +typedef CF_OPTIONS(unsigned, MyControlFlags) { + MyControlFlagsNone = 0, + MyControlFlagsFirst +}; diff --git a/test/Interop/Cxx/objc-correctness/Inputs/module.modulemap b/test/Interop/Cxx/objc-correctness/Inputs/module.modulemap index 1d273bf94ff87..9d3a9bf5ac1f0 100644 --- a/test/Interop/Cxx/objc-correctness/Inputs/module.modulemap +++ b/test/Interop/Cxx/objc-correctness/Inputs/module.modulemap @@ -9,6 +9,10 @@ module CxxClassWithNSStringInit { requires cplusplus } +module CustomNSOptions { + header "customNSOptions.h" +} + module NSOptionsMangling { header "NSOptionsMangling.h" } diff --git a/test/Interop/Cxx/objc-correctness/custom-nsoptions.swift b/test/Interop/Cxx/objc-correctness/custom-nsoptions.swift new file mode 100644 index 0000000000000..6c5e398643d10 --- /dev/null +++ b/test/Interop/Cxx/objc-correctness/custom-nsoptions.swift @@ -0,0 +1,7 @@ +// RUN: %target-typecheck-verify-swift -verify-ignore-unknown -I %S/Inputs -enable-objc-interop -enable-experimental-cxx-interop +// REQUIRES: objc_interop + +import CustomNSOptions + +let flags1: MyControlFlags = [] +let flags2: MyControlFlags = [.first] From 6a2f10a38848674bde476ddfe0743cc333bdbb51 Mon Sep 17 00:00:00 2001 From: Egor Zhdan Date: Fri, 15 Sep 2023 17:10:04 +0100 Subject: [PATCH 3/3] [cxx-interop] Refactor: do not rely on Clang module importer being available This makes sure that we can emit a pch from a C++ header that uses `CF_OPTIONS`. rdar://112225263 --- include/swift/AST/ClangModuleLoader.h | 3 --- include/swift/ClangImporter/ClangImporter.h | 4 ++-- lib/AST/ASTMangler.cpp | 4 ++-- lib/ClangImporter/ClangImporter.cpp | 22 ++++++++++--------- lib/ClangImporter/ImportType.cpp | 5 ++--- .../objc-correctness/custom-nsoptions.swift | 9 ++++++++ 6 files changed, 27 insertions(+), 20 deletions(-) diff --git a/include/swift/AST/ClangModuleLoader.h b/include/swift/AST/ClangModuleLoader.h index 2125f1f2b7482..46dec5027aafa 100644 --- a/include/swift/AST/ClangModuleLoader.h +++ b/include/swift/AST/ClangModuleLoader.h @@ -306,9 +306,6 @@ class ClangModuleLoader : public ModuleLoader { virtual EffectiveClangContext getEffectiveClangContext( const NominalTypeDecl *nominal) = 0; - virtual const clang::TypedefType * - getTypeDefForCXXCFOptionsDefinition(const clang::Decl *candidateDecl) = 0; - virtual SourceLoc importSourceLocation(clang::SourceLocation loc) = 0; }; diff --git a/include/swift/ClangImporter/ClangImporter.h b/include/swift/ClangImporter/ClangImporter.h index beb51d210bf41..def5610e50f11 100644 --- a/include/swift/ClangImporter/ClangImporter.h +++ b/include/swift/ClangImporter/ClangImporter.h @@ -603,8 +603,8 @@ class ClangImporter final : public ClangModuleLoader { /// Enable the symbolic import experimental feature for the given callback. void withSymbolicFeatureEnabled(llvm::function_ref callback); - const clang::TypedefType *getTypeDefForCXXCFOptionsDefinition( - const clang::Decl *candidateDecl) override; + static const clang::TypedefType *getTypedefForCXXCFOptionsDefinition( + const clang::Decl *candidateDecl, const ASTContext &ctx); SourceLoc importSourceLocation(clang::SourceLocation loc) override; }; diff --git a/lib/AST/ASTMangler.cpp b/lib/AST/ASTMangler.cpp index 21495a180eacd..6bea7ffacdda5 100644 --- a/lib/AST/ASTMangler.cpp +++ b/lib/AST/ASTMangler.cpp @@ -2541,8 +2541,8 @@ ASTMangler::getTypeDefForCXXCFOptionsDefinition(const ValueDecl *decl) { if (!clangDecl) return nullptr; - const auto &clangModuleLoader = decl->getASTContext().getClangModuleLoader(); - return clangModuleLoader->getTypeDefForCXXCFOptionsDefinition(clangDecl); + auto &ctx = decl->getASTContext(); + return ClangImporter::getTypedefForCXXCFOptionsDefinition(clangDecl, ctx); } const clang::NamedDecl * diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index 5fa4ce58db30d..7ac43a9713afd 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -7022,26 +7022,23 @@ void ClangImporter::withSymbolicFeatureEnabled( oldImportSymbolicCXXDecls.get()); } -const clang::TypedefType *ClangImporter::getTypeDefForCXXCFOptionsDefinition( - const clang::Decl *candidateDecl) { - - if (!Impl.SwiftContext.LangOpts.EnableCXXInterop) +const clang::TypedefType *ClangImporter::getTypedefForCXXCFOptionsDefinition( + const clang::Decl *candidateDecl, const ASTContext &ctx) { + if (!ctx.LangOpts.EnableCXXInterop) return nullptr; auto enumDecl = dyn_cast(candidateDecl); if (!enumDecl) return nullptr; - if (!enumDecl->getDeclName().isEmpty()) return nullptr; const clang::ElaboratedType *elaboratedType = - dyn_cast(enumDecl->getIntegerType().getTypePtr()); + enumDecl->getIntegerType()->getAs(); if (auto typedefType = elaboratedType ? dyn_cast(elaboratedType->desugar()) - : dyn_cast( - enumDecl->getIntegerType().getTypePtr())) { + : enumDecl->getIntegerType()->getAs()) { auto enumExtensibilityAttr = elaboratedType ? enumDecl->getAttr() @@ -7054,8 +7051,13 @@ const clang::TypedefType *ClangImporter::getTypeDefForCXXCFOptionsDefinition( enumExtensibilityAttr->getExtensibility() == clang::EnumExtensibilityAttr::Open && hasFlagEnumAttr) { - return Impl.isUnavailableInSwift(typedefType->getDecl()) ? typedefType - : nullptr; + // Make sure the typedef is marked as unavailable in Swift. + auto typedefDecl = typedefType->getDecl(); + for (auto *attr : + typedefDecl->specific_attrs()) { + if (attr->getPlatform()->getName() == "swift") + return typedefType; + } } } diff --git a/lib/ClangImporter/ImportType.cpp b/lib/ClangImporter/ImportType.cpp index b8540d3e12607..80dd134733c60 100644 --- a/lib/ClangImporter/ImportType.cpp +++ b/lib/ClangImporter/ImportType.cpp @@ -2656,9 +2656,8 @@ ArgumentAttrs ClangImporter::Implementation::inferDefaultArgument( if (declIter != declsInContext.end()) { if (auto enumDecl = dyn_cast(*declIter)) { if (auto cfOptionsTy = - nameImporter.getContext() - .getClangModuleLoader() - ->getTypeDefForCXXCFOptionsDefinition(enumDecl)) { + ClangImporter::getTypedefForCXXCFOptionsDefinition( + enumDecl, nameImporter.getContext())) { if (cfOptionsTy->getDecl() == typedefDecl) { auto enumName = typedefDecl->getName(); ArgumentAttrs argumentAttrs(DefaultArgumentKind::None, true, diff --git a/test/Interop/Cxx/objc-correctness/custom-nsoptions.swift b/test/Interop/Cxx/objc-correctness/custom-nsoptions.swift index 6c5e398643d10..b91ab7d4c8d64 100644 --- a/test/Interop/Cxx/objc-correctness/custom-nsoptions.swift +++ b/test/Interop/Cxx/objc-correctness/custom-nsoptions.swift @@ -1,7 +1,16 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t/pch + // RUN: %target-typecheck-verify-swift -verify-ignore-unknown -I %S/Inputs -enable-objc-interop -enable-experimental-cxx-interop + +// RUN: %target-swift-frontend -emit-pch -enable-objc-interop -enable-experimental-cxx-interop -o %t/pch/customNSOptions.pch %S/Inputs/customNSOptions.h +// RUN: %target-typecheck-verify-swift -D BRIDGING_HEADER -I %S/Inputs -import-objc-header %t/pch/customNSOptions.pch -enable-objc-interop -enable-experimental-cxx-interop %s + // REQUIRES: objc_interop +#if !BRIDGING_HEADER import CustomNSOptions +#endif let flags1: MyControlFlags = [] let flags2: MyControlFlags = [.first]