Skip to content

Commit 7c49480

Browse files
committed
[cxx-interop] Avoid diagnosing missing lifetime operations in symbolic mode
When importing C++ decls in symbolic mode, class templates are not instantiated, which means they might not have a destructor or a move constructor. Make sure we are not trying to diagnose those missing lifetime operations in symbolic mode. This fixes incorrect diagnostics that were emitted during indexing at the end of compilation: ``` warning: 'import_owned' Swift attribute ignored on type 'basic_string': type is not copyable or destructible ``` As a nice side effect, this moves the logic that emits these diagnostics from the request body, which might be invoked many times, to the importer itself, which is only invoked once per C++ class. rdar://147421710
1 parent 922a01d commit 7c49480

File tree

6 files changed

+28
-30
lines changed

6 files changed

+28
-30
lines changed

include/swift/ClangImporter/ClangImporterRequests.h

+2-9
Original file line numberDiff line numberDiff line change
@@ -360,16 +360,9 @@ struct CxxRecordSemanticsDescriptor final {
360360
ASTContext &ctx;
361361
ClangImporter::Implementation *importerImpl;
362362

363-
/// Whether to emit warnings for missing destructor or copy constructor
364-
/// whenever the classification of the type assumes that they exist (e.g. for
365-
/// a value type).
366-
bool shouldDiagnoseLifetimeOperations;
367-
368363
CxxRecordSemanticsDescriptor(const clang::RecordDecl *decl, ASTContext &ctx,
369-
ClangImporter::Implementation *importerImpl,
370-
bool shouldDiagnoseLifetimeOperations = true)
371-
: decl(decl), ctx(ctx), importerImpl(importerImpl),
372-
shouldDiagnoseLifetimeOperations(shouldDiagnoseLifetimeOperations) {}
364+
ClangImporter::Implementation *importerImpl)
365+
: decl(decl), ctx(ctx), importerImpl(importerImpl) {}
373366

374367
friend llvm::hash_code hash_value(const CxxRecordSemanticsDescriptor &desc) {
375368
return llvm::hash_combine(desc.decl);

lib/ClangImporter/ClangImporter.cpp

+2-14
Original file line numberDiff line numberDiff line change
@@ -7953,15 +7953,15 @@ static bool hasSwiftAttribute(const clang::Decl *decl, StringRef attr) {
79537953
return false;
79547954
}
79557955

7956-
static bool hasOwnedValueAttr(const clang::RecordDecl *decl) {
7956+
bool importer::hasOwnedValueAttr(const clang::RecordDecl *decl) {
79577957
return hasSwiftAttribute(decl, "import_owned");
79587958
}
79597959

79607960
bool importer::hasUnsafeAPIAttr(const clang::Decl *decl) {
79617961
return hasSwiftAttribute(decl, "import_unsafe");
79627962
}
79637963

7964-
static bool hasIteratorAPIAttr(const clang::Decl *decl) {
7964+
bool importer::hasIteratorAPIAttr(const clang::Decl *decl) {
79657965
return hasSwiftAttribute(decl, "import_iterator");
79667966
}
79677967

@@ -8197,18 +8197,6 @@ CxxRecordSemantics::evaluate(Evaluator &evaluator,
81978197

81988198
if (!hasDestroyTypeOperations(cxxDecl) ||
81998199
(!hasCopyTypeOperations(cxxDecl) && !hasMoveTypeOperations(cxxDecl))) {
8200-
if (desc.shouldDiagnoseLifetimeOperations) {
8201-
HeaderLoc loc(decl->getLocation());
8202-
if (hasUnsafeAPIAttr(cxxDecl))
8203-
importerImpl->diagnose(loc, diag::api_pattern_attr_ignored,
8204-
"import_unsafe", decl->getNameAsString());
8205-
if (hasOwnedValueAttr(cxxDecl))
8206-
importerImpl->diagnose(loc, diag::api_pattern_attr_ignored,
8207-
"import_owned", decl->getNameAsString());
8208-
if (hasIteratorAPIAttr(cxxDecl))
8209-
importerImpl->diagnose(loc, diag::api_pattern_attr_ignored,
8210-
"import_iterator", decl->getNameAsString());
8211-
}
82128200

82138201
if (hasConstructorWithUnsupportedDefaultArgs(cxxDecl))
82148202
return CxxRecordSemanticsKind::UnavailableConstructors;

lib/ClangImporter/ImportDecl.cpp

+12-6
Original file line numberDiff line numberDiff line change
@@ -173,14 +173,9 @@ bool importer::recordHasReferenceSemantics(
173173
// return MissingLifetimeOperation if the type is not a foreign reference
174174
// type. Note that this doesn't affect the correctness of this function, since
175175
// those implicit members aren't required for foreign reference types.
176-
177-
// To avoid emitting spurious diagnostics, let's disable them here. Types with
178-
// missing lifetime operations would get diagnosed later, once their members
179-
// are fully instantiated.
180176
auto semanticsKind = evaluateOrDefault(
181177
importerImpl->SwiftContext.evaluator,
182-
CxxRecordSemantics({decl, importerImpl->SwiftContext, importerImpl,
183-
/* shouldDiagnoseLifetimeOperations */ false}),
178+
CxxRecordSemantics({decl, importerImpl->SwiftContext, importerImpl}),
184179
{});
185180
return semanticsKind == CxxRecordSemanticsKind::Reference;
186181
}
@@ -2978,6 +2973,17 @@ namespace {
29782973
// members once they're instantiated.
29792974
!Impl.importSymbolicCXXDecls) {
29802975

2976+
HeaderLoc loc(decl->getLocation());
2977+
if (hasUnsafeAPIAttr(decl))
2978+
Impl.diagnose(loc, diag::api_pattern_attr_ignored, "import_unsafe",
2979+
decl->getNameAsString());
2980+
if (hasOwnedValueAttr(decl))
2981+
Impl.diagnose(loc, diag::api_pattern_attr_ignored, "import_owned",
2982+
decl->getNameAsString());
2983+
if (hasIteratorAPIAttr(decl))
2984+
Impl.diagnose(loc, diag::api_pattern_attr_ignored, "import_iterator",
2985+
decl->getNameAsString());
2986+
29812987
if (semanticsKind == CxxRecordSemanticsKind::UnavailableConstructors) {
29822988
Impl.addImportDiagnostic(
29832989
decl, Diagnostic(diag::record_unsupported_default_args),

lib/ClangImporter/ImporterImpl.h

+2
Original file line numberDiff line numberDiff line change
@@ -2051,7 +2051,9 @@ inline std::string getPrivateOperatorName(const std::string &OperatorToken) {
20512051
return "None";
20522052
}
20532053

2054+
bool hasOwnedValueAttr(const clang::RecordDecl *decl);
20542055
bool hasUnsafeAPIAttr(const clang::Decl *decl);
2056+
bool hasIteratorAPIAttr(const clang::Decl *decl);
20552057

20562058
bool hasNonEscapableAttr(const clang::RecordDecl *decl);
20572059

test/Interop/Cxx/symbolic-imports/indexing-emit-libcxx-symbolic-module-interface.swift

+2
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ int freeFunction(int x, int y);
3131
import CxxStdlib
3232
import CxxModule
3333

34+
// CHECK-NOT: warning: 'import_owned' Swift attribute ignored on type
35+
3436
// REMARK_NEW: remark: emitting symbolic interface at {{.*}}/interfaces/std-{{.*}}.pcm.symbolicswiftinterface{{$}}
3537
// REMARK_NEW: remark: emitting symbolic interface at {{.*}}/interfaces/CxxModule-{{.*}}.pcm.symbolicswiftinterface{{$}}
3638

test/Interop/Cxx/symbolic-imports/print-symbolic-module-interface.swift

+8-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// RUN: rm -rf %t
22
// RUN: split-file %s %t
3-
// RUN: %target-swift-ide-test -print-module -module-to-print=CxxModule -I %t/Inputs -source-filename=x -enable-experimental-cxx-interop -enable-experimental-feature ImportSymbolicCXXDecls | %FileCheck %s
3+
// RUN: %target-swift-ide-test -print-module -module-to-print=CxxModule -I %t/Inputs -source-filename=x -enable-experimental-cxx-interop -enable-experimental-feature ImportSymbolicCXXDecls 2>&1 | %FileCheck %s
44

55
// REQUIRES: swift_feature_ImportSymbolicCXXDecls
66

@@ -60,6 +60,11 @@ public:
6060
};
6161
};
6262

63+
template <class T>
64+
struct __attribute__((swift_attr("import_owned"))) AnnotatedTemplate {
65+
T t;
66+
};
67+
6368
#define IMMORTAL_FRT \
6469
__attribute__((swift_attr("import_reference"))) \
6570
__attribute__((swift_attr("retain:immortal"))) \
@@ -73,6 +78,8 @@ struct NonCopyable {
7378
NonCopyable(const NonCopyable& other) = delete;
7479
};
7580

81+
// CHECK-NOT: warning: 'import_owned' Swift attribute ignored on type
82+
7683
// CHECK: enum ns {
7784
// CHECK-NEXT: struct B {
7885
// CHECK-NEXT: init()

0 commit comments

Comments
 (0)