Merge pull request #61857 from gottesmm/pr-557d1bdfecfe6c7c67115122a6d499ff6c3e51f5

[move-only] Ban move only types being stored properties of copyable types or conforming to protocols

Author: gottesmm

include/swift/AST/DiagnosticsSema.def

@@ -6705,5 +6705,22 @@ ERROR(macro_undefined,PointsToFirstBadToken,
       "macro %0 is undefined; use `-load-plugin-library` to specify dynamic "
       "libraries that contain this macro", (Identifier))
 
+//------------------------------------------------------------------------------
+// MARK: Move Only Errors
+//------------------------------------------------------------------------------
+
+ERROR(moveonly_copyable_type_that_contains_moveonly_type, none,
+      "%0 %1 cannot contain a move-only type without also being move-only",
+      (DescriptiveDeclKind, DeclName))
+NOTE(moveonly_copyable_type_that_contains_moveonly_type_location, none,
+     "contained move-only %0 '%1.%2'",
+     (DescriptiveDeclKind, StringRef, StringRef))
+ERROR(moveonly_cannot_conform_to_protocol, none,
+     "move-only %0 %1 cannot conform yet to any protocols",
+     (DescriptiveDeclKind, DeclName))
+ERROR(moveonly_cannot_conform_to_protocol_with_name, none,
+     "move-only %0 %1 cannot conform to protocol %2",
+     (DescriptiveDeclKind, DeclName, DeclName))
 #define UNDEFINE_DIAGNOSTIC_MACROS
 #include "DefineDiagnosticMacros.h"
+

include/swift/AST/Types.h

@@ -577,6 +577,10 @@ class alignas(1 << TypeAlignInBits) TypeBase
 
   bool isPlaceholder();
 
+  /// Returns true if this is a move only type. Returns false if this is a
+  /// non-move only type or a move only wrapped type.
+  bool isPureMoveOnly() const;
+
   /// Does the type have outer parenthesis?
   bool hasParenSugar() const { return getKind() == TypeKind::Paren; }
 

lib/AST/Type.cpp

@@ -165,6 +165,12 @@ bool TypeBase::isAny() {
   return constraint->isEqual(getASTContext().TheAnyType);
 }
 
+bool TypeBase::isPureMoveOnly() const {
+  if (auto *nom = getCanonicalType()->getNominalOrBoundGenericNominal())
+    return nom->isMoveOnly();
+  return false;
+}
+
 bool TypeBase::isPlaceholder() {
   return is<PlaceholderType>();
 }

lib/Sema/MiscDiagnostics.cpp

@@ -5889,3 +5889,107 @@ bool swift::diagnoseUnhandledThrowsInAsyncContext(DeclContext *dc,
 
   return false;
 }
+
+//===----------------------------------------------------------------------===//
+//              Copyable Type Containing Move Only Type Visitor
+//===----------------------------------------------------------------------===//
+
+void swift::diagnoseCopyableTypeContainingMoveOnlyType(
+    NominalTypeDecl *copyableNominalType) {
+  // If we don't have move only enabled, bail early.
+  if (!copyableNominalType->getASTContext().LangOpts.Features.contains(
+          Feature::MoveOnly))
+    return;
+
+  // If we already have a move only type, just bail, we have no further work to
+  // do.
+  if (copyableNominalType->isMoveOnly())
+    return;
+
+  LLVM_DEBUG(llvm::dbgs() << "DiagnoseCopyableType for: "
+                          << copyableNominalType->getName() << '\n');
+
+  auto &DE = copyableNominalType->getASTContext().Diags;
+  auto emitError = [&copyableNominalType,
+                    &DE](PointerUnion<EnumElementDecl *, VarDecl *>
+                             topFieldToError,
+                         DeclBaseName parentName, DescriptiveDeclKind fieldKind,
+                         DeclBaseName fieldName) {
+    assert(!topFieldToError.isNull());
+    if (auto *eltDecl = topFieldToError.dyn_cast<EnumElementDecl *>()) {
+      DE.diagnoseWithNotes(
+          copyableNominalType->diagnose(
+              diag::moveonly_copyable_type_that_contains_moveonly_type,
+              copyableNominalType->getDescriptiveKind(),
+              copyableNominalType->getBaseName()),
+          [&]() {
+            eltDecl->diagnose(
+                diag::
+                    moveonly_copyable_type_that_contains_moveonly_type_location,
+                fieldKind, parentName.userFacingName(),
+                fieldName.userFacingName());
+          });
+      return;
+    }
+
+    auto *varDecl = topFieldToError.get<VarDecl *>();
+    DE.diagnoseWithNotes(
+        copyableNominalType->diagnose(
+            diag::moveonly_copyable_type_that_contains_moveonly_type,
+            copyableNominalType->getDescriptiveKind(),
+            copyableNominalType->getBaseName()),
+        [&]() {
+          varDecl->diagnose(
+              diag::moveonly_copyable_type_that_contains_moveonly_type_location,
+              fieldKind, parentName.userFacingName(),
+              fieldName.userFacingName());
+        });
+  };
+
+  // If we have a struct decl...
+  if (auto *structDecl = dyn_cast<StructDecl>(copyableNominalType)) {
+    // Visit each of the stored property var decls of the struct decl...
+    for (auto *fieldDecl : structDecl->getStoredProperties()) {
+      LLVM_DEBUG(llvm::dbgs()
+                 << "Visiting struct field: " << fieldDecl->getName() << '\n');
+      if (!fieldDecl->getInterfaceType()->isPureMoveOnly())
+        continue;
+      emitError(fieldDecl, structDecl->getBaseName(),
+                fieldDecl->getDescriptiveKind(), fieldDecl->getBaseName());
+    }
+    // We completed our checking, just return.
+    return;
+  }
+
+  if (auto *enumDecl = dyn_cast<EnumDecl>(copyableNominalType)) {
+    // If we have an enum but we don't have any elements, just continue, we
+    // have nothing to check.
+    if (enumDecl->getAllElements().empty())
+      return;
+
+    // Otherwise for each element...
+    for (auto *enumEltDecl : enumDecl->getAllElements()) {
+      // If the element doesn't have any associated values, we have nothing to
+      // check, so continue.
+      if (!enumEltDecl->hasAssociatedValues())
+        continue;
+
+      LLVM_DEBUG(llvm::dbgs() << "Visiting enum elt decl: "
+                 << enumEltDecl->getName() << '\n');
+
+      // Otherwise, we have a case and need to check the types of the
+      // parameters of the case payload.
+      for (auto payloadParam : *enumEltDecl->getParameterList()) {
+        LLVM_DEBUG(llvm::dbgs() << "Visiting payload param: "
+                   << payloadParam->getName() << '\n');
+        if (payloadParam->getInterfaceType()->isPureMoveOnly()) {
+            emitError(enumEltDecl, enumDecl->getBaseName(),
+                      enumEltDecl->getDescriptiveKind(),
+                      enumEltDecl->getBaseName());
+        }
+      }
+    }    
+    // We have finished processing this enum... so return.
+    return;
+  }
+}

lib/Sema/MiscDiagnostics.h

@@ -136,6 +136,9 @@ class BaseDiagnosticWalker : public ASTWalker {
   static bool shouldWalkIntoDeclInClosureContext(Decl *D);
 };
 
+void diagnoseCopyableTypeContainingMoveOnlyType(
+    NominalTypeDecl *copyableNominalType);
+
 } // namespace swift
 
 #endif // SWIFT_SEMA_MISC_DIAGNOSTICS_H

lib/Sema/TypeCheckConcurrency.cpp

@@ -4470,6 +4470,13 @@ ProtocolConformance *GetImplicitSendableRequest::evaluate(
   if (isa<ProtocolDecl>(nominal))
     return nullptr;
 
+  // Move only nominal types are currently never sendable since we have not yet
+  // finished the generics model for them.
+  //
+  // TODO: Remove this once this is complete!
+  if (nominal->isMoveOnly())
+    return nullptr;
+
   // Actor types are always Sendable; they don't get it via this path.
   auto classDecl = dyn_cast<ClassDecl>(nominal);
   if (classDecl && classDecl->isActor())

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -33,6 +33,7 @@
 #include "swift/AST/AccessScope.h"
 #include "swift/AST/Decl.h"
 #include "swift/AST/DeclContext.h"
+#include "swift/AST/DiagnosticsSema.h"
 #include "swift/AST/ExistentialLayout.h"
 #include "swift/AST/Expr.h"
 #include "swift/AST/ForeignErrorConvention.h"
@@ -2491,6 +2492,9 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
       }
     }
 
+    diagnoseCopyableTypeContainingMoveOnlyType(ED);
+    diagnoseMoveOnlyNominalDeclDoesntConformToProtocols(ED);
+
     checkExplicitAvailability(ED);
 
     TypeChecker::checkDeclCircularity(ED);
@@ -2513,8 +2517,9 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
 
     TypeChecker::checkDeclAttributes(SD);
 
-    for (Decl *Member : SD->getMembers())
+    for (Decl *Member : SD->getMembers()) {
       visit(Member);
+    }
 
     TypeChecker::checkPatternBindingCaptures(SD);
 
@@ -2528,6 +2533,12 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
     TypeChecker::checkDeclCircularity(SD);
 
     TypeChecker::checkConformancesInContext(SD);
+
+    // If this struct is not move only, check that all vardecls of nominal type
+    // are not move only.
+    diagnoseCopyableTypeContainingMoveOnlyType(SD);
+
+    diagnoseMoveOnlyNominalDeclDoesntConformToProtocols(SD);
   }
 
   /// Check whether the given properties can be @NSManaged in this class.
@@ -2629,6 +2640,17 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
     }
   }
 
+  void diagnoseMoveOnlyNominalDeclDoesntConformToProtocols(
+      NominalTypeDecl *nomDecl) {
+    if (!nomDecl->isMoveOnly())
+      return;
+
+    for (auto *prot : nomDecl->getAllProtocols()) {
+      nomDecl->diagnose(diag::moveonly_cannot_conform_to_protocol_with_name,
+                        nomDecl->getDescriptiveKind(),
+                        nomDecl->getBaseName(), prot->getBaseName());
+    }
+  }
 
   void visitClassDecl(ClassDecl *CD) {
     checkUnsupportedNestedType(CD);
@@ -2792,6 +2814,8 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
     TypeChecker::checkConformancesInContext(CD);
 
     maybeDiagnoseClassWithoutInitializers(CD);
+
+    diagnoseMoveOnlyNominalDeclDoesntConformToProtocols(CD);
   }
 
   void visitProtocolDecl(ProtocolDecl *PD) {
@@ -3246,6 +3270,12 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
 
     if (nominal->isDistributedActor())
       TypeChecker::checkDistributedActor(SF, nominal);
+
+    // If we have a move only type and allow it to extend any protocol, error.
+    if (nominal->isMoveOnly() && ED->getInherited().size()) {
+      ED->diagnose(diag::moveonly_cannot_conform_to_protocol,
+                   nominal->getDescriptiveKind(), nominal->getBaseName());
+    }
   }
 
   void visitTopLevelCodeDecl(TopLevelCodeDecl *TLCD) {

test/Interpreter/moveonly_bufferview.swift

@@ -10,16 +10,15 @@
 public struct BufferView<T> {
     var ptr: UnsafeBufferPointer<T>
 
-    deinit {}
-}
-
-extension BufferView : Sequence {
-    public typealias Iterator = UnsafeBufferPointer<T>.Iterator
-    public typealias Element = UnsafeBufferPointer<T>.Element
+    var count: Int {
+        return ptr.count
+    }
 
-    public func makeIterator() -> Self.Iterator {
-        return ptr.makeIterator()
+    subscript(_ x: Int) -> T {
+        return ptr[x]
     }
+
+    deinit {}
 }
 
 extension Array {
@@ -36,8 +35,8 @@ func testBufferView(_ x: __owned [Int]) {
     // CHECK: 2
     // CHECK: 3
     y.withBufferView {
-        for x in $0 {
-            print(x)
+        for i in 0..<$0.count {
+            print($0[i])
         }
     }
 }
@@ -52,8 +51,8 @@ func testConditionalBufferView(_ x: __owned [Int]) {
         // CHECK: 5
         // CHECK: 6
         if getBool() {
-            for z in y {
-                print(z)
+            for i in 0..<y.count {
+                print(y[i])
             }
         }
     }