[SE-0458] Bring implementation in line with the latest proposal revision

include/swift/AST/DiagnosticsSema.def

@@ -8149,15 +8149,22 @@ NOTE(note_reference_exclusivity_unchecked,none,
 NOTE(note_use_of_unsafe_conformance_is_unsafe,none,
      "@unsafe conformance of %0 to %kind1 involves unsafe code",
      (Type, const ValueDecl *))
+NOTE(note_unsafe_storage,none,
+     "%kindbase1 involves unsafe type %2", (bool, const ValueDecl *, Type))
 
-GROUPED_WARNING(decl_signature_involves_unsafe,Unsafe,none,
-      "%kindbase0 has an interface that involves unsafe types",
+GROUPED_WARNING(decl_unsafe_storage,Unsafe,none,
+      "%kindbase0 has storage involving unsafe types",
       (const Decl *))
-NOTE(decl_signature_mark_unsafe,none,
-      "add '@unsafe' to indicate that this declaration is unsafe to use",
+NOTE(decl_storage_mark_unsafe,none,
+      "add '@unsafe' if this type is also unsafe to use",
       ())
-NOTE(decl_signature_mark_safe,none,
-      "add '@safe' to indicate that this declaration is memory-safe to use", ())
+NOTE(decl_storage_mark_safe,none,
+      "add '@safe' if this type encapsulates the unsafe storage in "
+      "a safe interface", ())
+
+GROUPED_WARNING(unsafe_superclass,Unsafe,none,
+     "%kindbase0 has superclass involving unsafe type %1", 
+     (const ValueDecl *, Type))
 
 GROUPED_WARNING(conformance_involves_unsafe,Unsafe,none,
      "conformance of %0 to %kind1 involves unsafe code; use '@unsafe' to "

include/swift/AST/UnsafeUse.h

@@ -45,6 +45,8 @@ class UnsafeUse {
     NonisolatedUnsafe,
     /// A reference to an unsafe declaration.
     ReferenceToUnsafe,
+    /// A reference to an unsafe storage.
+    ReferenceToUnsafeStorage,
     /// A reference to a typealias that is not itself unsafe, but has
     /// an unsafe underlying type.
     ReferenceToUnsafeThroughTypealias,
@@ -113,6 +115,7 @@ class UnsafeUse {
            kind == ExclusivityUnchecked ||
            kind == NonisolatedUnsafe ||
            kind == ReferenceToUnsafe ||
+           kind == ReferenceToUnsafeStorage ||
            kind == ReferenceToUnsafeThroughTypealias ||
            kind == CallToUnsafe);
 
@@ -179,6 +182,12 @@ class UnsafeUse {
                         decl, type, location);
   }
 
+  static UnsafeUse forReferenceToUnsafeStorage(const Decl *decl,
+                                               Type type,
+                                               SourceLoc location) {
+    return forReference(ReferenceToUnsafeStorage, decl, type, location);
+  }
+
   static UnsafeUse forReferenceToUnsafeThroughTypealias(const Decl *decl,
                                         Type type,
                                         SourceLoc location) {
@@ -215,6 +224,7 @@ class UnsafeUse {
     case ExclusivityUnchecked:
     case NonisolatedUnsafe:
     case ReferenceToUnsafe:
+    case ReferenceToUnsafeStorage:
     case ReferenceToUnsafeThroughTypealias:
     case CallToUnsafe:
       return SourceLoc(
@@ -246,6 +256,7 @@ class UnsafeUse {
     case ExclusivityUnchecked:
     case NonisolatedUnsafe:
     case ReferenceToUnsafe:
+    case ReferenceToUnsafeStorage:
     case ReferenceToUnsafeThroughTypealias:
     case CallToUnsafe:
       storage.entity.location = loc.getOpaquePointerValue();
@@ -266,6 +277,7 @@ class UnsafeUse {
     case ExclusivityUnchecked:
     case NonisolatedUnsafe:
     case ReferenceToUnsafe:
+    case ReferenceToUnsafeStorage:
     case ReferenceToUnsafeThroughTypealias:
     case CallToUnsafe:
       return storage.entity.decl;
@@ -299,6 +311,7 @@ class UnsafeUse {
     case NonisolatedUnsafe:
     case ReferenceToUnsafe:
     case ReferenceToUnsafeThroughTypealias:
+    case ReferenceToUnsafeStorage:
     case CallToUnsafe:
     case UnsafeConformance:
     case PreconcurrencyImport:
@@ -324,6 +337,7 @@ class UnsafeUse {
     case ExclusivityUnchecked:
     case NonisolatedUnsafe:
     case ReferenceToUnsafe:
+    case ReferenceToUnsafeStorage:
     case ReferenceToUnsafeThroughTypealias:
     case CallToUnsafe:
       return storage.entity.type;
@@ -348,6 +362,7 @@ class UnsafeUse {
     case ExclusivityUnchecked:
     case NonisolatedUnsafe:
     case ReferenceToUnsafe:
+    case ReferenceToUnsafeStorage:
     case ReferenceToUnsafeThroughTypealias:
     case CallToUnsafe:
     case PreconcurrencyImport:

include/swift/ClangImporter/ClangImporterRequests.h

@@ -31,6 +31,7 @@ namespace swift {
 class Decl;
 class DeclName;
 class EnumDecl;
+enum class ExplicitSafety;
 
 /// The input type for a clang direct lookup request.
 struct ClangDirectLookupDescriptor final {
@@ -564,6 +565,25 @@ class ClangTypeEscapability
 void simple_display(llvm::raw_ostream &out, EscapabilityLookupDescriptor desc);
 SourceLoc extractNearestSourceLoc(EscapabilityLookupDescriptor desc);
 
+/// Determine the safety of the given Clang declaration.
+class ClangDeclExplicitSafety
+    : public SimpleRequest<ClangDeclExplicitSafety,
+                           ExplicitSafety(SafeUseOfCxxDeclDescriptor),
+                           RequestFlags::Cached> {
+public:
+  using SimpleRequest::SimpleRequest;
+
+  // Source location
+  SourceLoc getNearestLoc() const { return SourceLoc(); };
+  bool isCached() const;
+
+private:
+  friend SimpleRequest;
+
+  // Evaluation.
+  ExplicitSafety evaluate(Evaluator &evaluator, SafeUseOfCxxDeclDescriptor desc) const;
+};
+
 #define SWIFT_TYPEID_ZONE ClangImporter
 #define SWIFT_TYPEID_HEADER "swift/ClangImporter/ClangImporterTypeIDZone.def"
 #include "swift/Basic/DefineTypeIDZone.h"

include/swift/ClangImporter/ClangImporterTypeIDZone.def

@@ -45,3 +45,6 @@ SWIFT_REQUEST(ClangImporter, CustomRefCountingOperation,
 SWIFT_REQUEST(ClangImporter, ClangTypeEscapability,
               CxxEscapability(EscapabilityLookupDescriptor), Cached,
               NoLocationInfo)
+SWIFT_REQUEST(ClangImporter, ClangDeclExplicitSafety,
+              ExplicitSafety(SafeUseOfCxxDeclDescriptor), Cached,
+              NoLocationInfo)

lib/AST/Decl.cpp

@@ -1150,6 +1150,14 @@ ExplicitSafety Decl::getExplicitSafety() const {
   if (auto safety = getExplicitSafetyFromAttrs(this))
     return *safety;
 
+  // If this declaration is from C, ask the Clang importer.
+  if (auto clangDecl = getClangDecl()) {
+    ASTContext &ctx = getASTContext();
+    return evaluateOrDefault(
+        ctx.evaluator, ClangDeclExplicitSafety({clangDecl, ctx}),
+        ExplicitSafety::Unspecified);
+  }
+
   // Inference: Check the enclosing context.
   if (auto enclosingDC = getDeclContext()) {
     // Is this an extension with @safe or @unsafe on it?

lib/AST/Expr.cpp

@@ -1934,6 +1934,12 @@ unsigned AbstractClosureExpr::getDiscriminator() const {
   evaluateOrDefault(
       ctx.evaluator, LocalDiscriminatorsRequest{getParent()}, 0);
 
+#if NDEBUG
+  static constexpr bool useFallbackDiscriminator = true;
+#else
+  static constexpr bool useFallbackDiscriminator = false;
+#endif
+
   // If we don't have a discriminator, and either
   //   1. We have ill-formed code and we're able to assign a discriminator, or
   //   2. We are in a macro expansion buffer
@@ -1943,7 +1949,8 @@ unsigned AbstractClosureExpr::getDiscriminator() const {
   if (getRawDiscriminator() == InvalidDiscriminator &&
       (ctx.Diags.hadAnyError() ||
        getParentSourceFile()->getFulfilledMacroRole() != std::nullopt ||
-       getParent()->isModuleScopeContext())) {
+       getParent()->isModuleScopeContext() ||
+       useFallbackDiscriminator)) {
     auto discriminator = ctx.getNextDiscriminator(getParent());
     ctx.setMaxAssignedDiscriminator(getParent(), discriminator + 1);
     const_cast<AbstractClosureExpr *>(this)->

lib/ClangImporter/ClangImporter.cpp

@@ -8215,6 +8215,106 @@ CustomRefCountingOperationResult CustomRefCountingOperation::evaluate(
   return {CustomRefCountingOperationResult::tooManyFound, nullptr, name};
 }
 
+/// Check whether the given Clang type involves an unsafe type.
+static bool hasUnsafeType(
+    Evaluator &evaluator, ASTContext &swiftContext, clang::QualType clangType
+) {
+  // Handle pointers.
+  auto pointeeType = clangType->getPointeeType();
+  if (!pointeeType.isNull()) {
+    // Function pointers are okay.
+    if (pointeeType->isFunctionType())
+      return false;
+
+    // Pointers to record types are okay if they come in as foreign reference
+    // types.
+    if (auto recordDecl = pointeeType->getAsRecordDecl()) {
+      if (hasImportAsRefAttr(recordDecl))
+        return false;
+    }
+
+    // All other pointers are considered unsafe.
+    return true;
+  }
+
+  // Handle records recursively.
+  if (auto recordDecl = clangType->getAsTagDecl()) {
+    auto safety = evaluateOrDefault(
+        evaluator, 
+        ClangDeclExplicitSafety({recordDecl, swiftContext}),
+        ExplicitSafety::Unspecified);
+    switch (safety) {
+      case ExplicitSafety::Unsafe:
+        return true;
+
+      case ExplicitSafety::Safe:
+      case ExplicitSafety::Unspecified:
+        return false;        
+    }
+  }
+
+  // Everything else is safe.
+  return false;
+}
+
+ExplicitSafety ClangDeclExplicitSafety::evaluate(
+    Evaluator &evaluator,
+    SafeUseOfCxxDeclDescriptor desc
+) const {
+  // FIXME: Somewhat duplicative with importAsUnsafe.
+  // FIXME: Also similar to hasPointerInSubobjects
+  // FIXME: should probably also subsume IsSafeUseOfCxxDecl
+
+  // Explicitly unsafe.
+  auto decl = desc.decl;
+  if (hasUnsafeAPIAttr(decl) || hasSwiftAttribute(decl, "unsafe"))
+    return ExplicitSafety::Unsafe;
+
+  // Explicitly safe.
+  if (hasSwiftAttribute(decl, "safe"))
+    return ExplicitSafety::Safe;
+
+  // Enums are always safe.
+  if (isa<clang::EnumDecl>(decl))
+    return ExplicitSafety::Safe;
+
+  // If it's not a record, leave it unspecified.
+  auto recordDecl = dyn_cast<clang::RecordDecl>(decl);
+  if (!recordDecl)
+    return ExplicitSafety::Unspecified;
+
+  // Escapable and non-escapable annotations imply that the declaration is
+  // safe.
+  if (hasNonEscapableAttr(recordDecl) || hasEscapableAttr(recordDecl))
+    return ExplicitSafety::Safe;
+
+  // If we don't have a definition, leave it unspecified.
+  recordDecl = recordDecl->getDefinition();
+  if (!recordDecl)
+    return ExplicitSafety::Unspecified;
+
+  // If this is a C++ class, check its bases.
+  if (auto cxxRecordDecl = dyn_cast<clang::CXXRecordDecl>(recordDecl)) {
+    for (auto base : cxxRecordDecl->bases()) {
+      if (hasUnsafeType(evaluator, desc.ctx, base.getType()))
+        return ExplicitSafety::Unsafe;
+    }
+  }
+
+  // Check the fields.
+  for (auto field : recordDecl->fields()) {
+    if (hasUnsafeType(evaluator, desc.ctx, field->getType()))
+      return ExplicitSafety::Unsafe;
+  }
+
+  // Okay, call it safe.
+  return ExplicitSafety::Safe;
+}
+
+bool ClangDeclExplicitSafety::isCached() const {
+  return isa<clang::RecordDecl>(std::get<0>(getStorage()).decl);
+}
+
 void ClangImporter::withSymbolicFeatureEnabled(
     llvm::function_ref<void(void)> callback) {
   llvm::SaveAndRestore<bool> oldImportSymbolicCXXDecls(

lib/Sema/TypeCheckAccess.cpp

@@ -1332,7 +1332,7 @@ class AccessControlChecker : public AccessControlCheckerBase,
       if (macroDecl) {
         diagnoseDeclAvailability(
             macroDecl, customAttr->getTypeRepr()->getSourceRange(), nullptr,
-            ExportContext::forDeclSignature(const_cast<Decl *>(D), nullptr),
+            ExportContext::forDeclSignature(const_cast<Decl *>(D)),
             std::nullopt);
       }
     }
@@ -2678,35 +2678,9 @@ void swift::checkAccessControl(Decl *D) {
   if (isa<AccessorDecl>(D))
     return;
 
-  // If we need to gather all unsafe uses from the declaration signature,
-  // do so now.
-  llvm::SmallVector<UnsafeUse, 2> unsafeUsesVec;
-  llvm::SmallVectorImpl<UnsafeUse> *unsafeUses = nullptr;
-  if (D->getASTContext().LangOpts.hasFeature(Feature::WarnUnsafe) &&
-      !D->isImplicit() &&
-      D->getExplicitSafety() == ExplicitSafety::Unspecified) {
-    unsafeUses = &unsafeUsesVec;
-  }
-
-  auto where = ExportContext::forDeclSignature(D, unsafeUses);
+  auto where = ExportContext::forDeclSignature(D);
   if (where.isImplicit())
     return;
 
   DeclAvailabilityChecker(where).visit(D);
-
-  // If there were any unsafe uses, this declaration needs "@unsafe".
-  if (!unsafeUsesVec.empty()) {
-    D->diagnose(diag::decl_signature_involves_unsafe, D);
-
-    ASTContext &ctx = D->getASTContext();
-    auto insertLoc = D->getAttributeInsertionLoc(/*forModifier=*/false);
-
-    ctx.Diags.diagnose(insertLoc, diag::decl_signature_mark_unsafe)
-      .fixItInsert(insertLoc, "@unsafe ");
-    ctx.Diags.diagnose(insertLoc, diag::decl_signature_mark_safe)
-      .fixItInsert(insertLoc, "@safe ");
-
-    std::for_each(unsafeUsesVec.begin(), unsafeUsesVec.end(),
-                  diagnoseUnsafeUse);
-  }
 }

lib/Sema/TypeCheckAttr.cpp

@@ -3313,7 +3313,7 @@ SynthesizeMainFunctionRequest::evaluate(Evaluator &evaluator,
     return nullptr;
   }
 
-  auto where = ExportContext::forDeclSignature(D, nullptr);
+  auto where = ExportContext::forDeclSignature(D);
   diagnoseDeclAvailability(mainFunction, attr->getRange(), nullptr, where,
                            std::nullopt);
 

lib/Sema/TypeCheckAvailability.cpp

@@ -246,8 +246,7 @@ static void computeExportContextBits(ASTContext &Ctx, Decl *D, bool *spi,
   }
 }
 
-ExportContext ExportContext::forDeclSignature(
-    Decl *D, llvm::SmallVectorImpl<UnsafeUse> *unsafeUses) {
+ExportContext ExportContext::forDeclSignature(Decl *D) {
   auto &Ctx = D->getASTContext();
 
   auto *DC = D->getInnermostDeclContext();
@@ -263,7 +262,7 @@ ExportContext ExportContext::forDeclSignature(
 
   bool exported = ::isExported(D);
 
-  return ExportContext(DC, availabilityContext, fragileKind, unsafeUses,
+  return ExportContext(DC, availabilityContext, fragileKind, nullptr,
                        spi, exported, implicit);
 }
 
@@ -286,8 +285,7 @@ ExportContext ExportContext::forFunctionBody(DeclContext *DC, SourceLoc loc) {
 ExportContext ExportContext::forConformance(DeclContext *DC,
                                             ProtocolDecl *proto) {
   assert(isa<ExtensionDecl>(DC) || isa<NominalTypeDecl>(DC));
-  auto where = forDeclSignature(DC->getInnermostDeclarationDeclContext(),
-                                nullptr);
+  auto where = forDeclSignature(DC->getInnermostDeclarationDeclContext());
 
   where.Exported &= proto->getFormalAccessScope(
       DC, /*usableFromInlineAsPublic*/true).isPublic();
@@ -2947,7 +2945,7 @@ void swift::diagnoseOverrideOfUnavailableDecl(ValueDecl *override,
 
   // FIXME: [availability] Take an unsatisfied constraint as input instead of
   // recomputing it.
-  ExportContext where = ExportContext::forDeclSignature(override, nullptr);
+  ExportContext where = ExportContext::forDeclSignature(override);
   auto constraint =
       getUnsatisfiedAvailabilityConstraint(base, where.getAvailability());
   if (!constraint)

lib/Sema/TypeCheckAvailability.h

@@ -127,8 +127,7 @@ class ExportContext {
   ///
   /// If the declaration is exported, the resulting context is restricted to
   /// referencing exported types only. Otherwise it can reference anything.
-  static ExportContext forDeclSignature(
-      Decl *D, llvm::SmallVectorImpl<UnsafeUse> *unsafeUses);
+  static ExportContext forDeclSignature(Decl *D);
 
   /// Create an instance describing the declarations that can be referenced
   /// from the given function's body.