Merge pull request #65898 from kavon/subscript-ownership-fixit

improve error when using noncopyable type with subscript

Author: swift-ci

include/swift/AST/DiagnosticsSema.def

@@ -7143,6 +7143,8 @@ ERROR(moveonly_cannot_conform_to_type, none,
      (DescriptiveDeclKind, DeclName, Type))
 ERROR(moveonly_parameter_missing_ownership, none,
       "noncopyable parameter must specify its ownership", ())
+ERROR(moveonly_parameter_subscript_unsupported, none,
+      "subscripts cannot have noncopyable parameters yet", ())
 NOTE(moveonly_parameter_ownership_suggestion, none,
       "add '%0' %1", (StringRef, StringRef))
 ERROR(ownership_specifier_copyable,none,

lib/Sema/TypeCheckType.cpp

@@ -2307,31 +2307,39 @@ bool TypeResolver::diagnoseMoveOnlyMissingOwnership(
   if (options.contains(TypeResolutionFlags::HasOwnership))
     return false;
 
-  // Do not run this on SIL files since there is currently a bug where we are
-  // trying to parse it in SILBoxes.
-  //
-  // To track what we want to do long term: rdar://105635373.
+  // Don't diagnose in SIL; ownership is already required there.
   if (options.contains(TypeResolutionFlags::SILType))
     return false;
 
-  diagnose(repr->getLoc(),
-           diag::moveonly_parameter_missing_ownership);
+  //////////////////
+  // At this point, we know we have a noncopyable parameter that is missing an
+  // ownership specifier, so we need to emit an error
+
+  // We don't yet support any ownership specifiers for parameters of subscript
+  // decls, give a tailored error message saying you simply can't use a
+  // noncopyable type here.
+  if (options.hasBase(TypeResolverContext::SubscriptDecl)) {
+    diagnose(repr->getLoc(), diag::moveonly_parameter_subscript_unsupported);
+  } else {
+    // general error diagnostic
+    diagnose(repr->getLoc(),
+             diag::moveonly_parameter_missing_ownership);
 
-  diagnose(repr->getLoc(), diag::moveonly_parameter_ownership_suggestion,
-           "borrowing", "for an immutable reference")
-      .fixItInsert(repr->getStartLoc(), "borrowing ");
+    diagnose(repr->getLoc(), diag::moveonly_parameter_ownership_suggestion,
+             "borrowing", "for an immutable reference")
+        .fixItInsert(repr->getStartLoc(), "borrowing ");
 
-  diagnose(repr->getLoc(), diag::moveonly_parameter_ownership_suggestion,
-           "inout", "for a mutable reference")
-      .fixItInsert(repr->getStartLoc(), "inout ");
+    diagnose(repr->getLoc(), diag::moveonly_parameter_ownership_suggestion,
+             "inout", "for a mutable reference")
+        .fixItInsert(repr->getStartLoc(), "inout ");
 
-  diagnose(repr->getLoc(), diag::moveonly_parameter_ownership_suggestion,
-           "consuming", "to take the value from the caller")
-      .fixItInsert(repr->getStartLoc(), "consuming ");
+    diagnose(repr->getLoc(), diag::moveonly_parameter_ownership_suggestion,
+             "consuming", "to take the value from the caller")
+        .fixItInsert(repr->getStartLoc(), "consuming ");
+  }
 
   // to avoid duplicate diagnostics
   repr->setInvalid();
-
   return true;
 }
 

test/Sema/moveonly_require_ownership_specifier.swift

@@ -84,3 +84,12 @@ func takeInstantiated(_ x: NoncopyableWrapper<Int>) {}
 // expected-note@-2 {{add 'borrowing' for an immutable reference}}{{28-28=borrowing }}
 // expected-note@-3 {{add 'inout' for a mutable reference}}{{28-28=inout }}
 // expected-note@-4 {{add 'consuming' to take the value from the caller}}{{28-28=consuming }}
+
+struct O: ~Copyable {}
+
+public struct M: ~Copyable {
+  subscript(_ i: O) -> Int { // expected-error {{subscripts cannot have noncopyable parameters}}
+    get { fatalError() }
+    set { }
+  }
+}