Merge pull request #79693 from swiftlang/gaborh/remove-feature-flag-check

Author: Xazax-hun

lib/ClangImporter/ClangImporter.cpp

@@ -7792,7 +7792,7 @@ getRefParentOrDiag(const clang::RecordDecl *decl, ASTContext &ctx,
   if (refParentDecls.empty())
     return nullptr;
 
-  std::unordered_set<ValueDecl *> uniqueRetainDecls{}, uniqueReleaseDecls{};
+  std::set<StringRef> uniqueRetainDecls{}, uniqueReleaseDecls{};
   constexpr StringRef retainPrefix = "retain:";
   constexpr StringRef releasePrefix = "release:";
 
@@ -7801,16 +7801,10 @@ getRefParentOrDiag(const clang::RecordDecl *decl, ASTContext &ctx,
     for (const auto *attr : refParentDecl->getAttrs()) {
       if (const auto swiftAttr = llvm::dyn_cast<clang::SwiftAttrAttr>(attr)) {
         const auto &attribute = swiftAttr->getAttribute();
-        llvm::SmallVector<ValueDecl *, 1> valueDecls;
-        if (attribute.starts_with(retainPrefix)) {
-          auto name = attribute.drop_front(retainPrefix.size()).str();
-          valueDecls = getValueDeclsForName(decl, ctx, name);
-          uniqueRetainDecls.insert(valueDecls.begin(), valueDecls.end());
-        } else if (attribute.starts_with(releasePrefix)) {
-          auto name = attribute.drop_front(releasePrefix.size()).str();
-          valueDecls = getValueDeclsForName(decl, ctx, name);
-          uniqueReleaseDecls.insert(valueDecls.begin(), valueDecls.end());
-        }
+        if (attribute.starts_with(retainPrefix))
+          uniqueRetainDecls.insert(attribute.drop_front(retainPrefix.size()));
+        else if (attribute.starts_with(releasePrefix))
+          uniqueReleaseDecls.insert(attribute.drop_front(releasePrefix.size()));
       }
     }
   }
@@ -8175,7 +8169,7 @@ CxxRecordAsSwiftType::evaluate(Evaluator &evaluator,
   return nullptr;
 }
 
-bool anySubobjectsSelfContained(const clang::CXXRecordDecl *decl) {
+static bool anySubobjectsSelfContained(const clang::CXXRecordDecl *decl) {
   // std::pair and std::tuple might have copy and move constructors, or base
   // classes with copy and move constructors, but they are not self-contained
   // types, e.g. `std::pair<UnsafeType, T>`.
@@ -8339,11 +8333,11 @@ CustomRefCountingOperationResult CustomRefCountingOperation::evaluate(
     }
   }
 
-  if (retainReleaseAttrs.empty()) {
+  if (retainReleaseAttrs.empty())
     return {CustomRefCountingOperationResult::noAttribute, nullptr, ""};
-  } else if (retainReleaseAttrs.size() > 1) {
+
+  if (retainReleaseAttrs.size() > 1)
     return {CustomRefCountingOperationResult::tooManyAttributes, nullptr, ""};
-  }
 
   auto name = retainReleaseAttrs.front()
                   ->getAttribute()

lib/ClangImporter/ImportDecl.cpp

@@ -2197,38 +2197,6 @@ namespace {
             loc, std::nullopt, nullptr, dc);
       Impl.ImportedDecls[{decl->getCanonicalDecl(), getVersion()}] = result;
 
-      // We have to do this after populating ImportedDecls to avoid importing
-      // the same multiple times.
-      if (Impl.SwiftContext.LangOpts.hasFeature(
-              Feature::StrictMemorySafety)) {
-        if (const auto *ctsd =
-                dyn_cast<clang::ClassTemplateSpecializationDecl>(decl)) {
-          for (auto arg : ctsd->getTemplateArgs().asArray()) {
-            llvm::SmallVector<clang::TemplateArgument, 1> nonPackArgs;
-            if (arg.getKind() == clang::TemplateArgument::Pack) {
-              auto pack = arg.getPackAsArray();
-              nonPackArgs.assign(pack.begin(), pack.end());
-            } else {
-              nonPackArgs.push_back(arg);
-            }
-            for (auto realArg : nonPackArgs) {
-              if (realArg.getKind() != clang::TemplateArgument::Type)
-                continue;
-              auto SwiftType = Impl.importTypeIgnoreIUO(
-                  realArg.getAsType(), ImportTypeKind::Abstract,
-                  [](Diagnostic &&diag) {}, false, Bridgeability::None,
-                  ImportTypeAttrs());
-              if (SwiftType && SwiftType->isUnsafe()) {
-                auto attr =
-                    new (Impl.SwiftContext) UnsafeAttr(/*implicit=*/true);
-                result->getAttrs().add(attr);
-                break;
-              }
-            }
-          }
-        }
-      }
-
       if (recordHasMoveOnlySemantics(decl)) {
         if (decl->isInStdNamespace() && decl->getName() == "promise") {
           // Do not import std::promise.
@@ -2238,16 +2206,6 @@ namespace {
                                    MoveOnlyAttr(/*Implicit=*/true));
       }
 
-      bool isNonEscapable = false;
-      if (evaluateOrDefault(
-              Impl.SwiftContext.evaluator,
-              ClangTypeEscapability({decl->getTypeForDecl(), &Impl}),
-              CxxEscapability::Unknown) == CxxEscapability::NonEscapable) {
-        result->getAttrs().add(new (Impl.SwiftContext)
-                                   NonEscapableAttr(/*Implicit=*/true));
-        isNonEscapable = true;
-      }
-
       // FIXME: Figure out what to do with superclasses in C++. One possible
       // solution would be to turn them into members and add conversion
       // functions.
@@ -2259,6 +2217,44 @@ namespace {
         }
       }
 
+      // We have to do this after populating ImportedDecls to avoid importing
+      // the same decl multiple times. Also after we imported the bases.
+      if (const auto *ctsd =
+              dyn_cast<clang::ClassTemplateSpecializationDecl>(decl)) {
+        for (auto arg : ctsd->getTemplateArgs().asArray()) {
+          llvm::SmallVector<clang::TemplateArgument, 1> nonPackArgs;
+          if (arg.getKind() == clang::TemplateArgument::Pack) {
+            auto pack = arg.getPackAsArray();
+            nonPackArgs.assign(pack.begin(), pack.end());
+          } else {
+            nonPackArgs.push_back(arg);
+          }
+          for (auto realArg : nonPackArgs) {
+            if (realArg.getKind() != clang::TemplateArgument::Type)
+              continue;
+            auto SwiftType = Impl.importTypeIgnoreIUO(
+                realArg.getAsType(), ImportTypeKind::Abstract,
+                [](Diagnostic &&diag) {}, false, Bridgeability::None,
+                ImportTypeAttrs());
+            if (SwiftType && SwiftType->isUnsafe()) {
+              auto attr = new (Impl.SwiftContext) UnsafeAttr(/*implicit=*/true);
+              result->getAttrs().add(attr);
+              break;
+            }
+          }
+        }
+      }
+
+      bool isNonEscapable = false;
+      if (evaluateOrDefault(
+              Impl.SwiftContext.evaluator,
+              ClangTypeEscapability({decl->getTypeForDecl(), &Impl}),
+              CxxEscapability::Unknown) == CxxEscapability::NonEscapable) {
+        result->getAttrs().add(new (Impl.SwiftContext)
+                                   NonEscapableAttr(/*Implicit=*/true));
+        isNonEscapable = true;
+      }
+
       // Import each of the members.
       SmallVector<VarDecl *, 4> members;
       SmallVector<FuncDecl *, 4> methods;
@@ -2965,7 +2961,7 @@ namespace {
         }
       }
 
-      // It is import that we bail on an unimportable record *before* we import
+      // It is important that we bail on an unimportable record *before* we import
       // any of its members or cache the decl.
       auto semanticsKind = evaluateOrDefault(
           Impl.SwiftContext.evaluator,

test/Interop/Cxx/foreign-reference/Inputs/inheritance.h

@@ -80,9 +80,7 @@ __attribute__((swift_attr("release:immortal"))) ImmortalRefType {};
 
 ImmortalRefType *returnImmortalRefType() { return new ImmortalRefType(); };
 
-// Doubt: Is it fine to not infer in the case of SWIFT_IMMORTAL_REFERENCE
-// rdar://145193396
-struct DerivedFromImmortalRefType : ImmortalRefType {}; // expected-warning {{unable to infer SWIFT_SHARED_REFERENCE for 'DerivedFromImmortalRefType', although one of its transitive base types is marked as SWIFT_SHARED_REFERENCE}}
+struct DerivedFromImmortalRefType : ImmortalRefType {};
 DerivedFromImmortalRefType *returnDerivedFromImmortalRefType() {
     return new DerivedFromImmortalRefType();
 };
@@ -284,7 +282,8 @@ __attribute__((swift_attr("retain:sameretain")))
 __attribute__((swift_attr("release:samerelease"))) B2 {};  // expected-error {{multiple functions 'sameretain' found; there must be exactly one retain function for reference type 'B2'}}
                                                             // expected-error@-1 {{multiple functions 'samerelease' found; there must be exactly one release function for reference type 'B2'}}
 
-struct D : B1, B2 {}; // expected-warning {{unable to infer SWIFT_SHARED_REFERENCE for 'D', although one of its transitive base types is marked as SWIFT_SHARED_REFERENCE}}
+struct D : B1, B2 {}; // expected-error {{multiple functions 'sameretain' found; there must be exactly one retain function for reference type 'D'}}
+                      // expected-error@-1 {{multiple functions 'samerelease' found; there must be exactly one release function for reference type 'D'}}
 D *returnD() { return new D(); };
 } // namespace OverloadedRetainRelease
 

test/Interop/Cxx/foreign-reference/inheritance.swift

@@ -50,7 +50,7 @@ FrtInheritanceTestSuite.test("ParentChild") {
 
   let derivedFromImmortalRefType = ImmortalRefereceExample.returnDerivedFromImmortalRefType()
   expectTrue(
-    !(type(of: derivedFromImmortalRefType) is AnyObject.Type),
+    type(of: derivedFromImmortalRefType) is AnyObject.Type,
     "Expected derivedFromImmortalRefType to be a value type, but it’s a reference type")
 
   let valType = ExplicitAnnotationHasPrecedence1.returnValueType()