Skip to content

Commit 89018a7

Browse files
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++) (cherry picked from commit 63135e8)
1 parent 80d5b9d commit 89018a7

File tree

5 files changed

+282
-95
lines changed

5 files changed

+282
-95
lines changed

lib/ClangImporter/ImportDecl.cpp

Lines changed: 2 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -2666,103 +2666,13 @@ namespace {
26662666
}
26672667
}
26682668

2669-
// A type imported as noncopyable, check whether an explicit deinit
2670-
// should be introduced to call a user-defined "destroy" function.
2671-
if (recordHasMoveOnlySemantics(decl)) {
2672-
addExplicitDeinitIfRequired(result, decl);
2673-
}
2669+
// If we need it, add an explicit "deinit" to this type.
2670+
synthesizer.addExplicitDeinitIfRequired(result, decl);
26742671

26752672
result->setMemberLoader(&Impl, 0);
26762673
return result;
26772674
}
26782675

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

lib/ClangImporter/SwiftDeclSynthesizer.cpp

Lines changed: 214 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
#include "swift/AST/Pattern.h"
2121
#include "swift/AST/Stmt.h"
2222
#include "swift/Basic/Assertions.h"
23-
#include "swift/ClangImporter/CXXMethodBridging.h"
23+
#include "swift/ClangImporter/ClangImporterRequests.h"
2424
#include "clang/AST/Mangle.h"
2525
#include "clang/Sema/DelayedDiagnostic.h"
2626

@@ -2729,3 +2729,216 @@ SwiftDeclSynthesizer::synthesizeStaticFactoryForCXXForeignRef(
27292729

27302730
return synthesizedFactories;
27312731
}
2732+
2733+
/// Mark the given declaration as always deprecated for the given reason.
2734+
static void markDeprecated(Decl *decl, llvm::Twine message) {
2735+
ASTContext &ctx = decl->getASTContext();
2736+
decl->getAttrs().add(
2737+
AvailableAttr::createUniversallyDeprecated(
2738+
ctx, ctx.AllocateCopy(message.str())));
2739+
}
2740+
2741+
/// Find an explicitly-provided "destroy" operation specified for the
2742+
/// given Clang type and return it.
2743+
FuncDecl *SwiftDeclSynthesizer::findExplicitDestroy(
2744+
NominalTypeDecl *nominal, const clang::RecordDecl *clangType) {
2745+
if (!clangType->hasAttrs())
2746+
return nullptr;
2747+
2748+
llvm::SmallPtrSet<FuncDecl *, 2> matchingDestroyFuncs;
2749+
llvm::TinyPtrVector<FuncDecl *> nonMatchingDestroyFuncs;
2750+
for (auto attr : clangType->getAttrs()) {
2751+
auto swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr);
2752+
if (!swiftAttr)
2753+
continue;
2754+
2755+
auto destroyFuncName = swiftAttr->getAttribute();
2756+
if (!destroyFuncName.consume_front("destroy:"))
2757+
continue;
2758+
2759+
auto decls = getValueDeclsForName(
2760+
clangType, nominal->getASTContext(), destroyFuncName);
2761+
for (auto decl : decls) {
2762+
auto func = dyn_cast<FuncDecl>(decl);
2763+
if (!func)
2764+
continue;
2765+
2766+
auto params = func->getParameters();
2767+
if (params->size() != 1) {
2768+
nonMatchingDestroyFuncs.push_back(func);
2769+
continue;
2770+
}
2771+
2772+
if (!params->get(0)->getInterfaceType()->isEqual(
2773+
nominal->getDeclaredInterfaceType())) {
2774+
nonMatchingDestroyFuncs.push_back(func);
2775+
continue;
2776+
}
2777+
2778+
matchingDestroyFuncs.insert(func);
2779+
}
2780+
}
2781+
2782+
switch (matchingDestroyFuncs.size()) {
2783+
case 0:
2784+
if (!nonMatchingDestroyFuncs.empty()) {
2785+
markDeprecated(
2786+
nominal,
2787+
"destroy function '" +
2788+
nonMatchingDestroyFuncs.front()->getName().getBaseName()
2789+
.userFacingName() +
2790+
"' must have a single parameter with type '" +
2791+
nominal->getDeclaredInterfaceType().getString() + "'");
2792+
}
2793+
2794+
return nullptr;
2795+
2796+
case 1:
2797+
// Handled below.
2798+
break;
2799+
2800+
default: {
2801+
auto iter = matchingDestroyFuncs.begin();
2802+
auto first = *iter++;
2803+
auto second = *iter;
2804+
markDeprecated(
2805+
nominal,
2806+
"multiple destroy operations ('" +
2807+
first->getName().getBaseName().userFacingName() +
2808+
"' and '" +
2809+
second->getName().getBaseName().userFacingName() +
2810+
"') provided for type");
2811+
return nullptr;
2812+
}
2813+
}
2814+
2815+
auto destroyFunc = *matchingDestroyFuncs.begin();
2816+
2817+
// If this type isn't imported as noncopyable, we can't respect the request
2818+
// for a destroy operation.
2819+
ASTContext &ctx = ImporterImpl.SwiftContext;
2820+
auto semanticsKind = evaluateOrDefault(
2821+
ctx.evaluator,
2822+
CxxRecordSemantics({clangType, ctx, &ImporterImpl}), {});
2823+
switch (semanticsKind) {
2824+
case CxxRecordSemanticsKind::Owned:
2825+
case CxxRecordSemanticsKind::Reference:
2826+
if (auto cxxRecord = dyn_cast<clang::CXXRecordDecl>(clangType)) {
2827+
if (!cxxRecord->hasTrivialDestructor()) {
2828+
markDeprecated(
2829+
nominal,
2830+
"destroy operation '" +
2831+
destroyFunc->getName().getBaseName().userFacingName() +
2832+
"' is not allowed on types with a non-trivial destructor");
2833+
return nullptr;
2834+
}
2835+
}
2836+
2837+
LLVM_FALLTHROUGH;
2838+
2839+
case CxxRecordSemanticsKind::Trivial:
2840+
markDeprecated(
2841+
nominal,
2842+
"destroy operation '" +
2843+
destroyFunc->getName().getBaseName().userFacingName() +
2844+
"' is only allowed on non-copyable types; "
2845+
"did you mean to use SWIFT_NONCOPYABLE?");
2846+
return nullptr;
2847+
2848+
case CxxRecordSemanticsKind::Iterator:
2849+
case CxxRecordSemanticsKind::MissingLifetimeOperation:
2850+
case CxxRecordSemanticsKind::SwiftClassType:
2851+
return nullptr;
2852+
2853+
case CxxRecordSemanticsKind::MoveOnly:
2854+
case CxxRecordSemanticsKind::UnavailableConstructors:
2855+
// Handled below.
2856+
break;
2857+
}
2858+
if (semanticsKind != CxxRecordSemanticsKind::MoveOnly) {
2859+
return nullptr;
2860+
}
2861+
2862+
return destroyFunc;
2863+
}
2864+
2865+
/// Function body synthesizer for a deinit of a noncopyable type, which
2866+
/// passes "self" to the given "destroy" function.
2867+
static std::pair<BraceStmt *, bool>
2868+
synthesizeDeinitBodyForCustomDestroy(
2869+
AbstractFunctionDecl *deinitFunc, void *opaqueDestroyFunc) {
2870+
auto deinit = cast<DestructorDecl>(deinitFunc);
2871+
auto destroyFunc = static_cast<FuncDecl *>(opaqueDestroyFunc);
2872+
2873+
ASTContext &ctx = deinit->getASTContext();
2874+
auto funcRef = new (ctx) DeclRefExpr(
2875+
destroyFunc, DeclNameLoc(), /*Implicit=*/true);
2876+
auto selfRef = new (ctx) DeclRefExpr(
2877+
deinit->getImplicitSelfDecl(), DeclNameLoc(), /*Implicit=*/true);
2878+
auto callExpr = CallExpr::createImplicit(
2879+
ctx, funcRef,
2880+
ArgumentList::createImplicit(
2881+
ctx,
2882+
{ Argument(SourceLoc(), Identifier(), selfRef)}
2883+
)
2884+
);
2885+
2886+
auto braceStmt = BraceStmt::createImplicit(ctx, { ASTNode(callExpr) });
2887+
return std::make_pair(braceStmt, /*typechecked=*/false);
2888+
}
2889+
2890+
void SwiftDeclSynthesizer::addExplicitDeinitIfRequired(
2891+
NominalTypeDecl *nominal, const clang::RecordDecl *clangType) {
2892+
auto destroyFunc = findExplicitDestroy(nominal, clangType);
2893+
if (!destroyFunc)
2894+
return;
2895+
2896+
ASTContext &ctx = nominal->getASTContext();
2897+
auto destructor = new (ctx) DestructorDecl(SourceLoc(), nominal);
2898+
destructor->setSynthesized(true);
2899+
destructor->copyFormalAccessFrom(nominal, /*sourceIsParentContext*/true);
2900+
destructor->setBodySynthesizer(
2901+
synthesizeDeinitBodyForCustomDestroy, destroyFunc);
2902+
2903+
nominal->addMember(destructor);
2904+
}
2905+
2906+
FuncDecl *SwiftDeclSynthesizer::makeAvailabilityDomainPredicate(
2907+
const clang::VarDecl *var) {
2908+
ASTContext &ctx = ImporterImpl.SwiftContext;
2909+
clang::ASTContext &clangCtx = var->getASTContext();
2910+
auto featureInfo =
2911+
clangCtx.getFeatureAvailInfo(const_cast<clang::VarDecl *>(var));
2912+
2913+
// If the decl doesn't represent and availability domain, skip it.
2914+
if (featureInfo.first.empty())
2915+
return nullptr;
2916+
2917+
// Only dynamic availability domains require a predicate function.
2918+
if (featureInfo.second.Kind != clang::FeatureAvailKind::Dynamic)
2919+
return nullptr;
2920+
2921+
if (!featureInfo.second.Call)
2922+
return nullptr;
2923+
2924+
// Synthesize `func __swift_XYZ_isAvailable() -> Builtin.Int1 { ... }`.
2925+
std::string s;
2926+
llvm::raw_string_ostream os(s);
2927+
os << "__swift_" << featureInfo.first << "_isAvailable";
2928+
DeclName funcName(ctx, DeclBaseName(ctx.getIdentifier(s)),
2929+
ParameterList::createEmpty(ctx));
2930+
2931+
auto funcDecl = FuncDecl::createImplicit(
2932+
ctx, StaticSpellingKind::None, funcName, SourceLoc(), /*Async=*/false,
2933+
/*Throws=*/false, Type(), {}, ParameterList::createEmpty(ctx),
2934+
BuiltinIntegerType::get(1, ctx), ImporterImpl.ImportedHeaderUnit);
2935+
funcDecl->setBodySynthesizer(synthesizeAvailabilityDomainPredicateBody,
2936+
(void *)var);
2937+
funcDecl->setAccess(AccessLevel::Public);
2938+
funcDecl->getAttrs().add(new (ctx)
2939+
AlwaysEmitIntoClientAttr(/*IsImplicit=*/true));
2940+
2941+
ImporterImpl.availabilityDomainPredicates[var] = funcDecl;
2942+
2943+
return funcDecl;
2944+
}

lib/ClangImporter/SwiftDeclSynthesizer.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,8 +341,20 @@ 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
private:
345351
Type getConstantLiteralType(Type type, ConstantConvertKind convertKind);
352+
353+
/// Find an explicitly-provided "destroy" operation specified for the
354+
/// given Clang type and return it.
355+
FuncDecl *findExplicitDestroy(
356+
NominalTypeDecl *nominal, const clang::RecordDecl *clangType);
357+
346358
};
347359

348360
} // 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)