Initial ban of move-only types from being used generically

kavon · kavon · commit ab130883a3b9 · 2023-02-01T23:38:28.000-08:00
Since values of generic type are currently assumed to always
support copying, we need to prevent move-only types from
being substituted for generic type parameters.

This approach leans on a `_Copyable` marker protocol to which
all generic type parameters implicitly must conform.

A few other changes in this initial implementation:

- Now every concrete type that can conform to Copyable will do so. This fixes issues with conforming to a protocol that requires Copyable.
- Narrowly ban writing a concrete type `[T]` when `T` is move-only.
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -6653,6 +6653,7 @@ ERROR(noimplicitcopy_attr_valid_only_on_local_let_params,
       none, "'@_noImplicitCopy' attribute can only be applied to local lets and params", ())
 ERROR(noimplicitcopy_attr_invalid_in_generic_context,
       none, "'@_noImplicitCopy' attribute cannot be applied to entities in generic contexts", ())
+ERROR(moveonly_generics, none, "move-only type %0 cannot be used with generics yet", (Type))
 ERROR(noimplicitcopy_attr_not_allowed_on_moveonlytype,none,
       "'@_noImplicitCopy' has no effect when applied to a move only type", ())
 
@@ -6972,4 +6973,3 @@ NOTE(opt_out_from_missing_reflection_metadata_attr,none,
 
 #define UNDEFINE_DIAGNOSTIC_MACROS
 #include "DefineDiagnosticMacros.h"
-
diff --git a/include/swift/AST/KnownProtocols.def b/include/swift/AST/KnownProtocols.def
@@ -117,6 +117,8 @@ PROTOCOL(AsyncIteratorProtocol)
 
 PROTOCOL(FloatingPoint)
 
+PROTOCOL_(Copyable)
+
 EXPRESSIBLE_BY_LITERAL_PROTOCOL(ExpressibleByArrayLiteral, "Array", false)
 EXPRESSIBLE_BY_LITERAL_PROTOCOL(ExpressibleByBooleanLiteral, "BooleanLiteralType", true)
 EXPRESSIBLE_BY_LITERAL_PROTOCOL(ExpressibleByDictionaryLiteral, "Dictionary", false)
diff --git a/include/swift/Sema/CSFix.h b/include/swift/Sema/CSFix.h
@@ -422,6 +422,9 @@ enum class FixKind : uint8_t {
   /// Allow function type actor mismatch e.g. `@MainActor () -> Void`
   /// vs.`@OtherActor () -> Void`
   AllowGlobalActorMismatch,
+
+  /// Produce an error about a type that must be Copyable
+  MustBeCopyable,
 };
 
 class ConstraintFix {
@@ -2027,6 +2030,29 @@ class NotCompileTimeConst final : public ContextualMismatch {
   }
 };
 
+class MustBeCopyable final : public ConstraintFix {
+  Type noncopyableTy;
+
+  MustBeCopyable(ConstraintSystem &cs, Type noncopyableTy, ConstraintLocator *locator);
+
+public:
+  std::string getName() const override { return "remove move-only from type"; }
+
+  bool diagnose(const Solution &solution, bool asNote = false) const override;
+
+  bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override {
+    return diagnose(*commonFixes.front().first);
+  }
+
+  static MustBeCopyable *create(ConstraintSystem &cs,
+                             Type noncopyableTy,
+                             ConstraintLocator *locator);
+
+  static bool classof(ConstraintFix *fix) {
+    return fix->getKind() == FixKind::MustBeCopyable;
+  }
+};
+
 class CollectionElementContextualMismatch final
     : public ContextualMismatch,
       private llvm::TrailingObjects<CollectionElementContextualMismatch,
diff --git a/lib/AST/Module.cpp b/lib/AST/Module.cpp
@@ -1527,6 +1527,11 @@ LookupConformanceInModuleRequest::evaluate(
   // constraint and the superclass conforms to the protocol.
   if (auto archetype = type->getAs<ArchetypeType>()) {
 
+    // All archetypes conform to Copyable since they represent a generic.
+    // FIXME: can't tell whether this is a hack.
+    if (protocol->isSpecificProtocol(KnownProtocolKind::Copyable))
+      return ProtocolConformanceRef(protocol);
+
     // The generic signature builder drops conformance requirements that are made
     // redundant by a superclass requirement, so check for a concrete
     // conformance first, since an abstract conformance might not be
diff --git a/lib/AST/ProtocolConformance.cpp b/lib/AST/ProtocolConformance.cpp
@@ -1097,6 +1097,11 @@ void NominalTypeDecl::prepareConformanceTable() const {
   if (mutableThis->getAttrs().hasAttribute<GlobalActorAttr>()) {
     addSynthesized(ctx.getProtocol(KnownProtocolKind::GlobalActor));
   }
+
+  // All nominal types that are not move-only conform to Copyable.
+  if (!mutableThis->isMoveOnly()) {
+    addSynthesized(ctx.getProtocol(KnownProtocolKind::Copyable));
+  }
 }
 
 bool NominalTypeDecl::lookupConformance(
@@ -1224,24 +1229,24 @@ static SmallVector<ProtocolConformance *, 2> findSynthesizedConformances(
   // Try to find specific conformances
   SmallVector<ProtocolConformance *, 2> result;
 
-  // Sendable may be synthesized for concrete types
-  if (!isa<ProtocolDecl>(nominal)) {
-    if (auto sendable =
-            findSynthesizedConformance(dc, KnownProtocolKind::Sendable)) {
-      result.push_back(sendable);
+  auto trySynthesize = [&](KnownProtocolKind knownProto) {
+    if (auto conformance =
+            findSynthesizedConformance(dc, knownProto)) {
+      result.push_back(conformance);
     }
+  };
+
+  // Concrete types may synthesize some conformances
+  if (!isa<ProtocolDecl>(nominal)) {
+    trySynthesize(KnownProtocolKind::Sendable);
+    trySynthesize(KnownProtocolKind::Copyable); // FIXME: we do it when the conformance table is generaetd
+                                                // so this is actually useless now.
   }
 
   /// Distributed actors can synthesize Encodable/Decodable, so look for those
   if (nominal->isDistributedActor()) {
-    if (auto conformance =
-            findSynthesizedConformance(dc, KnownProtocolKind::Encodable)) {
-      result.push_back(conformance);
-    }
-    if (auto conformance =
-            findSynthesizedConformance(dc, KnownProtocolKind::Decodable)) {
-      result.push_back(conformance);
-    }
+    trySynthesize(KnownProtocolKind::Encodable);
+    trySynthesize(KnownProtocolKind::Decodable);
   }
 
   return result;
diff --git a/lib/IRGen/GenMeta.cpp b/lib/IRGen/GenMeta.cpp
@@ -5885,6 +5885,7 @@ SpecialProtocol irgen::getSpecialProtocolID(ProtocolDecl *P) {
   case KnownProtocolKind::UnsafeSendable:
   case KnownProtocolKind::RangeReplaceableCollection:
   case KnownProtocolKind::GlobalActor:
+  case KnownProtocolKind::Copyable:
     return SpecialProtocol::None;
   }
 
diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp
@@ -5822,6 +5822,11 @@ bool NotCompileTimeConstFailure::diagnoseAsError() {
   return true;
 }
 
+bool NotCopyableFailure::diagnoseAsError() {
+  emitDiagnostic(diag::moveonly_generics, noncopyableTy);
+  return true;
+}
+
 bool CollectionElementContextualFailure::diagnoseAsError() {
   auto anchor = getRawAnchor();
   auto *locator = getLocator();
diff --git a/lib/Sema/CSDiagnostics.h b/lib/Sema/CSDiagnostics.h
@@ -1823,6 +1823,15 @@ class NotCompileTimeConstFailure final : public FailureDiagnostic {
   bool diagnoseAsError() override;
 };
 
+class NotCopyableFailure final : public FailureDiagnostic {
+  Type noncopyableTy;
+public:
+  NotCopyableFailure(const Solution &solution, Type noncopyableTy, ConstraintLocator *locator)
+      : FailureDiagnostic(solution, locator), noncopyableTy(noncopyableTy) {}
+
+  bool diagnoseAsError() override;
+};
+
 /// Diagnose a contextual mismatch between expected collection element type
 /// and the one provided (e.g. source of the assignment or argument to a call)
 /// e.g.:
diff --git a/lib/Sema/CSFix.cpp b/lib/Sema/CSFix.cpp
@@ -1328,6 +1328,21 @@ bool NotCompileTimeConst::diagnose(const Solution &solution, bool asNote) const
   return failure.diagnose(asNote);
 }
 
+MustBeCopyable::MustBeCopyable(ConstraintSystem &cs, Type noncopyableTy, ConstraintLocator *locator)
+    : ConstraintFix(cs, FixKind::MustBeCopyable, locator, FixBehavior::Error),
+      noncopyableTy(noncopyableTy) {}
+
+bool MustBeCopyable::diagnose(const Solution &solution, bool asNote) const {
+  NotCopyableFailure failure(solution, noncopyableTy, getLocator());
+  return failure.diagnose(asNote);
+}
+
+MustBeCopyable* MustBeCopyable::create(ConstraintSystem &cs,
+                                              Type noncopyableTy,
+                                              ConstraintLocator *locator) {
+  return new (cs.getAllocator()) MustBeCopyable(cs, noncopyableTy, locator);
+}
+
 bool CollectionElementContextualMismatch::diagnose(const Solution &solution,
                                                    bool asNote) const {
   CollectionElementContextualFailure failure(
diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp
@@ -8129,6 +8129,14 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint(
           return SolutionKind::Solved;
       }
     }
+
+    // If this is a failure to conform to Copyable, tailor the error message.
+    if (protocol->isSpecificProtocol(KnownProtocolKind::Copyable)) {
+      auto *fix =
+          MustBeCopyable::create(*this, type, getConstraintLocator(locator));
+      if (!recordFix(fix))
+        return SolutionKind::Solved;
+    }
   }
 
   // There's nothing more we can do; fail.
@@ -13943,6 +13951,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
   case FixKind::IgnoreKeyPathContextualMismatch:
   case FixKind::NotCompileTimeConst:
   case FixKind::RenameConflictingPatternVariables:
+  case FixKind::MustBeCopyable:
   case FixKind::MacroMissingPound:
   case FixKind::AllowGlobalActorMismatch: {
     return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved;
diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp
@@ -1804,6 +1804,19 @@ TypeVariableType *ConstraintSystem::openGenericParameter(
   assert(result.second);
   (void)result;
 
+  // When move-only types are available, add a constraint to force generic
+  // parameters to conform to a "Copyable" protocol.
+  if (getASTContext().LangOpts.hasFeature(Feature::MoveOnly)) {
+    ProtocolDecl *copyable = TypeChecker::getProtocol(
+        getASTContext(), SourceLoc(), KnownProtocolKind::Copyable);
+
+    assert(copyable && "stdlib is missing _Copyable protocol!");
+    addConstraint(
+        ConstraintKind::ConformsTo, typeVar,
+        copyable->getDeclaredInterfaceType(),
+        locator.withPathElement(LocatorPathElt::GenericParameter(parameter)));
+  }
+
   return typeVar;
 }
 
diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp
@@ -2582,6 +2582,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
       }
     }
 
+    // FIXME(kavon): see if these can be integrated into other parts of Sema
     diagnoseCopyableTypeContainingMoveOnlyType(ED);
     diagnoseMoveOnlyNominalDeclDoesntConformToProtocols(ED);
 
diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp
@@ -964,6 +964,23 @@ static Type applyGenericArguments(Type type, TypeResolution resolution,
   return result;
 }
 
+/// if any of the generic args are a concrete move-only type, emit an error.
+/// returns true iff an error diagnostic was emitted
+static bool didDiagnoseMoveOnlyGenericArgs(ASTContext &ctx,
+                                         SourceLoc loc,
+                                         ArrayRef<Type> genericArgs) {
+  bool didEmitDiag = false;
+  for (auto t: genericArgs) {
+    if (!t->isPureMoveOnly())
+      continue;
+
+    ctx.Diags.diagnose(loc, diag::moveonly_generics, t);
+    didEmitDiag = true;
+  }
+
+  return didEmitDiag;
+}
+
 /// Apply generic arguments to the given type.
 Type TypeResolution::applyUnboundGenericArguments(
     GenericTypeDecl *decl, Type parentTy, SourceLoc loc,
@@ -984,6 +1001,10 @@ Type TypeResolution::applyUnboundGenericArguments(
   // do it when missing types are deduced.
   bool skipRequirementsCheck = false;
 
+  // check for generic args that are move-only
+  if (didDiagnoseMoveOnlyGenericArgs(getASTContext(), loc, genericArgs))
+    return ErrorType::get(getASTContext());
+
   // Get the substitutions for outer generic parameters from the parent
   // type.
   if (parentTy) {
@@ -1970,6 +1991,7 @@ namespace {
     }
 
     Type diagnoseDisallowedExistential(TypeRepr *repr, Type type);
+    bool diagnoseMoveOnly(TypeRepr *repr, Type genericArgTy);
 
     NeverNullType resolveOpenedExistentialArchetype(
         TypeAttributes &attrs, TypeRepr *repr,
@@ -2200,6 +2222,23 @@ Type TypeResolver::diagnoseDisallowedExistential(TypeRepr *repr, Type type) {
   return type;
 }
 
+/// Checks the given type, assuming that it appears as an argument for a
+/// generic parameter in the \c repr, to see if it is move-only.
+///
+/// Because generic type parameters currently all assume copyability of
+/// the substituted type, it's an error for a move-only type to appear
+/// as an argument for type parameters.
+///
+/// returns true if an error diagnostic was emitted
+bool TypeResolver::diagnoseMoveOnly(TypeRepr *repr, Type genericArgTy) {
+  if (genericArgTy->isPureMoveOnly()) {
+    diagnoseInvalid(repr, repr->getLoc(), diag::moveonly_generics,
+                    genericArgTy);
+    return true;
+  }
+  return false;
+}
+
 NeverNullType TypeResolver::resolveType(TypeRepr *repr,
                                         TypeResolutionOptions options) {
   assert(repr && "Cannot validate null TypeReprs!");
@@ -4190,6 +4229,10 @@ NeverNullType TypeResolver::resolveArrayType(ArrayTypeRepr *repr,
     return ErrorType::get(ctx);
   }
 
+  // do not allow move-only types in an array
+  if (diagnoseMoveOnly(repr, baseTy))
+    return ErrorType::get(ctx);
+
   return ArraySliceType::get(baseTy);
 }
 
@@ -4240,6 +4283,10 @@ NeverNullType TypeResolver::resolveOptionalType(OptionalTypeRepr *repr,
     return ErrorType::get(getASTContext());
   }
 
+  // do not allow move-only types in an optional
+  if (diagnoseMoveOnly(repr, baseTy))
+    return ErrorType::get(getASTContext());
+
   return optionalTy;
 }
 
@@ -4337,6 +4384,10 @@ NeverNullType TypeResolver::resolveImplicitlyUnwrappedOptionalType(
     return ErrorType::get(getASTContext());
   }
 
+  // do not allow move-only types in an implicitly-unwrapped optional
+  if (diagnoseMoveOnly(repr, baseTy))
+    return ErrorType::get(getASTContext());
+
   return uncheckedOptionalTy;
 }
 
@@ -4351,6 +4402,10 @@ NeverNullType TypeResolver::resolveVarargType(VarargTypeRepr *repr,
       .highlight(repr->getSourceRange());
   }
 
+  // do not allow move-only types as the element of a vararg
+  if (diagnoseMoveOnly(repr, element))
+    return ErrorType::get(getASTContext());
+
   return element;
 }
 
diff --git a/stdlib/public/core/Misc.swift b/stdlib/public/core/Misc.swift
@@ -154,3 +154,8 @@ public func _getTypeByMangledNameInContext(
 public func _unsafePerformance<T>(_ c: () -> T) -> T {
   return c()
 }
+
+
+/// This is not a protocol you can explicitly use in your programs.
+/// It exists for the compiler and type checker for diagnostic purposes.
+@_marker public protocol _Copyable { }
diff --git a/test/Constraints/moveonly_constraints.swift b/test/Constraints/moveonly_constraints.swift
diff --git a/test/Sema/copyable.swift b/test/Sema/copyable.swift
diff --git a/test/Sema/moveonly_illegal_types.swift b/test/Sema/moveonly_illegal_types.swift

Original file line number	Diff line number	Diff line change
`@@ -5885,6 +5885,7 @@ SpecialProtocol irgen::getSpecialProtocolID(ProtocolDecl *P) {`
`5885`	`5885`	`case KnownProtocolKind::UnsafeSendable:`
`5886`	`5886`	`case KnownProtocolKind::RangeReplaceableCollection:`
`5887`	`5887`	`case KnownProtocolKind::GlobalActor:`
	`5888`	`+ case KnownProtocolKind::Copyable:`
`5888`	`5889`	`return SpecialProtocol::None;`
`5889`	`5890`	`}`
`5890`	`5891`