Skip to content

[cxx-interop] Import custom NS_OPTIONS correctly #67865

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 24 additions & 37 deletions lib/ClangImporter/ImportType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2646,44 +2646,31 @@ ArgumentAttrs ClangImporter::Implementation::inferDefaultArgument(
}
} else if (const clang::TypedefType *typedefType =
type->getAs<clang::TypedefType>()) {
// 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<clang::EnumDecl>(*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"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I don't entirely understand the change here. This seems like a tightening of the rules, before we only checked for an unavailable typedef (which seems insufficient), and now we check for the anonymous enum next to it having the right attributes and matching value names. From the description I thought the idea was to relax the requirements here?

Aside from that, don't we still need the other cases here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're relaxing the requirements imposed on the name of the NS_OPTIONS type. Previously we were only detecting type names ending with certain words here, now we're ignoring the type name (since custom frameworks can declare NS_OPTIONS types with arbitrary names) and relying on the AST structure of an NS_OPTIONS expansion.
So the type name detection cases were intentionally removed.

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;
}
}
}
Expand Down
60 changes: 39 additions & 21 deletions test/Interop/Cxx/enum/Inputs/c-enums-withOptions-omit.h
Original file line number Diff line number Diff line change
@@ -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
};
Expand Down
6 changes: 6 additions & 0 deletions test/Interop/Cxx/objc-correctness/Inputs/customNSOptions.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#include "CFOptions.h"

typedef CF_OPTIONS(unsigned, MyControlFlags) {
MyControlFlagsNone = 0,
MyControlFlagsFirst
};
4 changes: 4 additions & 0 deletions test/Interop/Cxx/objc-correctness/Inputs/module.modulemap
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ module CxxClassWithNSStringInit {
requires cplusplus
}

module CustomNSOptions {
header "customNSOptions.h"
}

module NSOptionsMangling {
header "NSOptionsMangling.h"
}
Expand Down
7 changes: 7 additions & 0 deletions test/Interop/Cxx/objc-correctness/custom-nsoptions.swift
Original file line number Diff line number Diff line change
@@ -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]