[cxx-interop] Add SWIFT_COPYABLE_IF macro #84461
Conversation
susmonteiro
requested review from
Xazax-hun,
beccadax,
egorzhdan,
fahadnayyar,
hnrklssn,
ian-twilightcoder,
j-hui and
zoecarver
as code owners
|
@swift-ci please smoke test |
| if (result.has_value()) | ||
| return result.value(); | ||
|
|
||
| if (importerImpl) |
There was a problem hiding this comment.
This was a bug in the original code, but in case we figure out early that a type is non-copyable or non-escapable based on checking the first template argument, we will never diagnose that the annotation is not correct for the second template argument. I think this is probably not too bad, but maybe we want to have a rdar or a github issue so we can address this later.
There was a problem hiding this comment.
Fixing the annotation would never change the way we import the type right? i.e. if the first argument determines how we import the type, we will never look at the second one. Since this is an annotation, would it make sense to emit an error on a template argument that will never be used?
| /// Specifies that a C++ `class` or `struct` should be imported as a copyable | ||
| /// Swift value if all of the specified template arguments are copyable. | ||
| #define SWIFT_COPYABLE_IF(...) \ | ||
| __attribute__((swift_attr("copyable_if:" _CXX_INTEROP_CONCAT(__VA_ARGS__)))) |
There was a problem hiding this comment.
I know you're just following along the beaten path, but I believe if we named these attributes such that they follow the actual Swift attribute grammar, we wouldn't have to write custom parsers for each of them. E.g. if this was something like swift_attr("@Copyable(if: " _CXX_INTEROP_CONCAT(__VA_ARGS__) ")" ) the standard Swift attribute parser would be able to parse it. I think we should move over to that approach, to reduce complexity in ClangImporter. Even for existing attributes: since users are using the macro definitions, we can swap the underlying attribute without affecting anyone.
Thoughts?
@susmonteiro @Xazax-hun @egorzhdan @j-hui @fahadnayyar
There was a problem hiding this comment.
I believe we do not actually need to include the @ in swift_attr. I think it is not a bad idea to make it a valid attribute but we should do that in a separate PR that could be reverted independently (just in case).
There was a problem hiding this comment.
Makes sense to me! I agree that it should be a separate PR. Opened an issue: #84486
Refactor `ClangTypeEscapability::evaluate` so that we can reuse part of it for `CxxValueSemantics::evaluate`
susmonteiro
force-pushed
the
susmonteiro/copyable-if-annotation
branch
from
ca8d386 to
f4cf914
Compare
|
@swift-ci please smoke test |
4 participants
SWIFT_COPYABLE_IFis similar toSWIFT_ESCAPABLE_IF, allowing us to import a C++ type as copyable/non-copyable depending on some other types.rdar://158852663