[cxx-interop] Emit diagnostic to mark C++ type as non-copyable #84520

susmonteiro · 2025-09-25T16:00:02Z

In IRGen, we instantiate the definition of a C++ copy constructor for the first time. If this fails, we get an unrecoverable error from Clang, which doesn't allow us to rollback and try to import the type as ~Copyable.

Instead, we emit a Swift error, on top of the C++ error, suggesting that the user can either add a requires clause, a SWIFT_COPYABLE_IF annotation or a SWIFT_NONCOPYABLE annotation. For the general cause, the Swift diagnostic will look like this:

A(const A &other) : element(other.element) {}
   |   |- error: failed to copy 'A<NonCopyable>'; did you want to import 'A<NonCopyable>' as ~Copyable?
   |   |- note: use 'requires' (since C++20) to specify the constraints under which the copy constructor is available
   |   |- note: annotate a type with 'SWIFT_COPYABLE_IF(<T>)' in C++ to specify that the type is Copyable if <T> is Copyable
   |   `- note: annotate a type with 'SWIFT_NONCOPYABLE' in C++ to import it as ~Copyable

However, if the record already has a SWIFT_COPYABLE_IF annotation or if it's copy constructor has a requires, then we instead suggest that some other type might need more information. Example:

B(const B &other) : element(other.element) {}
   |   |- error: failed to copy 'B<A<NonCopyable>>'; did you want to import 'B<A<NonCopyable>>' as ~Copyable?
   |   |- note: maybe one of the types that 'B<A<NonCopyable>>' depends on needs a 'SWIFT_COPYABLE_IF' annotation
   |   `- note: annotate a type with 'SWIFT_NONCOPYABLE' in C++ to import it as ~Copyable

Would really appreciate feedback and/or suggestions for these diagnostics!

rdar://161169673

susmonteiro · 2025-09-25T16:00:17Z

@swift-ci please smoke test

hnrklssn · 2025-09-25T18:54:25Z

test/Interop/Cxx/class/noncopyable-irgen.swift

+
+using DerivedNonCopyable = Derived<NonCopyable>;
+
+template <typename T> struct SWIFT_COPYABLE_IF(T) Annotated {


I think we should have a note here, saying something like:

template <typename T> struct SWIFT_COPYABLE_IF(T) Annotated { `- note: Annotated<OwnsT<NonCopyable>> is not Copyable because the required type `T` (aka `OwnsT<NonCopyable>`) is not Copyable

(feel free to bikeshed this phrasing btw)

I like this idea, but SWIFT_COPYABLE_IF can take multiple parameters, so we may end up with something like Annotated<OwnsT<NonCopyable>, B, C<D<E>>> is not Copyable because none of the required types 'T1' (aka 'OwnsT<NonCopyable>'), 'T2' (aka 'B'), 'T3' (aka 'C<D<E>>') are ~Copyable

That's fair. It may be simpler to emit those as separate notes, since not all of the required types are necessary to be noncopyable for this. So one note pointing out each type that is noncopyable but needs to be copyable.

Sure, I can do that!

No wait, you mean emitting a note when we import Annotated<OwnsT<NonCopyable>> as Copyable? Or non copyable?

When would this note be emitted? When we try to "copy" a type twice and get an error saying that it cannot be consumed twice, so we explain to the user that actually the type got imported as ~Copyable?

Ah no my bad, I was misunderstanding the SWIFT_COPYABLE_IF attribute. Yeah in the cases we would emit this, every parameter is known to be noncopyable.

test/Interop/Cxx/class/noncopyable-irgen.swift

hnrklssn · 2025-09-25T19:00:20Z

test/Interop/Cxx/class/noncopyable-irgen.swift

+    Annotated() : element() {}
+    Annotated(const Annotated &other) : element(other.element) {}
+    // expected-TEST3-error@-1 {{failed to copy 'Annotated<OwnsT<NonCopyable>>'; did you want to import 'Annotated<OwnsT<NonCopyable>>' as ~Copyable?}}
+    // expected-TEST3-note@-2 {{maybe one of the types that 'Annotated<OwnsT<NonCopyable>>' depends on needs a 'SWIFT_COPYABLE_IF' annotation}}


If we keep this note I think it should be rephrased to something like "one of the types that 'Annotated<OwnsT>' depends on may need a 'SWIFT_COPYABLE_IF' annotation". Ideally I'd want to point out the singular type we depend on, in the case where there's only 1 (as opposed to referring to 1 type using "one of the types"), but that may be unnecessarily complex, idk

I think here we do not know what is the right fix for one of the types that Annotated<OwnsT<NonCopyable>> depends on. That type might miss a requires a SWIFT_COPYABLE_IF or maybe a SWIFT_NONCOPYABLE. Or maybe the SWIFT_COPYABLE_IF is missing a parameter. It is hard to tell. Picking only one in this diagnostic might give too much confidence.

@hnrklssn unfortunately it's very hard to figure out what's the specific type that needs an annotation, so I think we'll have to stick with "one of the types" for now

@Xazax-hun SWIFT_COPYABLE_IF and requires both can be used for conditional copyability, so I assumed that if the user opted for one of these in some parts of the code, probably will always use this option. That's why I filter out the other.

There's also a second note suggesting that SWIFT_NONCOPYABLE can be used.

Having said that, I'm also fine with always showing all the options in the diagnostic

test/Interop/Cxx/class/noncopyable-irgen.swift

hnrklssn · 2025-09-25T19:05:08Z

test/Interop/Cxx/class/noncopyable-irgen.swift

+    // expected-TEST1-error@-3 {{failed to copy 'OwnsT<NonCopyable>'; did you want to import 'OwnsT<NonCopyable>' as ~Copyable?}}
+    // expected-TEST1-note@-4 {{use 'requires' (since C++20) to specify the constraints under which the copy constructor is available}}
+    // expected-TEST1-note@-5 {{annotate a type with 'SWIFT_COPYABLE_IF(<T>)' in C++ to specify that the type is Copyable if <T> is Copyable}}
+    // expected-TEST1-note@-6 {{annotate a type with 'SWIFT_NONCOPYABLE' in C++ to import it as ~Copyable}}


These notes suggesting solutions should have fix-its attached to them

Unfortunately, we cannot easily create high-enough confidence diagnostics to generate fixits because:

In this case it is obvious where is the problem. But in the more general case we cannot yet what is the right place to insert the annotation. For example if we have Foo<A<B>, B> and we failed instantiate the copy ctor of B, what is the right solution? Is A missing a conditional copyable annotation or is B missing a SWIFT_NONCOPYABLE? It is possible A never copies B, in that case it is the latter. Or in case Owns never copies B it might be the latter. We could potentially figure out if we inspect the full stack of instantiations that lead to the error but this is a non-trivial and is way beyond the scope of this patch as that information is not readily available here. Also, did we want to annotate that class or one of the base classes?

I think currently we don't have a good way to present multiple alternative fixes. It is not easy to figure out if the right annotation is SWIFT_NONCOPYABLE or SWIFT_COPYABLE_IF(<T>), or some requires clause.

Fair enough on the complexity part, but I was under the impression that we can present multiple alternative fixes, as long as they are each attached to a separate note diagnostic.

Xazax-hun · 2025-09-26T11:11:07Z

test/Interop/Cxx/class/noncopyable-irgen.swift

+    // expected-TEST1-error@-3 {{failed to copy 'OwnsT<NonCopyable>'; did you want to import 'OwnsT<NonCopyable>' as ~Copyable?}}
+    // expected-TEST1-note@-4 {{use 'requires' (since C++20) to specify the constraints under which the copy constructor is available}}
+    // expected-TEST1-note@-5 {{annotate a type with 'SWIFT_COPYABLE_IF(<T>)' in C++ to specify that the type is Copyable if <T> is Copyable}}
+    // expected-TEST1-note@-6 {{annotate a type with 'SWIFT_NONCOPYABLE' in C++ to import it as ~Copyable}}


Unfortunately, we cannot easily create high-enough confidence diagnostics to generate fixits because:

In this case it is obvious where is the problem. But in the more general case we cannot yet what is the right place to insert the annotation. For example if we have Foo<A<B>, B> and we failed instantiate the copy ctor of B, what is the right solution? Is A missing a conditional copyable annotation or is B missing a SWIFT_NONCOPYABLE? It is possible A never copies B, in that case it is the latter. Or in case Owns never copies B it might be the latter. We could potentially figure out if we inspect the full stack of instantiations that lead to the error but this is a non-trivial and is way beyond the scope of this patch as that information is not readily available here. Also, did we want to annotate that class or one of the base classes?

I think currently we don't have a good way to present multiple alternative fixes. It is not easy to figure out if the right annotation is SWIFT_NONCOPYABLE or SWIFT_COPYABLE_IF(<T>), or some requires clause.

Xazax-hun · 2025-09-26T11:15:57Z

lib/IRGen/GenStruct.cpp

+                             recordDecl, "'SWIFT_COPYABLE_IF' annotation");
+        } else {
+          ctx.Diags.diagnose(copyConstructorLoc, diag::use_requires_expression);
+          ctx.Diags.diagnose(copyConstructorLoc, diag::annotate_copyable_if);


SWIFT_COPYABLE_IF only makes sense to annotate templated types (or types nested in templated types). Do we ever suggest this annotation for non-templated types?

I don't think we do, but I'll double check.
It's also the case for requires though, isn't it?

Yeah, I think so. Same for requires.

Xazax-hun · 2025-09-26T11:17:14Z

lib/IRGen/GenStruct.cpp

+            });
+
+        if (copyConstructor->getTrailingRequiresClause()) {
+          ctx.Diags.diagnose(copyConstructorLoc, diag::maybe_missing_annotation,


Maybe if the code is compiled in pre-c++20 standard we should not suggest using requires?

My thinking was: might be still worth it to make people aware that requires exists from C++20, in case they don't mind updating. But probably won't be very useful in most cases.

I don't feel strongly, so don't mind removing this in pre-c++20

Xazax-hun · 2025-09-26T11:21:46Z

test/Interop/Cxx/class/noncopyable-irgen.swift

+    Annotated() : element() {}
+    Annotated(const Annotated &other) : element(other.element) {}
+    // expected-TEST3-error@-1 {{failed to copy 'Annotated<OwnsT<NonCopyable>>'; did you want to import 'Annotated<OwnsT<NonCopyable>>' as ~Copyable?}}
+    // expected-TEST3-note@-2 {{maybe one of the types that 'Annotated<OwnsT<NonCopyable>>' depends on needs a 'SWIFT_COPYABLE_IF' annotation}}


I think here we do not know what is the right fix for one of the types that Annotated<OwnsT<NonCopyable>> depends on. That type might miss a requires a SWIFT_COPYABLE_IF or maybe a SWIFT_NONCOPYABLE. Or maybe the SWIFT_COPYABLE_IF is missing a parameter. It is hard to tell. Picking only one in this diagnostic might give too much confidence.

test/Interop/Cxx/class/noncopyable-irgen.swift

Xazax-hun

Thanks! This will be a good UX improvement on the status quo.

susmonteiro · 2025-09-26T14:35:21Z

@swift-ci please smoke test

susmonteiro requested a review from zoecarver as a code owner September 25, 2025 16:00

susmonteiro added the c++ interop Feature: Interoperability with C++ label Sep 25, 2025

susmonteiro requested review from egorzhdan, Xazax-hun, j-hui, fahadnayyar, hnrklssn and rjmccall as code owners September 25, 2025 16:00

hnrklssn reviewed Sep 25, 2025

View reviewed changes

test/Interop/Cxx/class/noncopyable-irgen.swift Outdated Show resolved Hide resolved

hnrklssn reviewed Sep 25, 2025

View reviewed changes

test/Interop/Cxx/class/noncopyable-irgen.swift Outdated Show resolved Hide resolved

hnrklssn reviewed Sep 25, 2025

View reviewed changes

Xazax-hun reviewed Sep 26, 2025

View reviewed changes

susmonteiro added 2 commits September 26, 2025 13:12

[cxx-interop] Emit diagnostic to mark C++ type as non-copyable

f5ff400

[cxx-interop] Implemented feedback on the diagnostics

633230d

susmonteiro force-pushed the susmonteiro/noncopyable-diagnostic branch from ed5147d to 633230d Compare September 26, 2025 12:54

Xazax-hun approved these changes Sep 26, 2025

View reviewed changes


		using DerivedNonCopyable = Derived<NonCopyable>;

		template <typename T> struct SWIFT_COPYABLE_IF(T) Annotated {

[cxx-interop] Emit diagnostic to mark C++ type as non-copyable #84520

Are you sure you want to change the base?

[cxx-interop] Emit diagnostic to mark C++ type as non-copyable #84520

Conversation

susmonteiro commented Sep 25, 2025

Uh oh!

susmonteiro commented Sep 25, 2025

Uh oh!

hnrklssn Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

susmonteiro Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Xazax-hun left a comment

Choose a reason for hiding this comment

Uh oh!

susmonteiro commented Sep 26, 2025

Uh oh!

Uh oh!

hnrklssn Sep 25, 2025 •

edited

Loading

susmonteiro Sep 26, 2025 •

edited

Loading