Merge pull request #80204 from swiftlang/egorzhdan/symbolic-missing-lifetime

[cxx-interop] Avoid diagnosing missing lifetime operations in symbolic mode

Author: egorzhdan

include/swift/ClangImporter/ClangImporterRequests.h

@@ -360,16 +360,9 @@ struct CxxRecordSemanticsDescriptor final {
   ASTContext &ctx;
   ClangImporter::Implementation *importerImpl;
 
-  /// Whether to emit warnings for missing destructor or copy constructor
-  /// whenever the classification of the type assumes that they exist (e.g. for
-  /// a value type).
-  bool shouldDiagnoseLifetimeOperations;
-
   CxxRecordSemanticsDescriptor(const clang::RecordDecl *decl, ASTContext &ctx,
-                               ClangImporter::Implementation *importerImpl,
-                               bool shouldDiagnoseLifetimeOperations = true)
-      : decl(decl), ctx(ctx), importerImpl(importerImpl),
-        shouldDiagnoseLifetimeOperations(shouldDiagnoseLifetimeOperations) {}
+                               ClangImporter::Implementation *importerImpl)
+      : decl(decl), ctx(ctx), importerImpl(importerImpl) {}
 
   friend llvm::hash_code hash_value(const CxxRecordSemanticsDescriptor &desc) {
     return llvm::hash_combine(desc.decl);

lib/ClangImporter/ClangImporter.cpp

@@ -7953,15 +7953,15 @@ static bool hasSwiftAttribute(const clang::Decl *decl, StringRef attr) {
   return false;
 }
 
-static bool hasOwnedValueAttr(const clang::RecordDecl *decl) {
+bool importer::hasOwnedValueAttr(const clang::RecordDecl *decl) {
   return hasSwiftAttribute(decl, "import_owned");
 }
 
 bool importer::hasUnsafeAPIAttr(const clang::Decl *decl) {
   return hasSwiftAttribute(decl, "import_unsafe");
 }
 
-static bool hasIteratorAPIAttr(const clang::Decl *decl) {
+bool importer::hasIteratorAPIAttr(const clang::Decl *decl) {
   return hasSwiftAttribute(decl, "import_iterator");
 }
 
@@ -8197,18 +8197,6 @@ CxxRecordSemantics::evaluate(Evaluator &evaluator,
 
   if (!hasDestroyTypeOperations(cxxDecl) ||
       (!hasCopyTypeOperations(cxxDecl) && !hasMoveTypeOperations(cxxDecl))) {
-    if (desc.shouldDiagnoseLifetimeOperations) {
-      HeaderLoc loc(decl->getLocation());
-      if (hasUnsafeAPIAttr(cxxDecl))
-        importerImpl->diagnose(loc, diag::api_pattern_attr_ignored,
-                               "import_unsafe", decl->getNameAsString());
-      if (hasOwnedValueAttr(cxxDecl))
-        importerImpl->diagnose(loc, diag::api_pattern_attr_ignored,
-                               "import_owned", decl->getNameAsString());
-      if (hasIteratorAPIAttr(cxxDecl))
-        importerImpl->diagnose(loc, diag::api_pattern_attr_ignored,
-                               "import_iterator", decl->getNameAsString());
-    }
 
     if (hasConstructorWithUnsupportedDefaultArgs(cxxDecl))
       return CxxRecordSemanticsKind::UnavailableConstructors;

lib/ClangImporter/ImportDecl.cpp

@@ -173,14 +173,9 @@ bool importer::recordHasReferenceSemantics(
   // return MissingLifetimeOperation if the type is not a foreign reference
   // type. Note that this doesn't affect the correctness of this function, since
   // those implicit members aren't required for foreign reference types.
-
-  // To avoid emitting spurious diagnostics, let's disable them here. Types with
-  // missing lifetime operations would get diagnosed later, once their members
-  // are fully instantiated.
   auto semanticsKind = evaluateOrDefault(
       importerImpl->SwiftContext.evaluator,
-      CxxRecordSemantics({decl, importerImpl->SwiftContext, importerImpl,
-                          /* shouldDiagnoseLifetimeOperations */ false}),
+      CxxRecordSemantics({decl, importerImpl->SwiftContext, importerImpl}),
       {});
   return semanticsKind == CxxRecordSemanticsKind::Reference;
 }
@@ -2978,6 +2973,17 @@ namespace {
           // members once they're instantiated.
           !Impl.importSymbolicCXXDecls) {
 
+        HeaderLoc loc(decl->getLocation());
+        if (hasUnsafeAPIAttr(decl))
+          Impl.diagnose(loc, diag::api_pattern_attr_ignored, "import_unsafe",
+                        decl->getNameAsString());
+        if (hasOwnedValueAttr(decl))
+          Impl.diagnose(loc, diag::api_pattern_attr_ignored, "import_owned",
+                        decl->getNameAsString());
+        if (hasIteratorAPIAttr(decl))
+          Impl.diagnose(loc, diag::api_pattern_attr_ignored, "import_iterator",
+                        decl->getNameAsString());
+
         if (semanticsKind == CxxRecordSemanticsKind::UnavailableConstructors) {
           Impl.addImportDiagnostic(
               decl, Diagnostic(diag::record_unsupported_default_args),

lib/ClangImporter/ImporterImpl.h

@@ -2051,7 +2051,9 @@ inline std::string getPrivateOperatorName(const std::string &OperatorToken) {
   return "None";
 }
 
+bool hasOwnedValueAttr(const clang::RecordDecl *decl);
 bool hasUnsafeAPIAttr(const clang::Decl *decl);
+bool hasIteratorAPIAttr(const clang::Decl *decl);
 
 bool hasNonEscapableAttr(const clang::RecordDecl *decl);
 

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

@@ -31,6 +31,8 @@ int freeFunction(int x, int y);
 import CxxStdlib
 import CxxModule
 
+// CHECK-NOT: warning: 'import_owned' Swift attribute ignored on type
+
 // REMARK_NEW: remark: emitting symbolic interface at {{.*}}/interfaces/std-{{.*}}.pcm.symbolicswiftinterface{{$}}
 // REMARK_NEW: remark: emitting symbolic interface at {{.*}}/interfaces/CxxModule-{{.*}}.pcm.symbolicswiftinterface{{$}}
 

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

@@ -1,6 +1,6 @@
 // RUN: rm -rf %t
 // RUN: split-file %s %t
-// 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
+// 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
 
 // REQUIRES: swift_feature_ImportSymbolicCXXDecls
 
@@ -60,6 +60,11 @@ public:
     };
 };
 
+template <class T>
+struct __attribute__((swift_attr("import_owned"))) AnnotatedTemplate {
+    T t;
+};
+
 #define IMMORTAL_FRT                                                         \
     __attribute__((swift_attr("import_reference")))                              \
     __attribute__((swift_attr("retain:immortal")))                               \
@@ -73,6 +78,8 @@ struct NonCopyable {
     NonCopyable(const NonCopyable& other) = delete;
 };
 
+// CHECK-NOT: warning: 'import_owned' Swift attribute ignored on type
+
 // CHECK:     enum ns {
 // CHECK-NEXT: struct B {
 // CHECK-NEXT:    init()