Skip to content

Commit 63135e8

Browse files
DougGregorDougGregor
committed
[Clang importer] Diagnose various issues with the new "destroy:"
When we cannot respect the "destroy:" annotation, mark the type as deprecated with a message thst says why there is a problem. There are various potential problems: * Multiple conflicting destroy functions * Destroy functions that don't meet the pattern * Type isn't imported as a move-only type * Type has a non-trivial destructor (in C++)
1 parent 0821009 commit 63135e8

File tree

5 files changed

+242
-94
lines changed

5 files changed

+242
-94
lines changed

‎lib/ClangImporter/ImportDecl.cpp‎

Lines changed: 2 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -2687,103 +2687,13 @@ namespace {
26872687
}
26882688
}
26892689

2690-
// A type imported as noncopyable, check whether an explicit deinit
2691-
// should be introduced to call a user-defined "destroy" function.
2692-
if (recordHasMoveOnlySemantics(decl)) {
2693-
addExplicitDeinitIfRequired(result, decl);
2694-
}
2690+
// If we need it, add an explicit "deinit" to this type.
2691+
synthesizer.addExplicitDeinitIfRequired(result, decl);
26952692

26962693
result->setMemberLoader(&Impl, 0);
26972694
return result;
26982695
}
26992696

2700-
/// Find an explicitly-provided "destroy" operation specified for the
2701-
/// given Clang type and return it.
2702-
static FuncDecl *findExplicitDestroy(
2703-
NominalTypeDecl *nominal, const clang::TypeDecl *clangType) {
2704-
if (!clangType->hasAttrs())
2705-
return nullptr;
2706-
2707-
llvm::TinyPtrVector<FuncDecl *> matchingDestroyFuncs;
2708-
for (auto attr : clangType->getAttrs()) {
2709-
auto swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr);
2710-
if (!swiftAttr)
2711-
continue;
2712-
2713-
auto attributeName = swiftAttr->getAttribute();
2714-
if (!attributeName.starts_with("destroy:"))
2715-
continue;
2716-
2717-
auto destroyFuncName = attributeName.drop_front(strlen("destroy:"));
2718-
auto decls = getValueDeclsForName(
2719-
clangType, nominal->getASTContext(), destroyFuncName);
2720-
for (auto decl : decls) {
2721-
auto func = dyn_cast<FuncDecl>(decl);
2722-
if (!func)
2723-
continue;
2724-
2725-
auto params = func->getParameters();
2726-
if (params->size() != 1)
2727-
continue;
2728-
2729-
if (!params->get(0)->getInterfaceType()->isEqual(
2730-
nominal->getDeclaredInterfaceType()))
2731-
continue;
2732-
2733-
matchingDestroyFuncs.push_back(func);
2734-
}
2735-
}
2736-
2737-
if (matchingDestroyFuncs.size() == 1)
2738-
return matchingDestroyFuncs[0];
2739-
2740-
return nullptr;
2741-
}
2742-
2743-
/// Function body synthesizer for a deinit of a noncopyable type, which
2744-
/// passes "self" to the given "destroy" function.
2745-
static std::pair<BraceStmt *, bool>
2746-
synthesizeDeinitBodyForCustomDestroy(
2747-
AbstractFunctionDecl *deinitFunc, void *opaqueDestroyFunc) {
2748-
auto deinit = cast<DestructorDecl>(deinitFunc);
2749-
auto destroyFunc = static_cast<FuncDecl *>(opaqueDestroyFunc);
2750-
2751-
ASTContext &ctx = deinit->getASTContext();
2752-
auto funcRef = new (ctx) DeclRefExpr(
2753-
destroyFunc, DeclNameLoc(), /*Implicit=*/true);
2754-
auto selfRef = new (ctx) DeclRefExpr(
2755-
deinit->getImplicitSelfDecl(), DeclNameLoc(), /*Implicit=*/true);
2756-
auto callExpr = CallExpr::createImplicit(
2757-
ctx, funcRef,
2758-
ArgumentList::createImplicit(
2759-
ctx,
2760-
{ Argument(SourceLoc(), Identifier(), selfRef)}
2761-
)
2762-
);
2763-
2764-
auto braceStmt = BraceStmt::createImplicit(ctx, { ASTNode(callExpr) });
2765-
return std::make_pair(braceStmt, /*typechecked=*/false);
2766-
}
2767-
2768-
/// For a type that is imported as noncopyable, look for an
2769-
/// explicitly-provided "destroy" operation. If present, introduce a deinit
2770-
/// that calls it.
2771-
void addExplicitDeinitIfRequired(
2772-
NominalTypeDecl *nominal, const clang::TypeDecl *clangType) {
2773-
auto destroyFunc = findExplicitDestroy(nominal, clangType);
2774-
if (!destroyFunc)
2775-
return;
2776-
2777-
ASTContext &ctx = Impl.SwiftContext;
2778-
auto destructor = new (ctx) DestructorDecl(SourceLoc(), nominal);
2779-
destructor->setSynthesized(true);
2780-
destructor->copyFormalAccessFrom(nominal, /*sourceIsParentContext*/true);
2781-
destructor->setBodySynthesizer(
2782-
synthesizeDeinitBodyForCustomDestroy, destroyFunc);
2783-
2784-
nominal->addMember(destructor);
2785-
}
2786-
27872697
void validatePrivateFileIDAttributes(const clang::CXXRecordDecl *decl) {
27882698
auto anns = importer::getPrivateFileIDAttrs(decl);
27892699

‎lib/ClangImporter/SwiftDeclSynthesizer.cpp‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "swift/AST/Stmt.h"
2424
#include "swift/AST/TypeCheckRequests.h"
2525
#include "swift/Basic/Assertions.h"
26+
#include "swift/ClangImporter/ClangImporterRequests.h"
2627
#include "clang/AST/Mangle.h"
2728
#include "clang/Sema/DelayedDiagnostic.h"
2829

@@ -2809,6 +2810,179 @@ synthesizeAvailabilityDomainPredicateBody(AbstractFunctionDecl *afd,
28092810
return {body, /*isTypeChecked=*/false};
28102811
}
28112812

2813+
/// Mark the given declaration as always deprecated for the given reason.
2814+
static void markDeprecated(Decl *decl, llvm::Twine message) {
2815+
ASTContext &ctx = decl->getASTContext();
2816+
decl->getAttrs().add(
2817+
AvailableAttr::createUniversallyDeprecated(
2818+
ctx, ctx.AllocateCopy(message.str())));
2819+
}
2820+
2821+
/// Find an explicitly-provided "destroy" operation specified for the
2822+
/// given Clang type and return it.
2823+
FuncDecl *SwiftDeclSynthesizer::findExplicitDestroy(
2824+
NominalTypeDecl *nominal, const clang::RecordDecl *clangType) {
2825+
if (!clangType->hasAttrs())
2826+
return nullptr;
2827+
2828+
llvm::SmallPtrSet<FuncDecl *, 2> matchingDestroyFuncs;
2829+
llvm::TinyPtrVector<FuncDecl *> nonMatchingDestroyFuncs;
2830+
for (auto attr : clangType->getAttrs()) {
2831+
auto swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr);
2832+
if (!swiftAttr)
2833+
continue;
2834+
2835+
auto destroyFuncName = swiftAttr->getAttribute();
2836+
if (!destroyFuncName.consume_front("destroy:"))
2837+
continue;
2838+
2839+
auto decls = getValueDeclsForName(
2840+
clangType, nominal->getASTContext(), destroyFuncName);
2841+
for (auto decl : decls) {
2842+
auto func = dyn_cast<FuncDecl>(decl);
2843+
if (!func)
2844+
continue;
2845+
2846+
auto params = func->getParameters();
2847+
if (params->size() != 1) {
2848+
nonMatchingDestroyFuncs.push_back(func);
2849+
continue;
2850+
}
2851+
2852+
if (!params->get(0)->getInterfaceType()->isEqual(
2853+
nominal->getDeclaredInterfaceType())) {
2854+
nonMatchingDestroyFuncs.push_back(func);
2855+
continue;
2856+
}
2857+
2858+
matchingDestroyFuncs.insert(func);
2859+
}
2860+
}
2861+
2862+
switch (matchingDestroyFuncs.size()) {
2863+
case 0:
2864+
if (!nonMatchingDestroyFuncs.empty()) {
2865+
markDeprecated(
2866+
nominal,
2867+
"destroy function '" +
2868+
nonMatchingDestroyFuncs.front()->getName().getBaseName()
2869+
.userFacingName() +
2870+
"' must have a single parameter with type '" +
2871+
nominal->getDeclaredInterfaceType().getString() + "'");
2872+
}
2873+
2874+
return nullptr;
2875+
2876+
case 1:
2877+
// Handled below.
2878+
break;
2879+
2880+
default: {
2881+
auto iter = matchingDestroyFuncs.begin();
2882+
auto first = *iter++;
2883+
auto second = *iter;
2884+
markDeprecated(
2885+
nominal,
2886+
"multiple destroy operations ('" +
2887+
first->getName().getBaseName().userFacingName() +
2888+
"' and '" +
2889+
second->getName().getBaseName().userFacingName() +
2890+
"') provided for type");
2891+
return nullptr;
2892+
}
2893+
}
2894+
2895+
auto destroyFunc = *matchingDestroyFuncs.begin();
2896+
2897+
// If this type isn't imported as noncopyable, we can't respect the request
2898+
// for a destroy operation.
2899+
ASTContext &ctx = ImporterImpl.SwiftContext;
2900+
auto semanticsKind = evaluateOrDefault(
2901+
ctx.evaluator,
2902+
CxxRecordSemantics({clangType, ctx, &ImporterImpl}), {});
2903+
switch (semanticsKind) {
2904+
case CxxRecordSemanticsKind::Owned:
2905+
case CxxRecordSemanticsKind::Reference:
2906+
if (auto cxxRecord = dyn_cast<clang::CXXRecordDecl>(clangType)) {
2907+
if (!cxxRecord->hasTrivialDestructor()) {
2908+
markDeprecated(
2909+
nominal,
2910+
"destroy operation '" +
2911+
destroyFunc->getName().getBaseName().userFacingName() +
2912+
"' is not allowed on types with a non-trivial destructor");
2913+
return nullptr;
2914+
}
2915+
}
2916+
2917+
LLVM_FALLTHROUGH;
2918+
2919+
case CxxRecordSemanticsKind::Trivial:
2920+
markDeprecated(
2921+
nominal,
2922+
"destroy operation '" +
2923+
destroyFunc->getName().getBaseName().userFacingName() +
2924+
"' is only allowed on non-copyable types; "
2925+
"did you mean to use SWIFT_NONCOPYABLE?");
2926+
return nullptr;
2927+
2928+
case CxxRecordSemanticsKind::Iterator:
2929+
case CxxRecordSemanticsKind::MissingLifetimeOperation:
2930+
case CxxRecordSemanticsKind::SwiftClassType:
2931+
return nullptr;
2932+
2933+
case CxxRecordSemanticsKind::MoveOnly:
2934+
case CxxRecordSemanticsKind::UnavailableConstructors:
2935+
// Handled below.
2936+
break;
2937+
}
2938+
if (semanticsKind != CxxRecordSemanticsKind::MoveOnly) {
2939+
return nullptr;
2940+
}
2941+
2942+
return destroyFunc;
2943+
}
2944+
2945+
/// Function body synthesizer for a deinit of a noncopyable type, which
2946+
/// passes "self" to the given "destroy" function.
2947+
static std::pair<BraceStmt *, bool>
2948+
synthesizeDeinitBodyForCustomDestroy(
2949+
AbstractFunctionDecl *deinitFunc, void *opaqueDestroyFunc) {
2950+
auto deinit = cast<DestructorDecl>(deinitFunc);
2951+
auto destroyFunc = static_cast<FuncDecl *>(opaqueDestroyFunc);
2952+
2953+
ASTContext &ctx = deinit->getASTContext();
2954+
auto funcRef = new (ctx) DeclRefExpr(
2955+
destroyFunc, DeclNameLoc(), /*Implicit=*/true);
2956+
auto selfRef = new (ctx) DeclRefExpr(
2957+
deinit->getImplicitSelfDecl(), DeclNameLoc(), /*Implicit=*/true);
2958+
auto callExpr = CallExpr::createImplicit(
2959+
ctx, funcRef,
2960+
ArgumentList::createImplicit(
2961+
ctx,
2962+
{ Argument(SourceLoc(), Identifier(), selfRef)}
2963+
)
2964+
);
2965+
2966+
auto braceStmt = BraceStmt::createImplicit(ctx, { ASTNode(callExpr) });
2967+
return std::make_pair(braceStmt, /*typechecked=*/false);
2968+
}
2969+
2970+
void SwiftDeclSynthesizer::addExplicitDeinitIfRequired(
2971+
NominalTypeDecl *nominal, const clang::RecordDecl *clangType) {
2972+
auto destroyFunc = findExplicitDestroy(nominal, clangType);
2973+
if (!destroyFunc)
2974+
return;
2975+
2976+
ASTContext &ctx = nominal->getASTContext();
2977+
auto destructor = new (ctx) DestructorDecl(SourceLoc(), nominal);
2978+
destructor->setSynthesized(true);
2979+
destructor->copyFormalAccessFrom(nominal, /*sourceIsParentContext*/true);
2980+
destructor->setBodySynthesizer(
2981+
synthesizeDeinitBodyForCustomDestroy, destroyFunc);
2982+
2983+
nominal->addMember(destructor);
2984+
}
2985+
28122986
FuncDecl *SwiftDeclSynthesizer::makeAvailabilityDomainPredicate(
28132987
const clang::VarDecl *var) {
28142988
ASTContext &ctx = ImporterImpl.SwiftContext;

‎lib/ClangImporter/SwiftDeclSynthesizer.h‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,12 @@ class SwiftDeclSynthesizer {
341341
synthesizeStaticFactoryForCXXForeignRef(
342342
const clang::CXXRecordDecl *cxxRecordDecl);
343343

344+
/// Look for an explicitly-provided "destroy" operation. If one exists
345+
/// and the type has been imported as noncopyable, add an explicit `deinit`
346+
/// that calls that destroy operation.
347+
void addExplicitDeinitIfRequired(
348+
NominalTypeDecl *nominal, const clang::RecordDecl *clangType);
349+
344350
/// Synthesize a Swift function that calls the Clang runtime predicate
345351
/// function for the availability domain represented by `var`.
346352
FuncDecl *makeAvailabilityDomainPredicate(const clang::VarDecl *var);
@@ -349,6 +355,12 @@ class SwiftDeclSynthesizer {
349355

350356
private:
351357
Type getConstantLiteralType(Type type, ConstantConvertKind convertKind);
358+
359+
/// Find an explicitly-provided "destroy" operation specified for the
360+
/// given Clang type and return it.
361+
FuncDecl *findExplicitDestroy(
362+
NominalTypeDecl *nominal, const clang::RecordDecl *clangType);
363+
352364
};
353365

354366
} // namespace swift

‎test/Interop/C/struct/Inputs/noncopyable-struct.h‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,44 @@ typedef struct __attribute__((swift_attr("~Copyable"))) __attribute__((swift_att
88
void *storage;
99
} NonCopyableWithDeinit;
1010

11+
typedef struct __attribute__((swift_attr("destroy:freeCopyableType"))) CopyableType {
12+
void *storage;
13+
} CopyableType;
14+
15+
typedef struct __attribute__((swift_attr("~Copyable"))) __attribute__((swift_attr("destroy:freeMultiNonCopyable1"))) __attribute__((swift_attr("destroy:freeMultiNonCopyable2"))) MultiNonCopyableType {
16+
void *storage;
17+
} MultiNonCopyableType;
18+
19+
typedef struct __attribute__((swift_attr("~Copyable"))) __attribute__((swift_attr("destroy:badDestroy1"))) BadDestroyNonCopyableType {
20+
void *storage;
21+
} BadDestroyNonCopyableType;
22+
23+
typedef struct __attribute__((swift_attr("~Copyable"))) __attribute__((swift_attr("destroy:badDestroy2"))) BadDestroyNonCopyableType2 {
24+
void *storage;
25+
} BadDestroyNonCopyableType2;
26+
1127
#ifdef __cplusplus
1228
extern "C" {
1329
#endif
1430

1531
void freeNonCopyableWithDeinit(NonCopyableWithDeinit ncd);
32+
void freeCopyableType(CopyableType ct);
33+
34+
35+
void freeMultiNonCopyable1(MultiNonCopyableType ct);
36+
void freeMultiNonCopyable2(MultiNonCopyableType ct);
37+
38+
void badDestroy1(void);
39+
void badDestroy2(BadDestroyNonCopyableType2 *ptr);
1640

1741
#ifdef __cplusplus
42+
43+
struct __attribute__((swift_attr("~Copyable"))) __attribute__((swift_attr("destroy:extraDestroy"))) ExtraDestroy {
44+
void *storage;
45+
46+
~ExtraDestroy() { }
47+
};
48+
49+
void extraDestroy(ExtraDestroy);
1850
}
1951
#endif

0 commit comments

Comments
 (0)