-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[cxx-interop] Add support for subscripts of non-copyable types #86125
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
base: main
Are you sure you want to change the base?
[cxx-interop] Add support for subscripts of non-copyable types #86125
Conversation
|
@swift-ci please smoke test |
Xazax-hun
left a comment
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!
egorzhdan
left a comment
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.
Thank you! Just a couple of small questions.
| for (auto ctor : decl->ctors()) { | ||
| if (ctor->isCopyConstructor() && | ||
| // FIXME: Support default arguments (rdar://142414553) | ||
| // FIXME: Support default arguments |
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.
Accidental change?
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.
Doug noticed these radars numbers and asked for them to be removed. Since this is a small change, in a file that this patch touches, I thought it would be reasonable to do it here instead of creating a separate patch just for this. Would you prefer me to do it in a separate patch?
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.
I think it's better to keep it out of this PR, since you're not modifying this code part here.
If you'd like to remove there rdar links, could you please file GitHub issues and link to them in the comments instead? That way someone encountering this FIXME and actually fixing it would remember to close the corresponding issue. If there isn't an issue link, the issue (or rdar) probably won't get closed.
| // FIXME: Support default arguments (rdar://142414553) | ||
| ctor->getNumParams() == 1 && | ||
| ctor->getAccess() == clang::AS_public; | ||
| // FIXME: Support default arguments |
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.
Accidental change?
| if (!useAddress) | ||
| getterDecl->setImplicit(); |
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.
Should we mark this getter as implicit unconditionally?
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.
If we mark an Address or MutableAddress accessor as implicit, then in AbstractStorageDecl::getOpaqueAccessor we return nullptr instead of returning the accessor, which causes the compiler to crash. So I think that these kind of accessors are never supposed to be implicit, though I'm not sure what's the reasoning behind this. We also use this trick in SwiftDeclSynthesizer::makeDereferencedPointeeProperty
| if (!useAddress) | ||
| setterDecl->setImplicit(); |
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.
Ditto
hnrklssn
left a comment
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.
nice
|
@swift-ci please smoke test |
This adds support for reading from and modifying subscripts of non-copyable types, including the subscript of a non-copyable
std::vector.rdar://156698253