Add a new parameter convention @in_cxx for non-trivial C++ classes that are passed indirectly and destructed by the caller
#73019
@in_cxx for non-trivial C++ classes
that are passed indirectly and destructed by the caller Fix a bug where `@in`, which indicates the callee is responsible for destroying the passed object, was being used to pass such classes. rdar://122707697
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -171,7 +171,7 @@ public struct ParameterInfo : CustomStringConvertible { | |
| switch convention { | ||
| case .indirectIn, .indirectInGuaranteed: | ||
| return hasLoweredAddresses || type.isOpenedExistentialWithError() | ||
| case .indirectInout, .indirectInoutAliasable, .indirectInCXX: | ||
|
There was a problem hiding this comment. I'd like @gottesmm to weigh in here. Naively, I'd say it'd be nice to model these as direct arguments in non-lowered-addresses mode, but the fact that we have to destroy the value in the caller (and observe any mutations made by the callee) might make that impractical. There was a problem hiding this comment. It's potentially useful to have a SIL convention that says: the caller and callee must observe the same address of an addressable value. But that isn't generally implementable for all I'm still baffled by what this convention means. The comment says it's about observing mutation. If that's true, then we should pass a SIL address in non-lowered-addresses mode. But why, then aren't you using If it is not the case that the caller and callee need to observe the same address, then we should pass SIL values instead. There was a problem hiding this comment. I saw an assertion failure when I tried using The failed assertion is in There was a problem hiding this comment. This is the Itanium C++ ABI convention in which non-trivial C++ values are passed indirectly but the caller is responsible for destroying them. The caller and callee do need to observe the same address. It is essentially the same as There was a problem hiding this comment. We should probably pass SIL arguments to Can the callee observe aliases? There was a problem hiding this comment. The parameter object can't be directly named except through the parameter; it's never the same object as e.g. a local variable in the caller. However, it can be aliased in the same way that almost any C++ object technically can, e.g. by escaping There was a problem hiding this comment. It occurs to me that we probably need special logic when consuming parameters like this. They probably should be consumable as long as the type is movable; we should compile consumption as a C++ move and then enforce use restrictions on the moved-from value just like we would for a Swift value, even though technically the object is still there. There was a problem hiding this comment. I think I'm starting to understand. At the source level, this is pass-by-value into the callee, and the caller has no visibility of that value. But, since the convention requires the caller to destroy the value, the caller's implementation must also observe any mutation. Just like Just like OwnershipModelEliminator could insert a |
||
| return true | ||
| case .directOwned, .directUnowned, .directGuaranteed: | ||
| return false | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2161,6 +2161,7 @@ NodePointer Demangler::demangleImplParamConvention(Node::Kind ConvKind) { | |
| case 'l': attr = "@inout"; break; | ||
| case 'b': attr = "@inout_aliasable"; break; | ||
| case 'n': attr = "@in_guaranteed"; break; | ||
| case 'C': attr = "@in_cxx"; break; | ||
|
There was a problem hiding this comment. This should be documented in docs/ABI/Mangling.rst. There was a problem hiding this comment. Also, please add a test in https://github.com/apple/swift/blob/main/test/Demangle/Inputs/manglings.txt |
||
| case 'x': attr = "@owned"; break; | ||
| case 'g': attr = "@guaranteed"; break; | ||
| case 'e': attr = "@deallocating"; break; | ||
|
|
||
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'm not sure what this property is used for, but if it includes both
@inand@in_guaranteed, it probably ought to include@in_cxx.