[cxx-interop] Import custom NS_OPTIONS correctly
#67865
Conversation
|
@swift-ci please smoke test |
|
I will take a closer look soon -- I've forgotten the AST structure that we to identify NS_OPTION, so I need to refresh my memory. That said, in #64043 I made some attempt to incapsulate the heuristic used to identify NS_OPTION into a helper method. Looks like its in the mangler, which probably isn't convenient to get to from the ClangImporter. That said, I think we should centralize this NS_OPTION identification heuristic logic somewhere, it appears in 4 or 5 places. |
egorzhdan
force-pushed
the
egorzhdan/cfoptions
branch
from
9fbf8bd to
2250b6d
Compare
| } | ||
| if (camel_case::sameWordIgnoreFirstCase(word, "units")) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
|
@NuriAmari thanks for pointing me towards that review, I'll unify the NS_OPTION type detection logic. |
There's more adhoc versions of this: https://github.com/apple/swift/blob/be3492152e93da17ca4def14c02bfb7229eb93f0/lib/ClangImporter/ImportType.cpp#L2135 . I think every call to |
egorzhdan
force-pushed
the
egorzhdan/cfoptions
branch
2 times, most recently
from
18fcadc to
1755b13
Compare
|
@swift-ci please smoke test |
This changes the heuristic that we use to detect `CF_OPTIONS`/`NS_OPTIONS` instantiations in C++ language mode. Since libraries sometimes declare custom option types that break the assumptions about the names of such types, this lifts the requirements on the type name. rdar://113524090 rdar://113279214
egorzhdan
force-pushed
the
egorzhdan/cfoptions
branch
from
1755b13 to
f20617b
Compare
|
@swift-ci please smoke test |
|
@swift-ci please test Windows |
This changes the heuristic that we use to detect
CF_OPTIONS/NS_OPTIONSinstantiations in C++ language mode.Since libraries sometimes declare custom option types that break the assumptions about the names of such types, this lifts the requirements on the type name.
rdar://113524090
rdar://113279214