Merge pull request #71670 from nate-chandler/partial-consumption/20240215/1/imported-types

[MoveChecker] Ban exported partial consumption.

Author: nate-chandler

include/swift/AST/DiagnosticsSIL.def

@@ -837,12 +837,24 @@ ERROR(sil_movechecking_notconsumable_but_assignable_was_consumed, none,
 ERROR(sil_movechecking_cannot_destructure_has_deinit, none,
       "cannot partially consume '%0' when it has a deinitializer",
       (StringRef))
+ERROR(sil_movechecking_cannot_destructure_imported_nonfrozen, none,
+      "cannot partially consume '%0' of non-frozen type %1 imported from %2",
+      (StringRef, Type, const ModuleDecl*))
+ERROR(sil_movechecking_cannot_destructure_exported_usableFromInline_alwaysEmitIntoClient, none,
+      "cannot partially consume '%0' of non-frozen usableFromInline type %1 within a function annotated @_alwaysEmitIntoClient",
+      (StringRef, Type))
 ERROR(sil_movechecking_cannot_destructure, none,
       "cannot partially consume '%0'",
       (StringRef))
 ERROR(sil_movechecking_cannot_partially_reinit_has_deinit, none,
       "cannot partially reinitialize '%0' when it has a deinitializer; only full reinitialization is allowed",
       (StringRef))
+ERROR(sil_movechecking_cannot_partially_reinit_nonfrozen, none,
+      "cannot partially reinitialize '%0' of non-frozen type %1 imported from %2; only full reinitialization is allowed",
+      (StringRef, Type, const ModuleDecl*))
+ERROR(sil_movechecking_cannot_partially_reinit_exported_usableFromInline_alwaysEmitIntoClient, none,
+      "cannot partially reinitialize '%0' of non-frozen usableFromInline type %1 within a function annotated @_alwaysEmitIntoClient",
+      (StringRef, Type))
 ERROR(sil_movechecking_cannot_partially_reinit, none,
       "cannot partially reinitialize '%0' after it has been consumed; only full reinitialization is allowed",
       (StringRef))

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

@@ -1682,6 +1682,41 @@ struct CopiedLoadBorrowEliminationVisitor
 //                   MARK: Partial Consume/Reinit Checking
 //===----------------------------------------------------------------------===//
 
+static bool hasExplicitFixedLayoutAnnotation(NominalTypeDecl *nominal) {
+  return nominal->getAttrs().hasAttribute<FixedLayoutAttr>() ||
+         nominal->getAttrs().hasAttribute<FrozenAttr>();
+}
+
+static std::optional<PartialMutationError>
+shouldEmitPartialMutationErrorForType(SILType ty, NominalTypeDecl *nominal,
+                                      SILFunction *fn) {
+  // A non-frozen type can't be partially mutated within code built in its
+  // defining module if that code will be emitted into a client.
+  if (fn->getLinkage() == SILLinkage::PublicNonABI &&
+      nominal->isUsableFromInline() &&
+      !hasExplicitFixedLayoutAnnotation(nominal)) {
+    return {PartialMutationError::nonfrozenUsableFromInlineType(ty, *nominal)};
+  }
+
+  // Otherwise, a type can be mutated partially in its defining module.
+  if (nominal->getModuleContext() == fn->getModule().getSwiftModule())
+    return std::nullopt;
+
+  // It's defined in another module and used here; it has to be visible.
+  assert(nominal
+             ->getFormalAccessScope(
+                 /*useDC=*/fn->getDeclContext(),
+                 /*treatUsableFromInlineAsPublic=*/true)
+             .isPublicOrPackage());
+
+  // Partial mutation is supported only for frozen/fixed-layout types from
+  // other modules.
+  if (hasExplicitFixedLayoutAnnotation(nominal))
+    return std::nullopt;
+
+  return {PartialMutationError::nonfrozenImportedType(ty, *nominal)};
+}
+
 /// Whether an error should be emitted in response to a partial consumption.
 static llvm::Optional<PartialMutationError>
 shouldEmitPartialMutationError(UseState &useState, PartialMutation::Kind kind,
@@ -1719,13 +1754,17 @@ shouldEmitPartialMutationError(UseState &useState, PartialMutation::Kind kind,
 
   LLVM_DEBUG(llvm::dbgs() << "    MoveOnlyPartialConsumption enabled!\n");
 
-  // Otherwise, walk the type looking for the deinit.
+  // Otherwise, walk the type looking for the deinit and visibility.
   while (iterType != targetType) {
     // If we have a nominal type as our parent type, see if it has a
     // deinit. We know that it must be non-copyable since copyable types
     // cannot contain non-copyable types and that our parent root type must be
     // an enum, tuple, or struct.
     if (auto *nom = iterType.getNominalOrBoundGenericNominal()) {
+      if (auto error = shouldEmitPartialMutationErrorForType(
+              iterType, nom, user->getFunction())) {
+        return error;
+      }
       if (nom->getValueTypeDestructor()) {
         // If we find one, emit an error since we are going to have to extract
         // through the deinit. Emit a nice error saying what it is. Since we

lib/SILOptimizer/Mandatory/MoveOnlyDiagnostics.cpp

@@ -824,24 +824,63 @@ void DiagnosticEmitter::emitCannotPartiallyMutateError(
         astContext.LangOpts.hasFeature(Feature::MoveOnlyPartialConsumption) ||
         astContext.LangOpts.hasFeature(
             Feature::MoveOnlyPartialReinitialization));
-    switch (kind) {
-    case PartialMutation::Kind::Consume:
-      diagnose(astContext, user,
-               diag::sil_movechecking_cannot_destructure_has_deinit, varName);
-      break;
-    case PartialMutation::Kind::Reinit:
-      diagnose(astContext, user,
-               diag::sil_movechecking_cannot_partially_reinit_has_deinit,
-               varName);
-      break;
-    }
+    auto diagnostic = [&]() {
+      switch (kind) {
+      case PartialMutation::Kind::Consume:
+        return diag::sil_movechecking_cannot_destructure_has_deinit;
+      case PartialMutation::Kind::Reinit:
+        return diag::sil_movechecking_cannot_partially_reinit_has_deinit;
+      }
+    }();
+    diagnose(astContext, user, diagnostic, varName);
     registerDiagnosticEmitted(address);
-    auto deinitLoc = error.getNominal().getValueTypeDestructor()->getLoc(
-        /*SerializedOK=*/false);
+    auto deinitLoc =
+        error.getDeinitingNominal().getValueTypeDestructor()->getLoc(
+            /*SerializedOK=*/false);
     if (!deinitLoc)
       return;
     astContext.Diags.diagnose(deinitLoc, diag::sil_movechecking_deinit_here);
     return;
   }
+  case PartialMutationError::Kind::NonfrozenImportedType: {
+    assert(
+        astContext.LangOpts.hasFeature(Feature::MoveOnlyPartialConsumption) ||
+        astContext.LangOpts.hasFeature(
+            Feature::MoveOnlyPartialReinitialization));
+    auto &nominal = error.getNonfrozenImportedNominal();
+    auto diagnostic = [&]() {
+      switch (kind) {
+      case PartialMutation::Kind::Consume:
+        return diag::sil_movechecking_cannot_destructure_imported_nonfrozen;
+      case PartialMutation::Kind::Reinit:
+        return diag::sil_movechecking_cannot_partially_reinit_nonfrozen;
+      }
+    }();
+    diagnose(astContext, user, diagnostic, varName,
+             nominal.getDeclaredInterfaceType(), nominal.getModuleContext());
+    registerDiagnosticEmitted(address);
+    return;
+  }
+  case PartialMutationError::Kind::NonfrozenUsableFromInlineType: {
+    assert(
+        astContext.LangOpts.hasFeature(Feature::MoveOnlyPartialConsumption) ||
+        astContext.LangOpts.hasFeature(
+            Feature::MoveOnlyPartialReinitialization));
+    auto &nominal = error.getNonfrozenUsableFromInlineNominal();
+    auto diagnostic = [&]() {
+      switch (kind) {
+      case PartialMutation::Kind::Consume:
+        return diag::
+            sil_movechecking_cannot_destructure_exported_usableFromInline_alwaysEmitIntoClient;
+      case PartialMutation::Kind::Reinit:
+        return diag::
+            sil_movechecking_cannot_partially_reinit_exported_usableFromInline_alwaysEmitIntoClient;
+      }
+    }();
+    diagnose(astContext, user, diagnostic, varName,
+             nominal.getDeclaredInterfaceType());
+    registerDiagnosticEmitted(address);
+    return;
+  }
   }
 }

lib/SILOptimizer/Mandatory/MoveOnlyTypeUtils.h

@@ -141,9 +141,14 @@ inline Feature partialMutationFeature(PartialMutation::Kind kind) {
 ///
 /// Emulates the following enum with associated value:
 ///
-/// enum class PartialMutationError {
-/// case featureDisabled(SILType)
-/// case hasDeinit(SILType, NominalTypeDecl)
+/// struct PartialMutationError {
+///   let type: SILType
+///   enum Kind {
+///     case featureDisabled
+///     case hasDeinit(NominalTypeDecl)
+///     case nonfrozenImportedType(NominalTypeDecl)
+///     case nonfrozenUsableFromInlineType(NominalTypeDecl)
+///   }
 /// }
 class PartialMutationError {
   struct FeatureDisabled {
@@ -152,11 +157,18 @@ class PartialMutationError {
   struct HasDeinit {
     NominalTypeDecl &nominal;
   };
-  TaggedUnion<FeatureDisabled, HasDeinit> kind;
+  struct NonfrozenImportedType {
+    NominalTypeDecl &nominal;
+  };
+  struct NonfrozenUsableFromInlineType {
+    NominalTypeDecl &nominal;
+  };
+  using Payload = TaggedUnion<FeatureDisabled, HasDeinit, NonfrozenImportedType,
+                              NonfrozenUsableFromInlineType>;
+  Payload payload;
 
-  PartialMutationError(SILType type, FeatureDisabled fd)
-      : kind({fd}), type(type) {}
-  PartialMutationError(SILType type, HasDeinit r) : kind({r}), type(type) {}
+  PartialMutationError(SILType type, Payload payload)
+      : payload(payload), type(type) {}
 
 public:
   /// The type within the aggregate responsible for the error.
@@ -180,12 +192,36 @@ class PartialMutationError {
     /// range and the full value.  It is the \p nominal field of
     /// PartialMutationError.
     HasDeinit,
+    /// A partially consumed/reinitialized aggregate is defined in a different
+    /// module and isn't frozen.
+    ///
+    /// In the case that the other module was built with library evolution, it
+    /// isn't legal (or valid SIL) to directly modify the aggregate's fields.
+    /// To have consistent semantics regardless of whether that other module was
+    /// built with library evolution, prevent the aggregate from being partially
+    /// mutated.
+    NonfrozenImportedType,
+    /// A partially consumed/reinitialized aggregate is defined in the same
+    /// module, but the function where it is used is @_alwaysEmitIntoClient.
+    ///
+    /// In this case, if the module were built with library evolution, it
+    /// wouldn't be legal (or valid SIL) to directly modify the aggregate's
+    /// fields within the function when it was included into some client
+    /// library.  To have consistent semantics regardless of having been built
+    /// with library evolution, prevent the aggregate from being partially
+    /// mutated.
+    NonfrozenUsableFromInlineType,
   };
 
   operator Kind() {
-    if (kind.isa<FeatureDisabled>())
+    if (payload.isa<FeatureDisabled>())
       return Kind::FeatureDisabled;
-    return Kind::HasDeinit;
+    else if (payload.isa<HasDeinit>())
+      return Kind::HasDeinit;
+    else if (payload.isa<NonfrozenImportedType>())
+      return Kind::NonfrozenImportedType;
+    assert(payload.isa<NonfrozenUsableFromInlineType>());
+    return Kind::NonfrozenUsableFromInlineType;
   }
 
   static PartialMutationError featureDisabled(SILType type,
@@ -198,8 +234,28 @@ class PartialMutationError {
     return PartialMutationError(type, HasDeinit{nominal});
   }
 
-  NominalTypeDecl &getNominal() { return kind.get<HasDeinit>().nominal; };
-  PartialMutation::Kind getKind() { return kind.get<FeatureDisabled>().kind; };
+  static PartialMutationError nonfrozenImportedType(SILType type,
+                                                    NominalTypeDecl &nominal) {
+    return PartialMutationError(type, NonfrozenImportedType{nominal});
+  }
+
+  static PartialMutationError
+  nonfrozenUsableFromInlineType(SILType type, NominalTypeDecl &nominal) {
+    return PartialMutationError(type, NonfrozenUsableFromInlineType{nominal});
+  }
+
+  PartialMutation::Kind getKind() {
+    return payload.get<FeatureDisabled>().kind;
+  }
+  NominalTypeDecl &getDeinitingNominal() {
+    return payload.get<HasDeinit>().nominal;
+  }
+  NominalTypeDecl &getNonfrozenImportedNominal() {
+    return payload.get<NonfrozenImportedType>().nominal;
+  }
+  NominalTypeDecl &getNonfrozenUsableFromInlineNominal() {
+    return payload.get<NonfrozenUsableFromInlineType>().nominal;
+  }
 };
 } // namespace siloptimizer
 } // namespace swift

test/SILOptimizer/moveonly_addresschecker_diagnostics_partial_consume_ufi.swift

@@ -0,0 +1,57 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend                                      \
+// RUN:     %s                                                      \
+// RUN:     -emit-sil -verify -verify-additional-prefix fragile-    \
+// RUN:     -sil-verify-all                                         \
+// RUN:     -enable-experimental-feature MoveOnlyPartialConsumption \
+// RUN:     -module-name Library
+// RUN: %target-swift-frontend                                      \
+// RUN:     %s                                                      \
+// RUN:     -emit-sil -verify -verify-additional-prefix resilient-  \
+// RUN:     -sil-verify-all                                         \
+// RUN:     -enable-library-evolution                               \
+// RUN:     -enable-experimental-feature MoveOnlyPartialConsumption \
+// RUN:     -module-name Library
+
+public func take(_ u: consuming Ur) {}
+
+public struct Ur : ~Copyable {
+  @usableFromInline init() {}
+  deinit {}
+}
+
+@usableFromInline
+internal struct Agg_Internal_UFI : ~Copyable {
+  @usableFromInline
+  init(u1: consuming Ur, u2: consuming Ur) {
+    self.u1 = u1
+    self.u2 = u2
+  }
+
+  @usableFromInline
+  internal var u1: Ur
+  @usableFromInline
+  internal var u2: Ur
+}
+
+@_alwaysEmitIntoClient
+func piecewise_Agg_Internal_UFI(_ a: consuming Agg_Internal_UFI) {
+  take(a.u1) // expected-fragile-error{{cannot partially consume 'a' of non-frozen usableFromInline type 'Agg_Internal_UFI' within a function annotated @_alwaysEmitIntoClient}}
+             // expected-resilient-error@-1{{field 'a.u1' was consumed but not reinitialized; the field must be reinitialized during the access}}
+             // expected-resilient-note@-2{{consumed here}}
+  take(a.u2) // expected-fragile-error{{cannot partially consume 'a' of non-frozen usableFromInline type 'Agg_Internal_UFI' within a function annotated @_alwaysEmitIntoClient}}
+             // expected-resilient-error@-1{{field 'a.u2' was consumed but not reinitialized; the field must be reinitialized during the access}}
+             // expected-resilient-note@-2{{consumed here}}
+}
+
+@_alwaysEmitIntoClient
+func get_Agg_Internal_UFI() -> Agg_Internal_UFI {
+  return Agg_Internal_UFI(u1: Ur(), u2: Ur())
+}
+
+@_alwaysEmitIntoClient
+public func call_piecewise_Agg_Internal_UFI() {
+  let a = get_Agg_Internal_UFI()
+  piecewise_Agg_Internal_UFI(a)
+}
+