-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[cxx-interop] Prevent crash when importing a std::optional of nonescapable #83973
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
[cxx-interop] Prevent crash when importing a std::optional of nonescapable #83973
Conversation
@swift-ci please smoke test |
ed52618
to
bae381f
Compare
@swift-ci please smoke test |
bae381f
to
1bea41b
Compare
@swift-ci please smoke test |
|
||
auto *vd = cast<VarDecl>(member); | ||
if (!isNonEscapable) { | ||
if (const auto *fd = dyn_cast<clang::FieldDecl>(nd)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-escapable is viral to the parents, so we should expect the parent record to be non-escapable when an indirect field is non-escapable. Would doing the same thing for both direct and indirect fields run into problems?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point, thanks. We can do the same thing for indirect fields by assuming that an anonymous union/struct can never be escapable, and thus we can compare the escapability of these indirect fields directly with the outer record (i.e. skipping the comparison with the anonymous record)
1bea41b
to
d0ec83c
Compare
d0ec83c
to
1bfa16a
Compare
@swift-ci please smoke test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG, thanks!
} | ||
|
||
// CHECK: error: a function with a ~Escapable result needs a parameter to depend on | ||
// CHECK-NO-LIFETIMES: test.swift:11:32: error: a function cannot return a ~Escapable result |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can make these line numbers resilient to this type of cascading change using:
// CHECK-NO-LIFETIMES: test.swift:[[@LINE+1]]:32: error: a function cannot return a ~Escapable result
By default this would be broken by split-files
if you use FileCheck %s
, since the line numbers change. This can be resolved by either using Filecheck %t/test.swift
, or calling split-files --leading-lines
which pads each file to preserve line numbers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh interesting, thanks!
When we tried to create a
std::optional
of a nonescapable type in Swift, the compiler would run into an assertion failure. This is becausestd::optional
has an anonymous union where one of its members would be of this nonescapable type.Turns out that we were not handling anonymous unions with nonescapable members correctly. Fields of anonymous unions are injected to the parent as
IndirectFieldDecl
, so we need to handle these indirect fields the same way we handle "normal" fields.rdar://156704699