Skip to content

Commit 74d76e2

Browse files
zoecarverzoecarver
committed
[cxx-interop] Add diagnostics and validation for retain/release function; require all foreign reference types to provide retain release functions or be immortal.
1 parent c9f54e1 commit 74d76e2

File tree

15 files changed

+380
-93
lines changed

15 files changed

+380
-93
lines changed

‎include/swift/AST/DiagnosticsClangImporter.def

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,27 @@ NOTE(dont_use_iterator_api, none, "C++ method '%0' that returns an unsafe "
170170
"APIs instead",
171171
(StringRef))
172172

173+
ERROR(reference_type_must_have_retain_attr,none,
174+
"reference type '%0' must have 'retain:' swift attribute.", (StringRef))
175+
ERROR(reference_type_must_have_release_attr,none,
176+
"reference type '%0' must have 'release:' swift attribute.", (StringRef))
177+
ERROR(foreign_reference_types_cannot_find_retain,none,
178+
"cannot find retain function '%0' for reference type '%1'.", (StringRef, StringRef))
179+
ERROR(foreign_reference_types_cannot_find_release,none,
180+
"cannot find release function '%0' for reference type '%1'.", (StringRef, StringRef))
181+
ERROR(too_many_reference_type_retain_operations,none,
182+
"too many functions with name '%0'. There must be exactly one retain "
183+
"function for reference type '%1'.", (StringRef, StringRef))
184+
ERROR(too_many_reference_type_release_operations,none,
185+
"too many functions with name '%0'. There must be exactly one release "
186+
"function for reference type '%1'.", (StringRef, StringRef))
187+
ERROR(foreign_reference_types_invalid_retain,none,
188+
"specified retain function '%0' is invalid. Retain must have exactly one "
189+
"argument of type '%1'", (StringRef, StringRef))
190+
ERROR(foreign_reference_types_invalid_release,none,
191+
"specified release function '%0' is invalid. Release must have exactly "
192+
"one argument of type '%1'", (StringRef, StringRef))
193+
173194
NOTE(unsupported_builtin_type, none, "built-in type '%0' not supported", (StringRef))
174195
NOTE(record_field_not_imported, none, "field %0 not imported", (const clang::NamedDecl*))
175196
NOTE(invoked_func_not_imported, none, "function %0 not imported", (const clang::NamedDecl*))

‎include/swift/AST/DiagnosticsIRGen.def

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -59,19 +59,5 @@ ERROR(temporary_allocation_alignment_not_positive,none,
5959
ERROR(temporary_allocation_alignment_not_power_of_2,none,
6060
"alignment value must be a power of two", ())
6161

62-
ERROR(foreign_reference_types_cannot_find_retain,none,
63-
"cannot find retain function '%0'.", (StringRef))
64-
65-
ERROR(foreign_reference_types_cannot_find_release,none,
66-
"cannot find release function '%0'.", (StringRef))
67-
68-
ERROR(foreign_reference_types_invalid_retain,none,
69-
"specified retain function '%0' is invalid. Retain must have exactly one "
70-
"argument of type '%1'", (StringRef, StringRef))
71-
72-
ERROR(foreign_reference_types_invalid_release,none,
73-
"specified release function '%0' is invalid. Release must have exactly "
74-
"one argument of type '%1'", (StringRef, StringRef))
75-
7662
#define UNDEFINE_DIAGNOSTIC_MACROS
7763
#include "DefineDiagnosticMacros.h"

‎include/swift/ClangImporter/ClangImporterRequests.h

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,67 @@ class IsSafeUseOfCxxDecl
285285
bool evaluate(Evaluator &evaluator, SafeUseOfCxxDeclDescriptor desc) const;
286286
};
287287

288+
enum class CustomRefCountingOperationKind { retain, release };
289+
290+
struct CustomRefCountingOperationDescriptor final {
291+
const ClassDecl *decl;
292+
CustomRefCountingOperationKind kind;
293+
294+
CustomRefCountingOperationDescriptor(const ClassDecl *decl,
295+
CustomRefCountingOperationKind kind)
296+
: decl(decl), kind(kind) {}
297+
298+
friend llvm::hash_code hash_value(const CustomRefCountingOperationDescriptor &desc) {
299+
return llvm::hash_combine(desc.decl, desc.kind);
300+
}
301+
302+
friend bool operator==(const CustomRefCountingOperationDescriptor &lhs,
303+
const CustomRefCountingOperationDescriptor &rhs) {
304+
return lhs.decl == rhs.decl && lhs.kind == rhs.kind;
305+
}
306+
307+
friend bool operator!=(const CustomRefCountingOperationDescriptor &lhs,
308+
const CustomRefCountingOperationDescriptor &rhs) {
309+
return !(lhs == rhs);
310+
}
311+
};
312+
313+
void simple_display(llvm::raw_ostream &out, CustomRefCountingOperationDescriptor desc);
314+
SourceLoc extractNearestSourceLoc(CustomRefCountingOperationDescriptor desc);
315+
316+
struct CustomRefCountingOperationResult {
317+
enum CustomRefCountingOperationResultKind {
318+
noAttribute, immortal, notFound, tooManyFound, foundOperation
319+
};
320+
321+
CustomRefCountingOperationResultKind kind;
322+
ValueDecl *operation;
323+
std::string name;
324+
};
325+
326+
class CustomRefCountingOperation
327+
: public SimpleRequest<CustomRefCountingOperation,
328+
CustomRefCountingOperationResult(
329+
CustomRefCountingOperationDescriptor),
330+
RequestFlags::Cached> {
331+
public:
332+
using SimpleRequest::SimpleRequest;
333+
334+
// Caching
335+
bool isCached() const { return true; }
336+
337+
// Source location
338+
SourceLoc getNearestLoc() const { return SourceLoc(); };
339+
340+
private:
341+
friend SimpleRequest;
342+
343+
// Evaluation.
344+
CustomRefCountingOperationResult evaluate(Evaluator &evaluator,
345+
CustomRefCountingOperationDescriptor desc) const;
346+
};
347+
348+
288349
#define SWIFT_TYPEID_ZONE ClangImporter
289350
#define SWIFT_TYPEID_HEADER "swift/ClangImporter/ClangImporterTypeIDZone.def"
290351
#include "swift/Basic/DefineTypeIDZone.h"

‎include/swift/ClangImporter/ClangImporterTypeIDZone.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,3 +30,6 @@ SWIFT_REQUEST(ClangImporter, CxxRecordSemantics,
3030
SWIFT_REQUEST(ClangImporter, IsSafeUseOfCxxDecl,
3131
bool(SafeUseOfCxxRecordDescriptor), Cached,
3232
NoLocationInfo)
33+
SWIFT_REQUEST(ClangImporter, CustomRefCountingOperation,
34+
CustomRefCountingOperationResult(CustomRefCountingOperationDescriptor), Cached,
35+
NoLocationInfo)

‎lib/AST/Decl.cpp

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
#include "swift/AST/SwiftNameTranslation.h"
5050
#include "swift/Basic/Defer.h"
5151
#include "swift/ClangImporter/ClangModule.h"
52+
#include "swift/ClangImporter/ClangImporterRequests.h"
5253
#include "swift/Parse/Lexer.h" // FIXME: Bad dependency
5354
#include "clang/Lex/MacroInfo.h"
5455
#include "llvm/ADT/SmallPtrSet.h"
@@ -5228,25 +5229,9 @@ bool ClassDecl::isForeignReferenceType() const {
52285229
}
52295230

52305231
bool ClassDecl::hasRefCountingAnnotations() const {
5231-
auto decl = dyn_cast_or_null<clang::RecordDecl>(getClangDecl());
5232-
if (!decl)
5233-
return false;
5234-
5235-
bool hasRetain = decl->hasAttrs() && llvm::any_of(decl->getAttrs(),
5236-
[](auto *attr) {
5237-
if (auto swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr))
5238-
return swiftAttr->getAttribute().startswith("retain:");
5239-
return false;
5240-
});
5241-
5242-
bool hasRelease = decl->hasAttrs() && llvm::any_of(decl->getAttrs(),
5243-
[](auto *attr) {
5244-
if (auto swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr))
5245-
return swiftAttr->getAttribute().startswith("release:");
5246-
return false;
5247-
});
5248-
5249-
return hasRetain && hasRelease;
5232+
return evaluateOrDefault(
5233+
getASTContext().evaluator,
5234+
CustomRefCountingOperation({this, CustomRefCountingOperationKind::release}), {}).kind != CustomRefCountingOperationResult::immortal;
52505235
}
52515236

52525237
ReferenceCounting ClassDecl::getObjectModel() const {

‎lib/ClangImporter/ClangImporter.cpp

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6073,4 +6073,56 @@ void swift::simple_display(llvm::raw_ostream &out,
60736073

60746074
SourceLoc swift::extractNearestSourceLoc(SafeUseOfCxxDeclDescriptor desc) {
60756075
return SourceLoc();
6076+
}
6077+
6078+
CustomRefCountingOperationResult CustomRefCountingOperation::evaluate(
6079+
Evaluator &evaluator, CustomRefCountingOperationDescriptor desc) const {
6080+
auto swiftDecl = desc.decl;
6081+
auto operation = desc.kind;
6082+
auto &ctx = swiftDecl->getASTContext();
6083+
6084+
std::string operationStr = operation == CustomRefCountingOperationKind::retain ? "retain:" : "release:";
6085+
6086+
auto decl = cast<clang::RecordDecl>(swiftDecl->getClangDecl());
6087+
if (!decl->hasAttrs())
6088+
return {CustomRefCountingOperationResult::noAttribute, nullptr, ""};
6089+
6090+
auto retainFnAttr = llvm::find_if(decl->getAttrs(),
6091+
[&operationStr](auto *attr) {
6092+
if (auto swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr))
6093+
return swiftAttr->getAttribute().startswith(operationStr);
6094+
return false;
6095+
});
6096+
if (retainFnAttr == decl->getAttrs().end()) {
6097+
return {CustomRefCountingOperationResult::noAttribute, nullptr, ""};
6098+
}
6099+
6100+
auto name = cast<clang::SwiftAttrAttr>(*retainFnAttr)->getAttribute().drop_front(StringRef(operationStr).size()).str();
6101+
6102+
if (name == "immortal")
6103+
return {CustomRefCountingOperationResult::immortal, nullptr, name};
6104+
6105+
llvm::SmallVector<ValueDecl *, 1> results;
6106+
ctx.lookupInModule(swiftDecl->getParentModule(), name, results);
6107+
6108+
if (results.size() == 1)
6109+
return {CustomRefCountingOperationResult::foundOperation, results.front(), name};
6110+
6111+
if (results.empty())
6112+
return {CustomRefCountingOperationResult::notFound, nullptr, name};
6113+
6114+
return {CustomRefCountingOperationResult::tooManyFound, nullptr, name};
6115+
}
6116+
6117+
void swift::simple_display(llvm::raw_ostream &out,
6118+
CustomRefCountingOperationDescriptor desc) {
6119+
out << "Finding custom (foreign reference) reference counting operation '"
6120+
<< (desc.kind == CustomRefCountingOperationKind::retain ? "retain" : "release")
6121+
<< "for '"
6122+
<< desc.decl->getNameStr()
6123+
<< "'.\n";
6124+
}
6125+
6126+
SourceLoc swift::extractNearestSourceLoc(CustomRefCountingOperationDescriptor desc) {
6127+
return SourceLoc();
60766128
}

‎lib/ClangImporter/ImportDecl.cpp

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2424,6 +2424,70 @@ namespace {
24242424
return result;
24252425
}
24262426

2427+
void validateForeignReferenceType(const clang::CXXRecordDecl *decl,
2428+
ClassDecl *classDecl) {
2429+
auto isValidOperation = [&](ValueDecl *operation) -> bool {
2430+
auto operationFn = dyn_cast<FuncDecl>(operation);
2431+
if (!operationFn)
2432+
return false;
2433+
2434+
if (!operationFn->getResultInterfaceType()->isVoid())
2435+
return false;
2436+
2437+
if (operationFn->getParameters()->size() != 1)
2438+
return false;
2439+
2440+
if (operationFn->getParameters()->get(0)->getInterfaceType()->isEqual(classDecl->getInterfaceType()))
2441+
return false;
2442+
2443+
return true;
2444+
};
2445+
2446+
auto retainOperation = evaluateOrDefault(
2447+
Impl.SwiftContext.evaluator,
2448+
CustomRefCountingOperation({classDecl, CustomRefCountingOperationKind::retain}), {});
2449+
if (retainOperation.kind == CustomRefCountingOperationResult::noAttribute) {
2450+
HeaderLoc loc(decl->getLocation());
2451+
Impl.diagnose(loc, diag::reference_type_must_have_retain_attr, decl->getNameAsString());
2452+
} else if (retainOperation.kind == CustomRefCountingOperationResult::notFound) {
2453+
HeaderLoc loc(decl->getLocation());
2454+
Impl.diagnose(loc, diag::foreign_reference_types_cannot_find_retain, retainOperation.name, decl->getNameAsString());
2455+
} else if (retainOperation.kind == CustomRefCountingOperationResult::tooManyFound) {
2456+
HeaderLoc loc(decl->getLocation());
2457+
Impl.diagnose(loc, diag::too_many_reference_type_retain_operations, retainOperation.name, decl->getNameAsString());
2458+
} else if (retainOperation.kind == CustomRefCountingOperationResult::foundOperation) {
2459+
if (!isValidOperation(retainOperation.operation)) {
2460+
HeaderLoc loc(decl->getLocation());
2461+
Impl.diagnose(loc, diag::foreign_reference_types_invalid_retain, retainOperation.name, decl->getNameAsString());
2462+
}
2463+
} else {
2464+
// Nothing to do.
2465+
assert(retainOperation.kind == CustomRefCountingOperationResult::immortal);
2466+
}
2467+
2468+
auto releaseOperation = evaluateOrDefault(
2469+
Impl.SwiftContext.evaluator,
2470+
CustomRefCountingOperation({classDecl, CustomRefCountingOperationKind::release}), {});
2471+
if (releaseOperation.kind == CustomRefCountingOperationResult::noAttribute) {
2472+
HeaderLoc loc(decl->getLocation());
2473+
Impl.diagnose(loc, diag::reference_type_must_have_release_attr, decl->getNameAsString());
2474+
} else if (releaseOperation.kind == CustomRefCountingOperationResult::notFound) {
2475+
HeaderLoc loc(decl->getLocation());
2476+
Impl.diagnose(loc, diag::foreign_reference_types_cannot_find_release, releaseOperation.name, decl->getNameAsString());
2477+
} else if (releaseOperation.kind == CustomRefCountingOperationResult::tooManyFound) {
2478+
HeaderLoc loc(decl->getLocation());
2479+
Impl.diagnose(loc, diag::too_many_reference_type_release_operations, releaseOperation.name, decl->getNameAsString());
2480+
} else if (releaseOperation.kind == CustomRefCountingOperationResult::foundOperation) {
2481+
if (!isValidOperation(releaseOperation.operation)) {
2482+
HeaderLoc loc(decl->getLocation());
2483+
Impl.diagnose(loc, diag::foreign_reference_types_invalid_release, releaseOperation.name, decl->getNameAsString());
2484+
}
2485+
} else {
2486+
// Nothing to do.
2487+
assert(releaseOperation.kind == CustomRefCountingOperationResult::immortal);
2488+
}
2489+
}
2490+
24272491
Decl *VisitCXXRecordDecl(const clang::CXXRecordDecl *decl) {
24282492
// This can be called from lldb without C++ interop being enabled: There
24292493
// may be C++ declarations in imported modules, but the interface for
@@ -2504,6 +2568,9 @@ namespace {
25042568

25052569
auto result = VisitRecordDecl(decl);
25062570

2571+
if (auto classDecl = dyn_cast_or_null<ClassDecl>(result))
2572+
validateForeignReferenceType(decl, classDecl);
2573+
25072574
// If this module is declared as a C++ module, try to synthesize
25082575
// conformances to Swift protocols from the Cxx module.
25092576
auto clangModule = decl->getOwningModule();

‎lib/IRGen/ClassTypeInfo.h

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
#include "ClassLayout.h"
2121
#include "HeapTypeInfo.h"
2222

23+
#include "swift/ClangImporter/ClangImporterRequests.h"
24+
2325
namespace swift {
2426
namespace irgen {
2527

@@ -62,8 +64,9 @@ class ClassTypeInfo : public HeapTypeInfo<ClassTypeInfo> {
6264
void emitScalarRelease(IRGenFunction &IGF, llvm::Value *value,
6365
Atomicity atomicity) const override {
6466
if (getReferenceCounting() == ReferenceCounting::Custom) {
65-
auto releaseFn = findForeignReferenceTypeRefCountingOperation(getClass(),
66-
ForeignReferenceTypeRefCountingOperation::release);
67+
auto releaseFn = evaluateOrDefault(
68+
getClass()->getASTContext().evaluator,
69+
CustomRefCountingOperation({getClass(), CustomRefCountingOperationKind::release}), {}).operation;
6770
IGF.emitForeignReferenceTypeLifetimeOperation(releaseFn, value);
6871
return;
6972
}
@@ -74,9 +77,10 @@ class ClassTypeInfo : public HeapTypeInfo<ClassTypeInfo> {
7477
void emitScalarRetain(IRGenFunction &IGF, llvm::Value *value,
7578
Atomicity atomicity) const override {
7679
if (getReferenceCounting() == ReferenceCounting::Custom) {
77-
auto releaseFn = findForeignReferenceTypeRefCountingOperation(getClass(),
78-
ForeignReferenceTypeRefCountingOperation::retain);
79-
IGF.emitForeignReferenceTypeLifetimeOperation(releaseFn, value);
80+
auto retainFn = evaluateOrDefault(
81+
getClass()->getASTContext().evaluator,
82+
CustomRefCountingOperation({getClass(), CustomRefCountingOperationKind::retain}), {}).operation;
83+
IGF.emitForeignReferenceTypeLifetimeOperation(retainFn, value);
8084
return;
8185
}
8286

@@ -89,9 +93,10 @@ class ClassTypeInfo : public HeapTypeInfo<ClassTypeInfo> {
8993
Atomicity atomicity) const override {
9094
if (getReferenceCounting() == ReferenceCounting::Custom) {
9195
llvm::Value *value = e.claimNext();
92-
auto releaseFn = findForeignReferenceTypeRefCountingOperation(getClass(),
93-
ForeignReferenceTypeRefCountingOperation::retain);
94-
IGF.emitForeignReferenceTypeLifetimeOperation(releaseFn, value);
96+
auto retainFn = evaluateOrDefault(
97+
getClass()->getASTContext().evaluator,
98+
CustomRefCountingOperation({getClass(), CustomRefCountingOperationKind::retain}), {}).operation;
99+
IGF.emitForeignReferenceTypeLifetimeOperation(retainFn, value);
95100
return;
96101
}
97102

@@ -102,8 +107,9 @@ class ClassTypeInfo : public HeapTypeInfo<ClassTypeInfo> {
102107
Atomicity atomicity) const override {
103108
if (getReferenceCounting() == ReferenceCounting::Custom) {
104109
llvm::Value *value = e.claimNext();
105-
auto releaseFn = findForeignReferenceTypeRefCountingOperation(getClass(),
106-
ForeignReferenceTypeRefCountingOperation::release);
110+
auto releaseFn = evaluateOrDefault(
111+
getClass()->getASTContext().evaluator,
112+
CustomRefCountingOperation({getClass(), CustomRefCountingOperationKind::release}), {}).operation;
107113
IGF.emitForeignReferenceTypeLifetimeOperation(releaseFn, value);
108114
return;
109115
}

‎lib/IRGen/HeapTypeInfo.h

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -31,33 +31,6 @@
3131
namespace swift {
3232
namespace irgen {
3333

34-
enum class ForeignReferenceTypeRefCountingOperation { retain, release };
35-
36-
static ValueDecl *findForeignReferenceTypeRefCountingOperation(ClassDecl *swiftDecl,
37-
ForeignReferenceTypeRefCountingOperation operation) {
38-
std::string operationStr = operation == ForeignReferenceTypeRefCountingOperation::retain ? "retain:" : "release:";
39-
40-
auto decl = cast<clang::RecordDecl>(swiftDecl->getClangDecl());
41-
assert(decl->hasAttrs());
42-
43-
auto retainFnAttr = llvm::find_if(decl->getAttrs(),
44-
[&operationStr](auto *attr) {
45-
if (auto swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr))
46-
return swiftAttr->getAttribute().startswith(operationStr);
47-
return false;
48-
});
49-
assert(retainFnAttr != decl->getAttrs().end());
50-
51-
auto name = cast<clang::SwiftAttrAttr>(*retainFnAttr)->getAttribute().drop_front(StringRef(operationStr).size());
52-
auto &ctx = swiftDecl->getASTContext();
53-
54-
llvm::SmallVector<ValueDecl *, 1> results;
55-
ctx.lookupInModule(swiftDecl->getParentModule(), name, results);
56-
57-
assert(results.size() == 1);
58-
return results.front();
59-
}
60-
6134
/// \group HeapTypeInfo
6235

6336
/// The kind of 'isa' encoding a heap object uses to reference its heap

0 commit comments

Comments
 (0)