Skip to content

Commit a472fa6

Browse files
susmonteirosusmonteiro
committed
[cxx-interop] Emit diagnostic to mark C++ type as non-copyable
1 parent 35fab79 commit a472fa6

File tree

4 files changed

+198
-2
lines changed

4 files changed

+198
-2
lines changed

‎include/swift/AST/DiagnosticsIRGen.def‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,5 +80,37 @@ ERROR(attr_objc_implementation_resilient_property_deployment_target, none,
8080
ERROR(unable_to_load_pass_plugin,none,
8181
"unable to load plugin '%0': '%1'", (StringRef, StringRef))
8282

83+
ERROR(failed_emit_copy, none,
84+
"failed to copy %0; did you mean to import %0 as ~Copyable?",
85+
(const clang::NamedDecl *))
86+
87+
NOTE(use_requires_expression, none,
88+
"use 'requires' (since C++20) to specify the constraints under which the "
89+
"copy "
90+
"constructor is available",
91+
())
92+
93+
NOTE(annotate_copyable_if, none,
94+
"annotate a type with 'SWIFT_COPYABLE_IF(<T>)' in C++ to specify "
95+
"that the type is Copyable if <T> is Copyable",
96+
())
97+
98+
NOTE(annotate_non_copyable, none,
99+
"annotate a type with 'SWIFT_NONCOPYABLE' in C++ to import it as "
100+
"~Copyable",
101+
())
102+
103+
NOTE(maybe_missing_annotation, none,
104+
"one of the types that %0 depends on may need a 'requires' clause (since "
105+
"C++20) in the copy constructor, a 'SWIFT_COPYABLE_IF' annotation or a "
106+
"'SWIFT_NONCOPYABLE' annotation'",
107+
(const clang::NamedDecl *))
108+
109+
NOTE(maybe_missing_parameter, none,
110+
"the %select{'requires' clause on the copy constructor "
111+
"of|'SWIFT_COPYABLE_IF' annotation on}0 %1 may be missing a "
112+
"%select{constraint|parameter}0",
113+
(bool, const clang::NamedDecl *))
114+
83115
#define UNDEFINE_DIAGNOSTIC_MACROS
84116
#include "DefineDiagnosticMacros.h"

‎lib/IRGen/GenStruct.cpp‎

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "swift/AST/SubstitutionMap.h"
2828
#include "swift/AST/Types.h"
2929
#include "swift/Basic/Assertions.h"
30+
#include "swift/ClangImporter/ClangImporter.h"
3031
#include "swift/IRGen/Linking.h"
3132
#include "swift/SIL/SILFunctionBuilder.h"
3233
#include "swift/SIL/SILModule.h"
@@ -556,7 +557,8 @@ namespace {
556557
if (ctor->isCopyConstructor() &&
557558
// FIXME: Support default arguments (rdar://142414553)
558559
ctor->getNumParams() == 1 &&
559-
ctor->getAccess() == clang::AS_public && !ctor->isDeleted())
560+
ctor->getAccess() == clang::AS_public && !ctor->isDeleted() &&
561+
!ctor->isIneligibleOrNotSelected())
560562
return ctor;
561563
}
562564
return nullptr;
@@ -570,7 +572,8 @@ namespace {
570572
if (ctor->isMoveConstructor() &&
571573
// FIXME: Support default arguments (rdar://142414553)
572574
ctor->getNumParams() == 1 &&
573-
ctor->getAccess() == clang::AS_public && !ctor->isDeleted())
575+
ctor->getAccess() == clang::AS_public && !ctor->isDeleted() &&
576+
!ctor->isIneligibleOrNotSelected())
574577
return ctor;
575578
}
576579
return nullptr;
@@ -623,8 +626,54 @@ namespace {
623626
auto fnType = createCXXCopyConstructorFunctionType(IGF, T);
624627
auto globalDecl =
625628
clang::GlobalDecl(copyConstructor, clang::Ctor_Complete);
629+
630+
auto &ctx = IGF.IGM.Context;
631+
auto *importer = static_cast<ClangImporter *>(ctx.getClangModuleLoader());
632+
633+
auto &diagEngine = importer->getClangSema().getDiagnostics();
634+
clang::DiagnosticErrorTrap trap(diagEngine);
626635
auto clangFnAddr =
627636
IGF.IGM.getAddrOfClangGlobalDecl(globalDecl, NotForDefinition);
637+
638+
if (trap.hasErrorOccurred()) {
639+
SourceLoc copyConstructorLoc =
640+
importer->importSourceLocation(copyConstructor->getLocation());
641+
auto *recordDecl = copyConstructor->getParent();
642+
ctx.Diags.diagnose(copyConstructorLoc, diag::failed_emit_copy,
643+
recordDecl);
644+
645+
bool hasCopyableIfAttr =
646+
recordDecl->hasAttrs() &&
647+
llvm::any_of(recordDecl->getAttrs(), [&](clang::Attr *attr) {
648+
if (auto swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr)) {
649+
StringRef attrStr = swiftAttr->getAttribute();
650+
assert(!attrStr.starts_with("~Copyable") &&
651+
"Trying to emit copy of a type annotated with "
652+
"'SWIFT_NONCOPYABLE'?");
653+
if (attrStr.starts_with("copyable_if:"))
654+
return true;
655+
}
656+
return false;
657+
});
658+
659+
bool hasRequiresClause =
660+
copyConstructor->getTrailingRequiresClause() != nullptr;
661+
662+
if (hasRequiresClause || hasCopyableIfAttr) {
663+
ctx.Diags.diagnose(copyConstructorLoc, diag::maybe_missing_annotation,
664+
recordDecl);
665+
ctx.Diags.diagnose(copyConstructorLoc, diag::maybe_missing_parameter,
666+
hasCopyableIfAttr, recordDecl);
667+
} else {
668+
ctx.Diags.diagnose(copyConstructorLoc, diag::use_requires_expression);
669+
ctx.Diags.diagnose(copyConstructorLoc, diag::annotate_copyable_if);
670+
}
671+
672+
if (!copyConstructor->isUserProvided()) {
673+
ctx.Diags.diagnose(copyConstructorLoc, diag::annotate_non_copyable);
674+
}
675+
}
676+
628677
auto callee = cast<llvm::Function>(clangFnAddr->stripPointerCasts());
629678
Signature signature = IGF.IGM.getSignature(fnType, copyConstructor);
630679
std::string name = "__swift_cxx_copy_ctor" + callee->getName().str();

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

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: split-file %s %t
3+
// RUN: %target-swift-frontend -cxx-interoperability-mode=default -emit-ir -I %swift_src_root/lib/ClangImporter/SwiftBridging -I %t%{fs-sep}Inputs %t%{fs-sep}test.swift -Xcc -fignore-exceptions -verify -verify-additional-file %t%{fs-sep}Inputs%{fs-sep}noncopyable.h -verify-additional-prefix TEST1- -D TEST1
4+
// RUN: %target-swift-frontend -cxx-interoperability-mode=default -emit-ir -I %swift_src_root/lib/ClangImporter/SwiftBridging -I %t%{fs-sep}Inputs %t%{fs-sep}test.swift -Xcc -fignore-exceptions -verify -verify-additional-file %t%{fs-sep}Inputs%{fs-sep}noncopyable.h -verify-additional-prefix TEST2- -D TEST2
5+
// RUN: %target-swift-frontend -cxx-interoperability-mode=default -emit-ir -I %swift_src_root/lib/ClangImporter/SwiftBridging -I %t%{fs-sep}Inputs %t%{fs-sep}test.swift -Xcc -fignore-exceptions -verify -verify-additional-file %t%{fs-sep}Inputs%{fs-sep}noncopyable.h -verify-additional-prefix TEST3- -D TEST3
6+
// RUN: %target-swift-frontend -cxx-interoperability-mode=default -emit-ir -I %swift_src_root/lib/ClangImporter/SwiftBridging -I %t%{fs-sep}Inputs %t%{fs-sep}test.swift -Xcc -fignore-exceptions -verify -verify-additional-file %t%{fs-sep}Inputs%{fs-sep}noncopyable.h -verify-additional-prefix TEST4- -D TEST4 -Xcc -std=c++20
7+
8+
//--- Inputs/module.modulemap
9+
module Test {
10+
header "noncopyable.h"
11+
requires cplusplus
12+
}
13+
14+
//--- Inputs/noncopyable.h
15+
#include "swift/bridging"
16+
#include <string>
17+
18+
struct NonCopyable {
19+
NonCopyable() = default;
20+
NonCopyable(const NonCopyable& other) = delete; // expected-note {{'NonCopyable' has been explicitly marked deleted here}}
21+
NonCopyable(NonCopyable&& other) = default;
22+
};
23+
24+
template <typename T>
25+
struct OwnsT {
26+
T element;
27+
OwnsT() {}
28+
OwnsT(const OwnsT &other) : element(other.element) {}
29+
// expected-error@-1 *{{call to deleted constructor of 'NonCopyable'}}
30+
// expected-note@-2 *{{in instantiation of member function 'OwnsT<NonCopyable>::OwnsT' requested here}}
31+
// expected-TEST1-error@-3 {{failed to copy 'OwnsT<NonCopyable>'; did you mean to import 'OwnsT<NonCopyable>' as ~Copyable?}}
32+
// expected-TEST1-note@-4 {{use 'requires' (since C++20) to specify the constraints under which the copy constructor is available}}
33+
// 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}}
34+
OwnsT(OwnsT&& other) {}
35+
};
36+
37+
using OwnsNonCopyable = OwnsT<NonCopyable>;
38+
39+
template <typename T> struct Derived : OwnsT<T> {
40+
// expected-TEST2-error@-1 {{failed to copy 'Derived<NonCopyable>'; did you mean to import 'Derived<NonCopyable>' as ~Copyable?}}
41+
// expected-TEST2-note@-2 {{use 'requires' (since C++20) to specify the constraints under which the copy constructor is available}}
42+
// expected-TEST2-note@-3 {{annotate a type with 'SWIFT_COPYABLE_IF(<T>)' in C++ to specify that the type is Copyable if <T> is Copyable}}
43+
// expected-TEST2-note@-4 {{annotate a type with 'SWIFT_NONCOPYABLE' in C++ to import it as ~Copyable}}
44+
};
45+
46+
using DerivedNonCopyable = Derived<NonCopyable>;
47+
48+
template <typename T> struct SWIFT_COPYABLE_IF(T) Annotated {
49+
T element;
50+
Annotated() : element() {}
51+
Annotated(const Annotated &other) : element(other.element) {}
52+
// expected-TEST3-error@-1 {{failed to copy 'Annotated<OwnsT<NonCopyable>>'; did you mean to import 'Annotated<OwnsT<NonCopyable>>' as ~Copyable?}}
53+
// expected-TEST3-note@-2 {{one of the types that 'Annotated<OwnsT<NonCopyable>>' depends on may need a 'requires' clause (since C++20) in the copy constructor, a 'SWIFT_COPYABLE_IF' annotation or a 'SWIFT_NONCOPYABLE' annotation'}}
54+
// expected-TEST3-note@-3 {{the 'SWIFT_COPYABLE_IF' annotation on 'Annotated<OwnsT<NonCopyable>>' may be missing a parameter}}
55+
56+
Annotated(Annotated &&) = default;
57+
};
58+
59+
using AnnotatedOwnsNonCopyable = Annotated<OwnsT<NonCopyable>>;
60+
61+
#if __cplusplus >= 202002L
62+
template <typename T> struct Requires {
63+
T element;
64+
Requires() : element() {}
65+
Requires(const Requires &other) requires std::is_copy_constructible_v<T> : element(other.element) {}
66+
// expected-TEST4-error@-1 {{failed to copy 'Requires<OwnsT<NonCopyable>>'; did you mean to import 'Requires<OwnsT<NonCopyable>>' as ~Copyable?}}
67+
// expected-TEST4-note@-2 {{one of the types that 'Requires<OwnsT<NonCopyable>>' depends on may need a 'requires' clause (since C++20) in the copy constructor, a 'SWIFT_COPYABLE_IF' annotation or a 'SWIFT_NONCOPYABLE' annotation'}}
68+
// expected-TEST4-note@-3 {{the 'requires' clause on the copy constructor of 'Requires<OwnsT<NonCopyable>>' may be missing a constraint}}
69+
70+
Requires(Requires &&) = default;
71+
};
72+
73+
using RequiresOwnsNonCopyable = Requires<OwnsT<NonCopyable>>;
74+
#endif
75+
76+
//--- test.swift
77+
import Test
78+
import CxxStdlib
79+
80+
func takeCopyable<T: Copyable>(_ x: T) {}
81+
82+
#if TEST1
83+
func simpleTest() {
84+
let s = OwnsNonCopyable()
85+
takeCopyable(s)
86+
}
87+
88+
#elseif TEST2
89+
func derived() {
90+
let s = DerivedNonCopyable()
91+
takeCopyable(s)
92+
}
93+
94+
#elseif TEST3
95+
func annotated() {
96+
let s = AnnotatedOwnsNonCopyable()
97+
// Annotated has a correct SWIFT_COPYABLE_IF annotation, but OwnsT does not
98+
// Since we import OwnsT<NonCopyable> as Copyable (even though it cannot be copy constructible),
99+
// we end up also importing Annotated<OwnsT<NonCopyable>> as Copyable
100+
takeCopyable(s)
101+
}
102+
103+
#elseif TEST4
104+
func requires() {
105+
let s = RequiresOwnsNonCopyable()
106+
// Requires makes use of 'requires' correctly, but OwnsT is missing some information
107+
// Since we import OwnsT<NonCopyable> as Copyable (even though it cannot be copy constructible),
108+
// we end up also importing Requires<OwnsT<NonCopyable>> as Copyable
109+
takeCopyable(s)
110+
}
111+
112+
#endif

‎test/Interop/Cxx/class/noncopyable-typechecker.swift‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
// RUN: %target-swift-frontend -cxx-interoperability-mode=default -typecheck -verify -I %swift_src_root/lib/ClangImporter/SwiftBridging -I %t/Inputs %t/test.swift
44
// RUN: %target-swift-frontend -cxx-interoperability-mode=default -Xcc -std=c++20 -verify-additional-prefix cpp20- -D CPP20 -typecheck -verify -I %swift_src_root/lib/ClangImporter/SwiftBridging -I %t/Inputs %t/test.swift
55

6+
// This test uses -verify-additional-file, which do not work well on Windows.
7+
// UNSUPPORTED: OS=windows-msvc
8+
69
//--- Inputs/module.modulemap
710
module Test {
811
header "noncopyable.h"

0 commit comments

Comments
 (0)