Merge pull request swiftlang#75410 from kavon/kavon-fixes-1

Small Noncopyable Fixes, pt 1

Author: kavon

include/swift/AST/DiagnosticsSema.def

@@ -654,6 +654,8 @@ ERROR(expr_keypath_type_of_property,none,
       (DeclNameRef, Type))
 ERROR(expr_keypath_generic_type,none,
       "key path cannot refer to generic type %0", (Identifier))
+ERROR(expr_keypath_noncopyable_type,none,
+      "key path cannot refer to noncopyable type %0", (Type))
 ERROR(expr_keypath_not_property,none,
       "%select{key path|dynamic key path member lookup}1 cannot refer to %kind0",
       (const ValueDecl *, bool))
@@ -7741,6 +7743,8 @@ ERROR(noncopyable_objc_enum, none,
       "noncopyable enums cannot be marked '@objc'", ())
 ERROR(moveOnly_not_allowed_here,none,
       "'@_moveOnly' attribute is only valid on structs or enums", ())
+ERROR(moveOnly_deprecated,none,
+      "'@_moveOnly' attribute is deprecated and will be removed; use '~Copyable' instead", ())
 ERROR(consume_expression_needed_for_cast,none,
       "implicit conversion to %0 is consuming", (Type))
 NOTE(add_consume_to_silence,none,

lib/Sema/CSApply.cpp

@@ -384,10 +384,12 @@ static bool willHaveConfusingConsumption(Type type,
   case ConstraintLocator::ApplyArgToParam: {
     auto argLoc = loc->castLastElementTo<LocatorPathElt::ApplyArgToParam>();
     auto paramFlags = argLoc.getParameterFlags();
-    if (paramFlags.getOwnershipSpecifier() == ParamSpecifier::Consuming)
-      return false; // Parameter already declares 'consuming'.
+    // If the param declares borrowing, then this implicit consumption
+    // due to the conversion to pass the argument is indeed confusing.
+    if (paramFlags.getOwnershipSpecifier() == ParamSpecifier::Borrowing)
+      return true;
 
-    return true;
+    return false;
   }
 
   default:

lib/Sema/MiscDiagnostics.cpp

@@ -168,10 +168,8 @@ static void diagSyntacticUseRestrictions(const Expr *E, const DeclContext *DC,
       if (isa<TypeExpr>(Base))
         checkUseOfMetaTypeName(Base);
 
-      if (auto *KPE = dyn_cast<KeyPathExpr>(E)) {
-        // raise an error if this KeyPath contains an effectful member.
-        checkForEffectfulKeyPath(KPE);
-      }
+      if (auto *KPE = dyn_cast<KeyPathExpr>(E))
+        checkForInvalidKeyPath(KPE);
 
       // Check function calls, looking through implicit conversions on the
       // function and inspecting the arguments directly.
@@ -364,11 +362,15 @@ static void diagSyntacticUseRestrictions(const Expr *E, const DeclContext *DC,
     }
 
     /// Visit each component of the keypath and emit a diagnostic if they
-    /// refer to a member that has effects.
-    void checkForEffectfulKeyPath(KeyPathExpr *keyPath) {
+    /// refer to a member that meets any of the following:
+    ///   - has effects.
+    ///   - is a noncopyable type.
+    void checkForInvalidKeyPath(KeyPathExpr *keyPath) {
       for (const auto &component : keyPath->getComponents()) {
         if (component.hasDeclRef()) {
           auto decl = component.getDeclRef().getDecl();
+
+          // Check for effects
           if (auto asd = dyn_cast<AbstractStorageDecl>(decl)) {
             if (auto getter = asd->getEffectfulGetAccessor()) {
               Ctx.Diags.diagnose(component.getLoc(),
@@ -378,6 +380,13 @@ static void diagSyntacticUseRestrictions(const Expr *E, const DeclContext *DC,
                                  asd->getDescriptiveKind());
             }
           }
+
+          // Check for the ability to copy.
+          if (component.getComponentType()->isNoncopyable()) {
+            Ctx.Diags.diagnose(component.getLoc(),
+                               diag::expr_keypath_noncopyable_type,
+                               component.getComponentType()->getRValueType());
+          }
         }
       }
     }

lib/Sema/TypeCheckAttr.cpp

@@ -2546,6 +2546,12 @@ void AttributeChecker::visitFinalAttr(FinalAttr *attr) {
 }
 
 void AttributeChecker::visitMoveOnlyAttr(MoveOnlyAttr *attr) {
+  // This attribute is deprecated and slated for removal.
+  diagnose(attr->getLocation(), diag::moveOnly_deprecated)
+    .fixItRemove(attr->getRange())
+    .warnInSwiftInterface(D->getDeclContext());
+
+
   if (isa<StructDecl>(D) || isa<EnumDecl>(D))
     return;
 

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -113,7 +113,7 @@ class CheckRepressions {
   /// repressed or return the inverted type.
   Type add(Type ty, InverseTypeRepr &repr,
            llvm::PointerUnion<const TypeDecl *, const ExtensionDecl *> decl) {
-    if (!ty)
+    if (!ty || ty->hasError())
       return Type();
     assert(!ty->is<ExistentialMetatypeType>());
     auto kp = ty->getKnownProtocol();

stdlib/public/Concurrency/PartialAsyncTask.swift

@@ -193,8 +193,7 @@ extension UnownedJob: CustomStringConvertible {
 @available(SwiftStdlib 5.9, *)
 @available(*, deprecated, renamed: "ExecutorJob")
 @frozen
-@_moveOnly
-public struct Job: Sendable {
+public struct Job: Sendable, ~Copyable {
   internal var context: Builtin.Job
 
   @usableFromInline
@@ -262,8 +261,7 @@ extension Job {
 /// you don't generally interact with jobs directly.
 @available(SwiftStdlib 5.9, *)
 @frozen
-@_moveOnly
-public struct ExecutorJob: Sendable {
+public struct ExecutorJob: Sendable, ~Copyable {
   internal var context: Builtin.Job
 
   @usableFromInline

test/Concurrency/transfernonsendable.sil

@@ -48,8 +48,7 @@ sil @transferKlassContainingKlasses : $@convention(thin) @async (@guaranteed Kla
 sil @useKlassContainingKlasses : $@convention(thin) (@guaranteed KlassContainingKlasses) -> ()
 sil @constructKlassContainingKlasses : $@convention(thin) () -> @owned KlassContainingKlasses
 
-@_moveOnly
-struct NonSendableMoveOnlyStruct {
+struct NonSendableMoveOnlyStruct: ~Copyable {
   var ns: NonSendableKlass
 
   deinit

test/Concurrency/transfernonsendable_instruction_matching.sil

@@ -42,8 +42,7 @@ sil @transferKlassContainingKlasses : $@convention(thin) @async (@guaranteed Kla
 sil @useKlassContainingKlasses : $@convention(thin) (@guaranteed KlassContainingKlasses) -> ()
 sil @constructKlassContainingKlasses : $@convention(thin) () -> @owned KlassContainingKlasses
 
-@_moveOnly
-struct NonSendableMoveOnlyStruct {
+struct NonSendableMoveOnlyStruct: ~Copyable {
   var ns: NonSendableKlass
 
   deinit

test/Concurrency/transfernonsendable_instruction_matching_differentiability.sil

@@ -36,8 +36,7 @@ sil @transferKlassContainingKlasses : $@convention(thin) @async (@guaranteed Kla
 sil @useKlassContainingKlasses : $@convention(thin) (@guaranteed KlassContainingKlasses) -> ()
 sil @constructKlassContainingKlasses : $@convention(thin) () -> @owned KlassContainingKlasses
 
-@_moveOnly
-struct NonSendableMoveOnlyStruct {
+struct NonSendableMoveOnlyStruct: ~Copyable {
   var ns: NonSendableKlass
 
   deinit

test/Concurrency/transfernonsendable_instruction_matching_opaquevalues.sil

@@ -22,8 +22,7 @@ class NonSendableKlass {}
 
 final class SendableKlass : Sendable {}
 
-@_moveOnly
-struct NonSendableMoveOnlyStruct {
+struct NonSendableMoveOnlyStruct: ~Copyable {
   var ns: NonSendableKlass
 }
 
@@ -194,4 +193,4 @@ bb0:
 
   %9999 = tuple ()
   return %9999 : $()
-}
+}