-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[cxx-interop] Avoid copies when accessing pointee #82480
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?
Conversation
16f625f
to
344db5a
Compare
Previously, we would get two copies, one accessing the pointee and one when we pass the pointee as a method as the implicit self argument. These copies are unsafe as they might introduce slicing. When addressable paramaters features are enabled, we no longer make these copies for the standard STL types. Custom smart pointers can replicate this by making the lifetime dependency between the implicit object parameter and the returned reference of operator* explicit via a lifetime annotation. rdar://154213694&128293252&112690482
344db5a
to
7474a51
Compare
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.
Thanks!
I'm not super familiar with the addressable attribute, so would be good for an expert to take a look as well.
if (enclosing->isInStdNamespace() && | ||
(enclosing->getName() == "unique_ptr" || | ||
enclosing->getName() == "shared_ptr") && | ||
method->isOverloadedOperator() && | ||
method->getOverloadedOperator() == clang::OO_Star) { |
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.
Ideally we would use API Notes to inject this, but that isn't possible right now since we don't support annotating C++ operators in API Notes, so this looks good.
@@ -0,0 +1,41 @@ | |||
// RUN: %target-run-simple-swift(-I %S/Inputs -cxx-interoperability-mode=upcoming-swift -enable-experimental-feature AddressableParameters) |
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 don't have a strong preference here, but I'm curious to hear your thoughts around pros and cons of a run time test vs checking SIL or IR output
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 runtime checks are less fragile. SIL or the LLVM IR can break easily when some passes or the IR changes. Runtime tests might be a bit less performant but I think in this case they are more directly test what we want here because we can make copies observable and directly observe it. It is not always possible to observe the property we want to test at runtime, SIL or IR tests are great for that. Also, I think here we do not care about the shape of the IRs, just the fact that there are no copies. I guess one caveat is, the runtime test does not tell us why are there no copies? Is it because there were no copies to begin with or is it because some optimisations got rid of them?
Despite this caveat, I prefer a runtime test in this case as I find it easier to read and write and less fragile in this case.
@swift-ci please smoke test |
Previously, we would get two copies, one accessing the pointee and one when we pass the pointee as a method as the implicit self argument. These copies are unsafe as they might introduce slicing. When addressable paramaters features are enabled, we no longer make these copies for the standard STL types. Custom smart pointers can replicate this by making the lifetime dependency between the implicit object parameter and the returned reference of operator* explicit via a lifetime annotation.
rdar://154213694&128293252&112690482