From 351644866ad14bc2208b28ed1e90e254fe134d91 Mon Sep 17 00:00:00 2001 From: susmonteiro Date: Tue, 11 Nov 2025 13:58:38 +0000 Subject: [PATCH 1/4] Make CxxValueSemantics request non-recursive --- lib/ClangImporter/ClangImporter.cpp | 167 +++++++++++++++------------- 1 file changed, 91 insertions(+), 76 deletions(-) diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index ab6eedc32fa62..0d10f05074a10 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -8459,103 +8459,118 @@ CxxValueSemanticsKind CxxValueSemantics::evaluate(Evaluator &evaluator, CxxValueSemanticsDescriptor desc) const { - const auto *type = desc.type; + // A C++ type can be imported to Swift as Copyable or ~Copyable. + // We assume a type can be imported as Copyable unless: + // - There's no copy constructor + // - The type has a SWIFT_NONCOPYABLE annotation + // - The type has a SWIFT_COPYABLE_IF(T...) annotation, where at least one of T is ~Copyable + // - It is one of the STL types in `STLConditionalParams` + + const auto *type = desc.type->getUnqualifiedDesugaredType(); auto *importerImpl = desc.importerImpl; - auto desugared = type->getUnqualifiedDesugaredType(); - const auto *recordType = desugared->getAs(); - if (!recordType) - return CxxValueSemanticsKind::Copyable; + llvm::SmallVector stack; + // Keep track of Decls we've seen to avoid cycles + llvm::SmallDenseSet seen; - auto recordDecl = recordType->getDecl(); + auto maybePushToStack = [&](const clang::Type *type) { + auto recordDecl = type->getAsRecordDecl(); + if (!recordDecl) + return; - // When a reference type is copied, the pointer’s value is copied rather than - // the object’s storage. This means reference types can be imported as - // copyable to Swift, even when they are non-copyable in C++. - if (recordHasReferenceSemantics(recordDecl, importerImpl)) - return CxxValueSemanticsKind::Copyable; + if (seen.insert(recordDecl).second) { + // When a reference type is copied, the pointer’s value is copied rather + // than the object’s storage. This means reference types can be imported + // as copyable to Swift, even when they are non-copyable in C++. + if (recordHasReferenceSemantics(recordDecl, importerImpl)) + return; - if (recordDecl->isInStdNamespace()) { - // Hack for a base type of std::optional from the Microsoft standard - // library. - if (recordDecl->getIdentifier() && - recordDecl->getName() == "_Optional_construct_base") - return CxxValueSemanticsKind::Copyable; - } + if (recordDecl->isInStdNamespace()) { + // Hack for a base type of std::optional from the Microsoft standard + // library. + if (recordDecl->getIdentifier() && + recordDecl->getName() == "_Optional_construct_base") + return; + } + stack.push_back(recordDecl); + } + }; - if (!hasNonCopyableAttr(recordDecl)) { - auto injectedStlAnnotation = - recordDecl->isInStdNamespace() - ? STLConditionalParams.find(recordDecl->getName()) - : STLConditionalParams.end(); - auto STLParams = injectedStlAnnotation != STLConditionalParams.end() - ? injectedStlAnnotation->second - : std::vector(); - auto conditionalParams = getConditionalCopyableAttrParams(recordDecl); + maybePushToStack(type); + while (!stack.empty()) { + clang::RecordDecl *recordDecl = stack.back(); + stack.pop_back(); - if (!STLParams.empty() || !conditionalParams.empty()) { - HeaderLoc loc{recordDecl->getLocation()}; - std::function checkArgValueSemantics = - [&](clang::TemplateArgument &arg, - StringRef argToCheck) -> std::optional { - if (arg.getKind() != clang::TemplateArgument::Type && importerImpl) { - importerImpl->diagnose(loc, diag::type_template_parameter_expected, - argToCheck); - return CxxValueSemanticsKind::Unknown; - } + if (!hasNonCopyableAttr(recordDecl)) { + auto injectedStlAnnotation = + recordDecl->isInStdNamespace() + ? STLConditionalParams.find(recordDecl->getName()) + : STLConditionalParams.end(); + auto STLParams = injectedStlAnnotation != STLConditionalParams.end() + ? injectedStlAnnotation->second + : std::vector(); + auto conditionalParams = getConditionalCopyableAttrParams(recordDecl); + + if (!STLParams.empty() || !conditionalParams.empty()) { + HeaderLoc loc{recordDecl->getLocation()}; + std::function checkArgValueSemantics = + [&](clang::TemplateArgument &arg, + StringRef argToCheck) -> std::optional { + if (arg.getKind() != clang::TemplateArgument::Type) { + if (importerImpl) + importerImpl->diagnose( + loc, diag::type_template_parameter_expected, argToCheck); + return CxxValueSemanticsKind::Unknown; + } + maybePushToStack(arg.getAsType()->getUnqualifiedDesugaredType()); + // FIXME: return std::nullopt for now, while we don't refactor ClangTypeEscapability request + return std::nullopt; + }; - auto argValueSemantics = evaluateOrDefault( - evaluator, - CxxValueSemantics( - {arg.getAsType()->getUnqualifiedDesugaredType(), importerImpl}), - {}); - if (argValueSemantics != CxxValueSemanticsKind::Copyable) - return argValueSemantics; - return std::nullopt; - }; + checkConditionalParams( + recordDecl, STLParams, conditionalParams, checkArgValueSemantics); - auto result = checkConditionalParams( - recordDecl, STLParams, conditionalParams, checkArgValueSemantics); - if (result.has_value()) - return result.value(); + if (importerImpl) + for (auto name : conditionalParams) + importerImpl->diagnose(loc, diag::unknown_template_parameter, name); - if (importerImpl) - for (auto name : conditionalParams) - importerImpl->diagnose(loc, diag::unknown_template_parameter, name); + continue; + } + } - return CxxValueSemanticsKind::Copyable; + const auto cxxRecordDecl = dyn_cast(recordDecl); + if (!cxxRecordDecl || !cxxRecordDecl->isCompleteDefinition()) { + if (hasNonCopyableAttr(recordDecl)) + return CxxValueSemanticsKind::MoveOnly; + continue; } - } - const auto cxxRecordDecl = dyn_cast(recordDecl); - if (!cxxRecordDecl || !cxxRecordDecl->isCompleteDefinition()) { - if (hasNonCopyableAttr(recordDecl)) - return CxxValueSemanticsKind::MoveOnly; - return CxxValueSemanticsKind::Copyable; - } + bool isCopyable = !hasNonCopyableAttr(cxxRecordDecl) && + hasCopyTypeOperations(cxxRecordDecl); + bool isMovable = hasMoveTypeOperations(cxxRecordDecl); - bool isCopyable = !hasNonCopyableAttr(cxxRecordDecl) && - hasCopyTypeOperations(cxxRecordDecl); - bool isMovable = hasMoveTypeOperations(cxxRecordDecl); + if (!hasDestroyTypeOperations(cxxRecordDecl) || + (!isCopyable && !isMovable)) { - if (!hasDestroyTypeOperations(cxxRecordDecl) || (!isCopyable && !isMovable)) { + if (hasConstructorWithUnsupportedDefaultArgs(cxxRecordDecl)) + return CxxValueSemanticsKind::UnavailableConstructors; - if (hasConstructorWithUnsupportedDefaultArgs(cxxRecordDecl)) - return CxxValueSemanticsKind::UnavailableConstructors; + return CxxValueSemanticsKind::MissingLifetimeOperation; + } - return CxxValueSemanticsKind::MissingLifetimeOperation; - } + if (hasNonCopyableAttr(cxxRecordDecl) && isMovable) + return CxxValueSemanticsKind::MoveOnly; - if (hasNonCopyableAttr(cxxRecordDecl) && isMovable) - return CxxValueSemanticsKind::MoveOnly; + if (isCopyable) + continue; - if (isCopyable) - return CxxValueSemanticsKind::Copyable; + if (isMovable) + return CxxValueSemanticsKind::MoveOnly; - if (isMovable) - return CxxValueSemanticsKind::MoveOnly; + llvm_unreachable("Could not classify C++ type."); + } - llvm_unreachable("Could not classify C++ type."); + return CxxValueSemanticsKind::Copyable; } void swift::simple_display(llvm::raw_ostream &out, From 5dfb76637d012a3c78c35c992fd9cab587064f5e Mon Sep 17 00:00:00 2001 From: susmonteiro Date: Tue, 11 Nov 2025 18:11:13 +0000 Subject: [PATCH 2/4] Make ClangTypeEscapability request non-recursive --- lib/ClangImporter/ClangImporter.cpp | 183 +++++++++--------- .../Cxx/class/nonescapable-errors.swift | 4 +- 2 files changed, 89 insertions(+), 98 deletions(-) diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index 0d10f05074a10..b5601f7888b6f 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -5467,112 +5467,103 @@ getConditionalCopyableAttrParams(const clang::RecordDecl *decl) { CxxEscapability ClangTypeEscapability::evaluate(Evaluator &evaluator, EscapabilityLookupDescriptor desc) const { - bool hadUnknown = false; - auto evaluateEscapability = [&](const clang::Type *type) { - auto escapability = evaluateOrDefault( - evaluator, - ClangTypeEscapability({type, desc.impl, desc.annotationOnly}), - CxxEscapability::Unknown); - if (escapability == CxxEscapability::Unknown) - hadUnknown = true; - return escapability; - }; - + bool hasUnknown = false; auto desugared = desc.type->getUnqualifiedDesugaredType(); if (const auto *recordType = desugared->getAs()) { auto recordDecl = recordType->getDecl(); - if (hasNonEscapableAttr(recordDecl)) - return CxxEscapability::NonEscapable; if (hasEscapableAttr(recordDecl)) return CxxEscapability::Escapable; - auto injectedStlAnnotation = - recordDecl->isInStdNamespace() - ? STLConditionalParams.find(recordDecl->getName()) - : STLConditionalParams.end(); - auto STLParams = injectedStlAnnotation != STLConditionalParams.end() - ? injectedStlAnnotation->second - : std::vector(); - auto conditionalParams = getConditionalEscapableAttrParams(recordDecl); - - if (!STLParams.empty() || !conditionalParams.empty()) { - HeaderLoc loc{recordDecl->getLocation()}; - std::function checkArgEscapability = - [&](clang::TemplateArgument &arg, - StringRef argToCheck) -> std::optional { - if (arg.getKind() != clang::TemplateArgument::Type && desc.impl) { - desc.impl->diagnose(loc, diag::type_template_parameter_expected, - argToCheck); - return CxxEscapability::Unknown; - } + } - auto argEscapability = evaluateEscapability( - arg.getAsType()->getUnqualifiedDesugaredType()); - if (argEscapability == CxxEscapability::NonEscapable) - return CxxEscapability::NonEscapable; - return std::nullopt; - }; - - auto result = checkConditionalParams( - recordDecl, STLParams, conditionalParams, checkArgEscapability); - if (result.has_value()) - return result.value(); - - if (desc.impl) - for (auto name : conditionalParams) - desc.impl->diagnose(loc, diag::unknown_template_parameter, name); - - return hadUnknown ? CxxEscapability::Unknown : CxxEscapability::Escapable; - } - if (desc.annotationOnly) - return CxxEscapability::Unknown; - auto cxxRecordDecl = dyn_cast(recordDecl); - if (recordDecl->getDefinition() && - (!cxxRecordDecl || cxxRecordDecl->isAggregate())) { - if (cxxRecordDecl) { - for (auto base : cxxRecordDecl->bases()) { - auto baseEscapability = evaluateEscapability( - base.getType()->getUnqualifiedDesugaredType()); - if (baseEscapability == CxxEscapability::NonEscapable) - return CxxEscapability::NonEscapable; - } - } + llvm::SmallVector stack; + // Keep track of Decls we've seen to avoid cycles + llvm::SmallDenseSet seen; + + auto maybePushToStack = [&](const clang::Type *type) { + auto desugared = type->getUnqualifiedDesugaredType(); + if (seen.insert(desugared).second) + stack.push_back(desugared); + }; + + maybePushToStack(desc.type); + while (!stack.empty()) { + auto type = stack.back(); + stack.pop_back(); + if (const auto *recordType = type->getAs()) { + auto recordDecl = recordType->getDecl(); + if (hasNonEscapableAttr(recordDecl)) + return CxxEscapability::NonEscapable; + if (hasEscapableAttr(recordDecl)) + continue; + auto injectedStlAnnotation = + recordDecl->isInStdNamespace() + ? STLConditionalParams.find(recordDecl->getName()) + : STLConditionalParams.end(); + auto STLParams = injectedStlAnnotation != STLConditionalParams.end() + ? injectedStlAnnotation->second + : std::vector(); + auto conditionalParams = getConditionalEscapableAttrParams(recordDecl); + + if (!STLParams.empty() || !conditionalParams.empty()) { + HeaderLoc loc{recordDecl->getLocation()}; + std::function checkArgEscapability = + [&](clang::TemplateArgument &arg, + StringRef argToCheck) -> std::optional { + if (arg.getKind() != clang::TemplateArgument::Type) { + if (desc.impl) + desc.impl->diagnose(loc, diag::type_template_parameter_expected, + argToCheck); + hasUnknown = true; + return std::nullopt; + } + maybePushToStack(arg.getAsType()->getUnqualifiedDesugaredType()); + // FIXME don't return anything + return std::nullopt; + }; + + checkConditionalParams( + recordDecl, STLParams, conditionalParams, checkArgEscapability); + + if (desc.impl) + for (auto name : conditionalParams) + desc.impl->diagnose(loc, diag::unknown_template_parameter, name); - for (auto field : recordDecl->fields()) { - auto fieldEscapability = evaluateEscapability( - field->getType()->getUnqualifiedDesugaredType()); - if (fieldEscapability == CxxEscapability::NonEscapable) - return CxxEscapability::NonEscapable; + continue; + } + if (desc.annotationOnly) { + hasUnknown = true; + continue; } + auto cxxRecordDecl = dyn_cast(recordDecl); + if (recordDecl->getDefinition() && + (!cxxRecordDecl || cxxRecordDecl->isAggregate())) { + if (cxxRecordDecl) { + // TODO llvm::foreach ? + for (auto base : cxxRecordDecl->bases()) + maybePushToStack(base.getType()->getUnqualifiedDesugaredType()); + } - return hadUnknown ? CxxEscapability::Unknown : CxxEscapability::Escapable; + for (auto field : recordDecl->fields()) + maybePushToStack(field->getType()->getUnqualifiedDesugaredType()); + continue; + } + } + if (type->isArrayType()) { + auto elemTy = cast(type) + ->getElementType() + ->getUnqualifiedDesugaredType(); + maybePushToStack(elemTy); + } else if (const auto *vecTy = type->getAs()) { + maybePushToStack(vecTy->getElementType()->getUnqualifiedDesugaredType()); + } else if (type->isAnyPointerType() || type->isBlockPointerType() || + type->isMemberPointerType() || type->isReferenceType()) { + if (desc.annotationOnly) + hasUnknown = true; + else + return CxxEscapability::NonEscapable; } } - if (desugared->isArrayType()) { - auto elemTy = cast(desugared) - ->getElementType() - ->getUnqualifiedDesugaredType(); - return evaluateOrDefault( - evaluator, - ClangTypeEscapability({elemTy, desc.impl, desc.annotationOnly}), - CxxEscapability::Unknown); - } - if (const auto *vecTy = desugared->getAs()) { - return evaluateOrDefault( - evaluator, - ClangTypeEscapability( - {vecTy->getElementType()->getUnqualifiedDesugaredType(), desc.impl, - desc.annotationOnly}), - CxxEscapability::Unknown); - } - - // Base cases - if (desugared->isAnyPointerType() || desugared->isBlockPointerType() || - desugared->isMemberPointerType() || desugared->isReferenceType()) - return desc.annotationOnly ? CxxEscapability::Unknown - : CxxEscapability::NonEscapable; - if (desugared->isScalarType()) - return CxxEscapability::Escapable; - return CxxEscapability::Unknown; + return hasUnknown ? CxxEscapability::Unknown : CxxEscapability::Escapable; } void swift::simple_display(llvm::raw_ostream &out, @@ -8520,7 +8511,7 @@ CxxValueSemantics::evaluate(Evaluator &evaluator, if (importerImpl) importerImpl->diagnose( loc, diag::type_template_parameter_expected, argToCheck); - return CxxValueSemanticsKind::Unknown; + return std::nullopt; } maybePushToStack(arg.getAsType()->getUnqualifiedDesugaredType()); // FIXME: return std::nullopt for now, while we don't refactor ClangTypeEscapability request diff --git a/test/Interop/Cxx/class/nonescapable-errors.swift b/test/Interop/Cxx/class/nonescapable-errors.swift index e8e7e6b6a0329..29d2a9330e7b8 100644 --- a/test/Interop/Cxx/class/nonescapable-errors.swift +++ b/test/Interop/Cxx/class/nonescapable-errors.swift @@ -193,10 +193,10 @@ public func noAnnotations() -> View { // CHECK-NO-LIFETIMES: nonescapable.h:56:39: error: template parameter 'Missing' does not exist i2() // CHECK: nonescapable.h:62:33: error: template parameter 'S' expected to be a type parameter - // CHECK: nonescapable.h:80:41: error: a function with a ~Escapable result needs a parameter to depend on - // CHECK: note: '@_lifetime(immortal)' can be used to indicate that values produced // CHECK-NO-LIFETIMES: nonescapable.h:62:33: error: template parameter 'S' expected to be a type parameter j1() + // CHECK: nonescapable.h:80:41: error: a function with a ~Escapable result needs a parameter to depend on + // CHECK: note: '@_lifetime(immortal)' can be used to indicate that values produced // CHECK-NO-LIFETIMES: nonescapable.h:80:41: error: a function cannot return a ~Escapable result j2() // CHECK: nonescapable.h:81:41: error: a function with a ~Escapable result needs a parameter to depend on From de52134e29df183ef06862cbfb23b93b0c90bdb8 Mon Sep 17 00:00:00 2001 From: susmonteiro Date: Wed, 12 Nov 2025 18:00:30 +0000 Subject: [PATCH 3/4] Refactor checkConditionalParams --- lib/ClangImporter/ClangImporter.cpp | 63 +++++++++++++---------------- 1 file changed, 29 insertions(+), 34 deletions(-) diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index b5601f7888b6f..6a54a2c1d3376 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -5381,11 +5381,12 @@ static const llvm::StringMap> STLConditionalParams{ }; template -static std::optional checkConditionalParams( - clang::RecordDecl *recordDecl, const std::vector &STLParams, - std::set &conditionalParams, - std::function(clang::TemplateArgument &, StringRef)> - &checkArg) { +static bool checkConditionalParams( + clang::RecordDecl *recordDecl, ClangImporter::Implementation *impl, + const std::vector &STLParams, std::set &conditionalParams, + std::function &maybePushToStack) { + HeaderLoc loc{recordDecl->getLocation()}; + bool foundErrors = false; auto specDecl = cast(recordDecl); SmallVector, 4> argumentsToCheck; bool hasInjectedSTLAnnotation = !STLParams.empty(); @@ -5413,9 +5414,15 @@ static std::optional checkConditionalParams( } else nonPackArgs.push_back(arg); for (auto nonPackArg : nonPackArgs) { - auto result = checkArg(nonPackArg, argToCheck.second); - if (result.has_value()) - return result.value(); + if (nonPackArg.getKind() != clang::TemplateArgument::Type) { + if (impl) + impl->diagnose(loc, diag::type_template_parameter_expected, + argToCheck.second); + foundErrors = true; + } else { + maybePushToStack( + nonPackArg.getAsType()->getUnqualifiedDesugaredType()); + } } } if (hasInjectedSTLAnnotation) @@ -5428,7 +5435,7 @@ static std::optional checkConditionalParams( break; } } - return std::nullopt; + return foundErrors; } static std::set @@ -5479,7 +5486,7 @@ ClangTypeEscapability::evaluate(Evaluator &evaluator, // Keep track of Decls we've seen to avoid cycles llvm::SmallDenseSet seen; - auto maybePushToStack = [&](const clang::Type *type) { + std::function maybePushToStack = [&](const clang::Type *type) { auto desugared = type->getUnqualifiedDesugaredType(); if (seen.insert(desugared).second) stack.push_back(desugared); @@ -5505,28 +5512,15 @@ ClangTypeEscapability::evaluate(Evaluator &evaluator, auto conditionalParams = getConditionalEscapableAttrParams(recordDecl); if (!STLParams.empty() || !conditionalParams.empty()) { - HeaderLoc loc{recordDecl->getLocation()}; - std::function checkArgEscapability = - [&](clang::TemplateArgument &arg, - StringRef argToCheck) -> std::optional { - if (arg.getKind() != clang::TemplateArgument::Type) { - if (desc.impl) - desc.impl->diagnose(loc, diag::type_template_parameter_expected, - argToCheck); - hasUnknown = true; - return std::nullopt; - } - maybePushToStack(arg.getAsType()->getUnqualifiedDesugaredType()); - // FIXME don't return anything - return std::nullopt; - }; - - checkConditionalParams( - recordDecl, STLParams, conditionalParams, checkArgEscapability); + hasUnknown &= checkConditionalParams( + recordDecl, desc.impl, STLParams, conditionalParams, + maybePushToStack); - if (desc.impl) + if (desc.impl) { + HeaderLoc loc{recordDecl->getLocation()}; for (auto name : conditionalParams) desc.impl->diagnose(loc, diag::unknown_template_parameter, name); + } continue; } @@ -5538,11 +5532,9 @@ ClangTypeEscapability::evaluate(Evaluator &evaluator, if (recordDecl->getDefinition() && (!cxxRecordDecl || cxxRecordDecl->isAggregate())) { if (cxxRecordDecl) { - // TODO llvm::foreach ? for (auto base : cxxRecordDecl->bases()) maybePushToStack(base.getType()->getUnqualifiedDesugaredType()); } - for (auto field : recordDecl->fields()) maybePushToStack(field->getType()->getUnqualifiedDesugaredType()); continue; @@ -8464,7 +8456,7 @@ CxxValueSemantics::evaluate(Evaluator &evaluator, // Keep track of Decls we've seen to avoid cycles llvm::SmallDenseSet seen; - auto maybePushToStack = [&](const clang::Type *type) { + std::function maybePushToStack = [&](const clang::Type *type) { auto recordDecl = type->getAsRecordDecl(); if (!recordDecl) return; @@ -8519,11 +8511,14 @@ CxxValueSemantics::evaluate(Evaluator &evaluator, }; checkConditionalParams( - recordDecl, STLParams, conditionalParams, checkArgValueSemantics); + recordDecl, importerImpl, STLParams, conditionalParams, + maybePushToStack); - if (importerImpl) + if (importerImpl) { + HeaderLoc loc{recordDecl->getLocation()}; for (auto name : conditionalParams) importerImpl->diagnose(loc, diag::unknown_template_parameter, name); + } continue; } From 791194b0ffb1255a8c9db3d78a5370b6d3761f64 Mon Sep 17 00:00:00 2001 From: susmonteiro Date: Thu, 20 Nov 2025 12:35:58 +0000 Subject: [PATCH 4/4] Simplify CxxValueSemanticsKind --- .../ClangImporter/ClangImporterRequests.h | 10 +-- lib/ClangImporter/ClangImporter.cpp | 85 ++++++++++--------- lib/ClangImporter/ImportDecl.cpp | 17 ++-- lib/ClangImporter/SwiftDeclSynthesizer.cpp | 3 +- 4 files changed, 54 insertions(+), 61 deletions(-) diff --git a/include/swift/ClangImporter/ClangImporterRequests.h b/include/swift/ClangImporter/ClangImporterRequests.h index d435571c12272..256e0234d462a 100644 --- a/include/swift/ClangImporter/ClangImporterRequests.h +++ b/include/swift/ClangImporter/ClangImporterRequests.h @@ -575,15 +575,7 @@ SourceLoc extractNearestSourceLoc(EscapabilityLookupDescriptor desc); // When a reference type is copied, the pointer’s value is copied rather than // the object’s storage. This means reference types can be imported as // copyable to Swift, even when they are non-copyable in C++. -enum class CxxValueSemanticsKind { - Unknown, - Copyable, - MoveOnly, - // A record that is either not copyable/movable or not destructible. - MissingLifetimeOperation, - // A record that has no copy and no move operations - UnavailableConstructors, -}; +enum class CxxValueSemanticsKind { Unknown, Copyable, MoveOnly }; struct CxxValueSemanticsDescriptor final { const clang::Type *type; diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index 6a54a2c1d3376..3cbfe8a6107ae 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -5382,10 +5382,9 @@ static const llvm::StringMap> STLConditionalParams{ template static bool checkConditionalParams( - clang::RecordDecl *recordDecl, ClangImporter::Implementation *impl, + const clang::RecordDecl *recordDecl, ClangImporter::Implementation *impl, const std::vector &STLParams, std::set &conditionalParams, - std::function &maybePushToStack) { - HeaderLoc loc{recordDecl->getLocation()}; + llvm::function_ref maybePushToStack) { bool foundErrors = false; auto specDecl = cast(recordDecl); SmallVector, 4> argumentsToCheck; @@ -5416,7 +5415,8 @@ static bool checkConditionalParams( for (auto nonPackArg : nonPackArgs) { if (nonPackArg.getKind() != clang::TemplateArgument::Type) { if (impl) - impl->diagnose(loc, diag::type_template_parameter_expected, + impl->diagnose(HeaderLoc(recordDecl->getLocation()), + diag::type_template_parameter_expected, argToCheck.second); foundErrors = true; } else { @@ -5427,7 +5427,7 @@ static bool checkConditionalParams( } if (hasInjectedSTLAnnotation) break; - clang::DeclContext *dc = specDecl; + const clang::DeclContext *dc = specDecl; specDecl = nullptr; while ((dc = dc->getParent())) { specDecl = dyn_cast(dc); @@ -5474,19 +5474,33 @@ getConditionalCopyableAttrParams(const clang::RecordDecl *decl) { CxxEscapability ClangTypeEscapability::evaluate(Evaluator &evaluator, EscapabilityLookupDescriptor desc) const { + + // Escapability inference rules: + // - array and vector types have the same escapability as their element type + // - pointer and reference types are currently imported as escapable + // (importing them as non-escapable broke backward compatibility) + // - a record type is escapable or non-escapable if it is explicitly annotated + // as such + // - a record type is escapable if it is annotated with SWIFT_ESCAPABLE_IF() + // and none of the annotation arguments are non-escapable + // - in all other cases, the record has unknown escapability (e.g. no + // escapability annotations, malformed escapability annotations) + bool hasUnknown = false; auto desugared = desc.type->getUnqualifiedDesugaredType(); if (const auto *recordType = desugared->getAs()) { auto recordDecl = recordType->getDecl(); + // If the root type has a SWIFT_ESCAPABLE annotation, we import the type as + // Escapable without entering the loop if (hasEscapableAttr(recordDecl)) return CxxEscapability::Escapable; } llvm::SmallVector stack; - // Keep track of Decls we've seen to avoid cycles + // Keep track of Types we've seen to avoid cycles llvm::SmallDenseSet seen; - std::function maybePushToStack = [&](const clang::Type *type) { + auto maybePushToStack = [&](const clang::Type *type) { auto desugared = type->getUnqualifiedDesugaredType(); if (seen.insert(desugared).second) stack.push_back(desugared); @@ -5524,6 +5538,10 @@ ClangTypeEscapability::evaluate(Evaluator &evaluator, continue; } + // The `annotationOnly` flag used to control which types we infered + // escapability for. Currently, this flag is always set to true, meaning + // that any type without an annotation (CxxRecordDecls, aggregates, decls + // lacking definition, etc.) will raise `hasUnknown`. if (desc.annotationOnly) { hasUnknown = true; continue; @@ -5539,8 +5557,7 @@ ClangTypeEscapability::evaluate(Evaluator &evaluator, maybePushToStack(field->getType()->getUnqualifiedDesugaredType()); continue; } - } - if (type->isArrayType()) { + } else if (type->isArrayType()) { auto elemTy = cast(type) ->getElementType() ->getUnqualifiedDesugaredType(); @@ -8446,21 +8463,25 @@ CxxValueSemantics::evaluate(Evaluator &evaluator, // We assume a type can be imported as Copyable unless: // - There's no copy constructor // - The type has a SWIFT_NONCOPYABLE annotation - // - The type has a SWIFT_COPYABLE_IF(T...) annotation, where at least one of T is ~Copyable - // - It is one of the STL types in `STLConditionalParams` + // - The type has a SWIFT_COPYABLE_IF(T...) annotation, where at least one of + // T is ~Copyable + // - It is one of the STL types in `STLConditionalParams`, and at least one of + // its revelant types is ~Copyable const auto *type = desc.type->getUnqualifiedDesugaredType(); auto *importerImpl = desc.importerImpl; - llvm::SmallVector stack; + bool hasUnknown = false; + llvm::SmallVector stack; // Keep track of Decls we've seen to avoid cycles - llvm::SmallDenseSet seen; + llvm::SmallDenseSet seen; - std::function maybePushToStack = [&](const clang::Type *type) { - auto recordDecl = type->getAsRecordDecl(); - if (!recordDecl) + auto maybePushToStack = [&](const clang::Type *type) { + auto recordType = type->getAs(); + if (!recordType) return; + auto recordDecl = recordType->getDecl(); if (seen.insert(recordDecl).second) { // When a reference type is copied, the pointer’s value is copied rather // than the object’s storage. This means reference types can be imported @@ -8481,7 +8502,7 @@ CxxValueSemantics::evaluate(Evaluator &evaluator, maybePushToStack(type); while (!stack.empty()) { - clang::RecordDecl *recordDecl = stack.back(); + const clang::RecordDecl *recordDecl = stack.back(); stack.pop_back(); if (!hasNonCopyableAttr(recordDecl)) { @@ -8495,22 +8516,7 @@ CxxValueSemantics::evaluate(Evaluator &evaluator, auto conditionalParams = getConditionalCopyableAttrParams(recordDecl); if (!STLParams.empty() || !conditionalParams.empty()) { - HeaderLoc loc{recordDecl->getLocation()}; - std::function checkArgValueSemantics = - [&](clang::TemplateArgument &arg, - StringRef argToCheck) -> std::optional { - if (arg.getKind() != clang::TemplateArgument::Type) { - if (importerImpl) - importerImpl->diagnose( - loc, diag::type_template_parameter_expected, argToCheck); - return std::nullopt; - } - maybePushToStack(arg.getAsType()->getUnqualifiedDesugaredType()); - // FIXME: return std::nullopt for now, while we don't refactor ClangTypeEscapability request - return std::nullopt; - }; - - checkConditionalParams( + hasUnknown &= checkConditionalParams( recordDecl, importerImpl, STLParams, conditionalParams, maybePushToStack); @@ -8525,7 +8531,7 @@ CxxValueSemantics::evaluate(Evaluator &evaluator, } const auto cxxRecordDecl = dyn_cast(recordDecl); - if (!cxxRecordDecl || !cxxRecordDecl->isCompleteDefinition()) { + if (!cxxRecordDecl || !recordDecl->isCompleteDefinition()) { if (hasNonCopyableAttr(recordDecl)) return CxxValueSemanticsKind::MoveOnly; continue; @@ -8539,9 +8545,11 @@ CxxValueSemantics::evaluate(Evaluator &evaluator, (!isCopyable && !isMovable)) { if (hasConstructorWithUnsupportedDefaultArgs(cxxRecordDecl)) - return CxxValueSemanticsKind::UnavailableConstructors; - - return CxxValueSemanticsKind::MissingLifetimeOperation; + importerImpl->addImportDiagnostic( + cxxRecordDecl, Diagnostic(diag::record_unsupported_default_args), + cxxRecordDecl->getLocation()); + hasUnknown = true; + continue; } if (hasNonCopyableAttr(cxxRecordDecl) && isMovable) @@ -8556,7 +8564,8 @@ CxxValueSemantics::evaluate(Evaluator &evaluator, llvm_unreachable("Could not classify C++ type."); } - return CxxValueSemanticsKind::Copyable; + return hasUnknown ? CxxValueSemanticsKind::Unknown + : CxxValueSemanticsKind::Copyable; } void swift::simple_display(llvm::raw_ostream &out, diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp index e1fbb751bd557..cfee7b70209d6 100644 --- a/lib/ClangImporter/ImportDecl.cpp +++ b/lib/ClangImporter/ImportDecl.cpp @@ -2103,11 +2103,11 @@ namespace { return importer::recordHasReferenceSemantics(decl, &Impl); } - bool recordHasMoveOnlySemantics(const clang::RecordDecl *decl) { + bool recordIsCopyable(const clang::RecordDecl *decl) { auto semanticsKind = evaluateOrDefault( Impl.SwiftContext.evaluator, CxxValueSemantics({decl->getTypeForDecl(), &Impl}), {}); - return semanticsKind == CxxValueSemanticsKind::MoveOnly; + return semanticsKind == CxxValueSemanticsKind::Copyable; } void markReturnsUnsafeNonescapable(AbstractFunctionDecl *fd) { @@ -2286,7 +2286,7 @@ namespace { loc, ArrayRef(), nullptr, dc); Impl.ImportedDecls[{decl->getCanonicalDecl(), getVersion()}] = result; - if (recordHasMoveOnlySemantics(decl)) { + if (!recordIsCopyable(decl)) { if (decl->isInStdNamespace() && decl->getName() == "promise") { // Do not import std::promise. return nullptr; @@ -3170,8 +3170,7 @@ namespace { auto valueSemanticsKind = evaluateOrDefault( Impl.SwiftContext.evaluator, CxxValueSemantics({decl->getTypeForDecl(), &Impl}), {}); - if (valueSemanticsKind == CxxValueSemanticsKind::MissingLifetimeOperation || - valueSemanticsKind == CxxValueSemanticsKind::UnavailableConstructors) { + if (valueSemanticsKind == CxxValueSemanticsKind::Unknown) { HeaderLoc loc(decl->getLocation()); if (hasUnsafeAPIAttr(decl)) @@ -3184,12 +3183,6 @@ namespace { Impl.diagnose(loc, diag::api_pattern_attr_ignored, "import_iterator", decl->getNameAsString()); - if (valueSemanticsKind == CxxValueSemanticsKind::UnavailableConstructors) { - Impl.addImportDiagnostic( - decl, Diagnostic(diag::record_unsupported_default_args), - decl->getLocation()); - } - Impl.addImportDiagnostic( decl, Diagnostic(diag::record_not_automatically_importable, @@ -3426,7 +3419,7 @@ namespace { auto semanticsKind = evaluateOrDefault( Impl.SwiftContext.evaluator, CxxValueSemantics({parent->getTypeForDecl(), &Impl}), {}); - if (semanticsKind == CxxValueSemanticsKind::MissingLifetimeOperation) + if (semanticsKind == CxxValueSemanticsKind::Unknown) return nullptr; } diff --git a/lib/ClangImporter/SwiftDeclSynthesizer.cpp b/lib/ClangImporter/SwiftDeclSynthesizer.cpp index 19b774af92d83..0e5f9742e2280 100644 --- a/lib/ClangImporter/SwiftDeclSynthesizer.cpp +++ b/lib/ClangImporter/SwiftDeclSynthesizer.cpp @@ -3074,8 +3074,7 @@ FuncDecl *SwiftDeclSynthesizer::findExplicitDestroy( ctx.evaluator, CxxValueSemantics({clangType->getTypeForDecl(), &ImporterImpl}), {}); - if (valueSemanticsKind != CxxValueSemanticsKind::Copyable && - valueSemanticsKind != CxxValueSemanticsKind::MoveOnly) + if (valueSemanticsKind == CxxValueSemanticsKind::Unknown) return nullptr; auto cxxRecordSemanticsKind = evaluateOrDefault(