From c81325461afdc602b0901fa06f9d6f988381622b Mon Sep 17 00:00:00 2001 From: Egor Zhdan Date: Tue, 4 Apr 2023 14:45:39 +0100 Subject: [PATCH] [cxx-interop] Treat un-instantiated templated types as unsafe When determining whether a C++ method is safe to be imported, we look at its return type to see if it stores any pointers in its fields. If the type is templated, we might not have its definition available yet. Unfortunately we cannot instantiate it on the spot, since the Clang AST would be read and written at the same time. Let's stay on the safe side and treat such methods as unsafe. rdar://107609381 --- lib/ClangImporter/ClangImporter.cpp | 9 ++++-- .../Cxx/class/Inputs/type-classification.h | 28 +++++++++++++++++++ ...type-classification-module-interface.swift | 15 ++++++++++ .../templates/Inputs/large-class-templates.h | 22 +++++++-------- 4 files changed, 60 insertions(+), 14 deletions(-) diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index f0b84a7cc1609..395b2a0d2d58b 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -6395,7 +6395,9 @@ static bool hasIteratorAPIAttr(const clang::Decl *decl) { static bool hasPointerInSubobjects(const clang::CXXRecordDecl *decl) { // Probably a class template that has not yet been specialized: if (!decl->getDefinition()) - return false; + // If the definition is unknown, there is no way to determine if the type + // stores pointers. Stay on the safe side and assume that it does. + return true; auto checkType = [](clang::QualType t) { if (t->isPointerType()) @@ -6690,9 +6692,10 @@ bool IsSafeUseOfCxxDecl::evaluate(Evaluator &evaluator, return false; } - // Mark this as safe to help our diganostics down the road. if (!cxxRecordReturnType->getDefinition()) { - return true; + // This is a templated type that has not been instantiated yet. We do + // not know if it is safe. Assume that it isn't. + return false; } if (!cxxRecordReturnType->hasUserDeclaredCopyConstructor() && diff --git a/test/Interop/Cxx/class/Inputs/type-classification.h b/test/Interop/Cxx/class/Inputs/type-classification.h index 205c456ff3d10..ccc91241b9e42 100644 --- a/test/Interop/Cxx/class/Inputs/type-classification.h +++ b/test/Interop/Cxx/class/Inputs/type-classification.h @@ -213,4 +213,32 @@ struct HasMethodThatReturnsIteratorBox { IteratorBox getIteratorBox() const; }; +template +struct TemplatedPointerBox { + T *ptr; +}; + +struct HasMethodThatReturnsTemplatedPointerBox { + TemplatedPointerBox getTemplatedPointerBox() const; +}; + +template +struct TemplatedBox { + T value; +}; + +struct HasMethodThatReturnsTemplatedBox { + TemplatedBox getIntBox() const; + TemplatedBox getIntPtrBox() const; +}; + +template +struct __attribute__((swift_attr("import_iterator"))) TemplatedIterator { + T idx; +}; + +struct HasMethodThatReturnsTemplatedIterator { + TemplatedIterator getIterator() const; +}; + #endif // TEST_INTEROP_CXX_CLASS_INPUTS_TYPE_CLASSIFICATION_H diff --git a/test/Interop/Cxx/class/type-classification-module-interface.swift b/test/Interop/Cxx/class/type-classification-module-interface.swift index f2bbc3436134f..f571f4327d7b0 100644 --- a/test/Interop/Cxx/class/type-classification-module-interface.swift +++ b/test/Interop/Cxx/class/type-classification-module-interface.swift @@ -35,3 +35,18 @@ // CHECK: func __getIteratorBoxUnsafe() -> IteratorBox // CHECK-SKIP-UNSAFE-NOT: func __getIteratorBoxUnsafe() -> IteratorBox // CHECK: } + +// CHECK: struct HasMethodThatReturnsTemplatedPointerBox { +// CHECK: func __getTemplatedPointerBoxUnsafe() -> TemplatedPointerBox +// CHECK-SKIP-UNSAFE-NOT: func __getTemplatedPointerBoxUnsafe() -> TemplatedPointerBox +// CHECK: } + +// CHECK: struct HasMethodThatReturnsTemplatedBox { +// FIXME: This is unfortunate, we should be able to recognize that TemplatedBox does not store any pointers as fields. +// CHECK: func __getIntBoxUnsafe() -> TemplatedBox +// CHECK: func __getIntPtrBoxUnsafe() +// CHECK: } + +// CHECK: struct HasMethodThatReturnsTemplatedIterator { +// CHECK: func __getIteratorUnsafe() +// CHECK: } diff --git a/test/Interop/Cxx/templates/Inputs/large-class-templates.h b/test/Interop/Cxx/templates/Inputs/large-class-templates.h index dbefed620bd3b..4f0b93138f0ce 100644 --- a/test/Interop/Cxx/templates/Inputs/large-class-templates.h +++ b/test/Interop/Cxx/templates/Inputs/large-class-templates.h @@ -28,17 +28,17 @@ struct ValExpr { using type = typename E::type; E expr; - ValExpr> test1() { return {SliceExpr{expr}}; } - ValExpr> test2() { return {SliceExpr{expr}}; } - ValExpr> test3() { return {SliceExpr{expr}}; } - ValExpr> test4() { return {SliceExpr{expr}}; } - ValExpr> test5() { return {SliceExpr{expr}}; } - ValExpr> test6() { return {SliceExpr{expr}}; } - ValExpr> test7() { return {SliceExpr{expr}}; } - ValExpr> test8() { return {SliceExpr{expr}}; } - ValExpr> test9() { return {SliceExpr{expr}}; } - ValExpr> test11() { return {SliceExpr{expr}}; } - ValExpr> test12() { return {SliceExpr{expr}}; } + ValExpr> test1() __attribute__((swift_attr("import_unsafe"))) { return {SliceExpr{expr}}; } + ValExpr> test2() __attribute__((swift_attr("import_unsafe"))) { return {SliceExpr{expr}}; } + ValExpr> test3() __attribute__((swift_attr("import_unsafe"))) { return {SliceExpr{expr}}; } + ValExpr> test4() __attribute__((swift_attr("import_unsafe"))) { return {SliceExpr{expr}}; } + ValExpr> test5() __attribute__((swift_attr("import_unsafe"))) { return {SliceExpr{expr}}; } + ValExpr> test6() __attribute__((swift_attr("import_unsafe"))) { return {SliceExpr{expr}}; } + ValExpr> test7() __attribute__((swift_attr("import_unsafe"))) { return {SliceExpr{expr}}; } + ValExpr> test8() __attribute__((swift_attr("import_unsafe"))) { return {SliceExpr{expr}}; } + ValExpr> test9() __attribute__((swift_attr("import_unsafe"))) { return {SliceExpr{expr}}; } + ValExpr> test11() __attribute__((swift_attr("import_unsafe"))) { return {SliceExpr{expr}}; } + ValExpr> test12() __attribute__((swift_attr("import_unsafe"))) { return {SliceExpr{expr}}; } }; // This class template is exponentially slow to *fully* instantiate (and the