[cxx-interop] Fix assertion failure from unavailable typedef + enum pattern #81625
Conversation
…nction
Also renames it to findOptionSetEnum() to make it a bit clearer at face
value that the returned ImportedType will contain a Swift enum.
Also refactors some nearby instances of
if (auto e = dyn_cast<ElaboratedType>(t))
t = e->desugar();
into a helper function, desugarIfElaborated().
…EnumInfo.cpp It's at home there alongside other ObjC enum-specific logic, rather than in the middle of ImportDecl.cpp (since it isn't directly or exclusively related to importing decls).
|
Draft PR with only NFC commits for now, to run under CI |
|
@swift-ci please test |
|
macOS tests are still hanging but Linux + Windows have passed, so it seems I didn't accidentally break anything with the [NFC] commits. Pushing actual patch + opening the PR now |
The NS_OPTIONS macro sometimes uses a pattern where it loosely associates a typedef with an anonymous enum via a shared underlying integer type (emphasis on "loosely"). The typedef is marked as unavailable in Swift so as to not cause name ambiguity when we associate the anonymous enum with said typedef. We use unavailability as a heuristic during the import process, but that conflates NS_OPTIONS with NS_ENUMs that can be marked as unavailable for entirely unrelated reasons. That in and of itself is fine, because the import logic is general enough to handle both cases, but we have an assertion that seems to be unaware of this scenario, and trips on unavailable NS_ENUMs. (In those cases, the typedef points to the enum rather than the underlying integer type.) This patch fixes the assertion to be resilient against such cases by looking through the enum a typedef refers to. rdar://150399978
j-hui
force-pushed
the
fix-unavailable-enum
branch
from
bea20a2 to
75e4f04
Compare
j-hui
requested review from
zoecarver,
egorzhdan,
beccadax,
ian-twilightcoder,
Xazax-hun,
fahadnayyar and
susmonteiro
as code owners
|
@egorzhdan for what it's worth, I tried explicitly checking for For now, 75e4f04 keeps the behavior consistent with what it was before, and simply adjusts the assertion to accommodate |
The
NS_OPTIONSmacro sometimes uses a pattern where it looselyassociates a typedef with an anonymous enum via a shared underlying
integer type (emphasis on "loosely"). The typedef is marked as
unavailable in Swift so as to not cause name ambiguity when we associate
the anonymous enum with said typedef. We use unavailability as
a heuristic during the import process, but that conflates
NS_OPTIONSwith
NS_ENUMs that can be marked as unavailable for entirely unrelatedreasons.
That in and of itself is fine, because the import logic is general
enough to handle both cases, but we have an assertion that seems to be
unaware of this scenario, and trips on unavailable
NS_ENUMs. (In thosecases, the typedef points to the enum rather than the underlying integer
type.) This patch fixes the assertion to be resilient against such cases
by looking through the enum a typedef refers to.
This PR also includes some NFC commits which consolidate usage of the
findOptionSetEnum()helper function. This pattern (and the failing assertion) was previously duplicated across six distinct locations acrossImportDecl.cppandImportType.cpp. The interface and behavior of this function is based onfindOptionSetType(), which was previously defined inImportDecl.cppbut also used inImportType.cppvia an ad hoc function declaration.Those NFC commits make way for 75e4f04, where the actual fix is contained to a fairly small change.
rdar://150399978