Skip to content

[cxx-interop] Support nested templates for conditional escapability #77678

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 20, 2024
Merged

[cxx-interop] Support nested templates for conditional escapability #77678

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 32 additions & 20 deletions lib/ClangImporter/ClangImporter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include "swift/AST/NameLookupRequests.h"
#include "swift/AST/PrettyStackTrace.h"
#include "swift/AST/SourceFile.h"
#include "swift/AST/Type.h"
#include "swift/AST/TypeCheckRequests.h"
#include "swift/AST/Types.h"
#include "swift/Basic/Assertions.h"
Expand All @@ -51,6 +52,7 @@
#include "swift/Strings.h"
#include "swift/Subsystems.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/DeclBase.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/Mangle.h"
Expand Down Expand Up @@ -5080,31 +5082,41 @@ ClangTypeEscapability::evaluate(Evaluator &evaluator,
importer::getConditionalEscapableAttrParams(recordDecl);
if (!conditionalParams.empty()) {
auto specDecl = cast<clang::ClassTemplateSpecializationDecl>(recordDecl);
auto templateDecl = specDecl->getSpecializedTemplate();
SmallVector<std::pair<unsigned, StringRef>, 4> argumentsToCheck;
for (auto [idx, param] :
llvm::enumerate(*templateDecl->getTemplateParameters())) {
if (conditionalParams.erase(param->getName()))
argumentsToCheck.push_back(std::make_pair(idx, param->getName()));
}
HeaderLoc loc{recordDecl->getLocation()};
for (auto name : conditionalParams)
desc.impl.diagnose(loc, diag::unknown_template_parameter, name);

auto &argList = specDecl->getTemplateArgs();
for (auto argToCheck : argumentsToCheck) {
auto arg = argList[argToCheck.first];
if (arg.getKind() != clang::TemplateArgument::Type) {
desc.impl.diagnose(loc, diag::type_template_parameter_expected,
argToCheck.second);
return CxxEscapability::Unknown;
while (specDecl) {
auto templateDecl = specDecl->getSpecializedTemplate();
for (auto [idx, param] :
llvm::enumerate(*templateDecl->getTemplateParameters())) {
if (conditionalParams.erase(param->getName()))
argumentsToCheck.push_back(std::make_pair(idx, param->getName()));
}
auto &argList = specDecl->getTemplateArgs();
for (auto argToCheck : argumentsToCheck) {
auto arg = argList[argToCheck.first];
if (arg.getKind() != clang::TemplateArgument::Type) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if this would work with variadic templates, e.g. std::tuple? But that's out of scope of this patch 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! I think this would not work for that. I'd address that in a follow-up PR.

desc.impl.diagnose(loc, diag::type_template_parameter_expected,
argToCheck.second);
return CxxEscapability::Unknown;
}

auto argEscapability = evaluateEscapability(
arg.getAsType()->getUnqualifiedDesugaredType());
if (argEscapability == CxxEscapability::NonEscapable)
return CxxEscapability::NonEscapable;
auto argEscapability = evaluateEscapability(
arg.getAsType()->getUnqualifiedDesugaredType());
if (argEscapability == CxxEscapability::NonEscapable)
return CxxEscapability::NonEscapable;
}
clang::DeclContext *rec = specDecl;
specDecl = nullptr;
while ((rec = rec->getParent())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this loop stop once rec->isTranslationUnit()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When rec->isTranslationUnit() is true, rec->getParent() returns null and the loop terminates. That being said, I just realized rec is a bit confusing name here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, that makes sense.

specDecl = dyn_cast<clang::ClassTemplateSpecializationDecl>(rec);
if (specDecl)
break;
}
}

for (auto name : conditionalParams)
desc.impl.diagnose(loc, diag::unknown_template_parameter, name);

return hadUnknown ? CxxEscapability::Unknown : CxxEscapability::Escapable;
}
if (desc.annotationOnly)
Expand Down
20 changes: 20 additions & 0 deletions test/Interop/Cxx/class/nonescapable-errors.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,21 @@ struct SWIFT_ESCAPABLE_IF(F, S) MyType {
MyPair2<Owner, Owner> i1();
MyType<Owner, 0> i2();

template<typename T>
struct Outer {
struct NonTemplated {
template <typename S>
struct SWIFT_ESCAPABLE_IF(T, S) Inner {
T t;
S s;
};
};
};

Outer<View>::NonTemplated::Inner<Owner> j1();
Outer<Owner>::NonTemplated::Inner<View> j2();
Outer<Owner>::NonTemplated::Inner<Owner> j3();

//--- test.swift

import Test
Expand All @@ -85,6 +100,11 @@ public func noAnnotations() -> View {
// CHECK: nonescapable.h:38:39: error: template parameter 'Missing' does not exist
i2()
// CHECK: nonescapable.h:44:33: error: template parameter 'S' expected to be a type parameter
j1()
// CHECK: nonescapable.h:62:41: error: cannot infer lifetime dependence, no parameters found that are either ~Escapable or Escapable with a borrowing ownership
j2()
// CHECK: nonescapable.h:63:41: error: cannot infer lifetime dependence, no parameters found that are either ~Escapable or Escapable with a borrowing ownership
j3()
// CHECK-NOT: error
// CHECK-NOT: warning
return View()
Expand Down