Merge pull request #63674 from kavon/require-ownership-specifier-noncopyable

Require parameters of noncopyable type to specify ownership

Author: swift-ci

include/swift/AST/DiagnosticsSema.def

@@ -6864,6 +6864,10 @@ ERROR(moveonly_cannot_conform_to_type, none,
      (DescriptiveDeclKind, DeclName, Type))
 ERROR(moveonly_non_final_class_cannot_contain_moveonly_field, none,
      "non-final classes containing move only fields is not yet supported", ())
+ERROR(moveonly_parameter_missing_ownership, none,
+      "noncopyable parameter must specify its ownership", ())
+NOTE(moveonly_parameter_ownership_suggestion, none,
+      "add '%0' %1", (StringRef, StringRef))
 
 //------------------------------------------------------------------------------
 // MARK: Runtime discoverable attributes (@runtimeMetadata)

include/swift/AST/TypeRepr.h

@@ -1054,6 +1054,8 @@ class SpecifierTypeRepr : public TypeRepr {
 public:
   SpecifierTypeRepr(TypeReprKind Kind, TypeRepr *Base, SourceLoc Loc)
     : TypeRepr(Kind), Base(Base), SpecifierLoc(Loc) {
+    // ensure the kind passed-in matches up with our TypeRepr classof.
+    assert(SpecifierTypeRepr::classof(cast<TypeRepr>(this)));
   }
 
   TypeRepr *getBase() const { return Base; }
@@ -1063,7 +1065,8 @@ class SpecifierTypeRepr : public TypeRepr {
     return T->getKind() == TypeReprKind::InOut ||
            T->getKind() == TypeReprKind::Shared ||
            T->getKind() == TypeReprKind::Owned ||
-           T->getKind() == TypeReprKind::Isolated;
+           T->getKind() == TypeReprKind::Isolated ||
+           T->getKind() == TypeReprKind::CompileTimeConst;
   }
   static bool classof(const SpecifierTypeRepr *T) { return true; }
 
@@ -1073,45 +1076,65 @@ class SpecifierTypeRepr : public TypeRepr {
   void printImpl(ASTPrinter &Printer, const PrintOptions &Opts) const;
   friend class TypeRepr;
 };
+
+// This repr holds types that are associated with some ownership specifier
+// like 'inout', '__shared', etc. It's a simple grouping of those specifiers.
+class OwnershipTypeRepr : public SpecifierTypeRepr {
+public:
+  OwnershipTypeRepr(TypeReprKind Kind, TypeRepr *Base, SourceLoc OwnershipLoc)
+      : SpecifierTypeRepr(Kind, Base, OwnershipLoc) {
+    assert(OwnershipTypeRepr::classof(cast<TypeRepr>(this)));
+  }
+
+  static bool classof(const TypeRepr *T) {
+    return T->getKind() == TypeReprKind::InOut ||
+           T->getKind() == TypeReprKind::Shared ||
+           T->getKind() == TypeReprKind::Owned;
+  }
+  static bool classof(const OwnershipTypeRepr *T) { return true; }
+  friend class SpecifierTypeRepr;
+};
 
 /// An 'inout' type.
 /// \code
 ///   x : inout Int
 /// \endcode
-class InOutTypeRepr : public SpecifierTypeRepr {
+class InOutTypeRepr : public OwnershipTypeRepr {
 public:
   InOutTypeRepr(TypeRepr *Base, SourceLoc InOutLoc)
-    : SpecifierTypeRepr(TypeReprKind::InOut, Base, InOutLoc) {}
+    : OwnershipTypeRepr(TypeReprKind::InOut, Base, InOutLoc) {}
 
   static bool classof(const TypeRepr *T) {
     return T->getKind() == TypeReprKind::InOut;
   }
   static bool classof(const InOutTypeRepr *T) { return true; }
 };
 
-/// A 'shared' type.
+/// A 'borrowing' parameter type.
 /// \code
-///   x : shared Int
+///   x : borrowing Int
 /// \endcode
-class SharedTypeRepr : public SpecifierTypeRepr {
+/// Historically, this attribute was spelled '__shared'.
+class SharedTypeRepr : public OwnershipTypeRepr {
 public:
   SharedTypeRepr(TypeRepr *Base, SourceLoc SharedLoc)
-    : SpecifierTypeRepr(TypeReprKind::Shared, Base, SharedLoc) {}
+    : OwnershipTypeRepr(TypeReprKind::Shared, Base, SharedLoc) {}
 
   static bool classof(const TypeRepr *T) {
     return T->getKind() == TypeReprKind::Shared;
   }
   static bool classof(const SharedTypeRepr *T) { return true; }
 };
 
-/// A 'owned' type.
+/// A 'consuming' parameter type.
 /// \code
-///   x : owned Int
+///   x : consuming Int
 /// \endcode
-class OwnedTypeRepr : public SpecifierTypeRepr {
+/// Historically, this attribute was spelled '__owned'.
+class OwnedTypeRepr : public OwnershipTypeRepr {
 public:
   OwnedTypeRepr(TypeRepr *Base, SourceLoc OwnedLoc)
-      : SpecifierTypeRepr(TypeReprKind::Owned, Base, OwnedLoc) {}
+      : OwnershipTypeRepr(TypeReprKind::Owned, Base, OwnedLoc) {}
 
   static bool classof(const TypeRepr *T) {
     return T->getKind() == TypeReprKind::Owned;

lib/Parse/ParsePattern.cpp

@@ -607,13 +607,10 @@ mapParsedParameters(Parser &parser,
           if (auto *STR = dyn_cast<SpecifierTypeRepr>(unwrappedType)) {
             if (isa<IsolatedTypeRepr>(STR))
               param->setIsolated(true);
-            unwrappedType = STR->getBase();
-            continue;
-          }
+            else if (isa<CompileTimeConstTypeRepr>(STR))
+              param->setCompileTimeConst(true);
 
-          if (auto *CTR = dyn_cast<CompileTimeConstTypeRepr>(unwrappedType)) {
-            param->setCompileTimeConst(true);
-            unwrappedType = CTR->getBase();
+            unwrappedType = STR->getBase();
             continue;
           }
 

lib/Sema/TypeCheckGeneric.cpp

@@ -653,12 +653,16 @@ GenericSignatureRequest::evaluate(Evaluator &evaluator,
       for (auto param : *params) {
         auto *typeRepr = param->getTypeRepr();
         if (typeRepr == nullptr)
-          continue;
+            continue;
 
         auto paramOptions = baseOptions;
 
-        if (auto *specifier = dyn_cast<SpecifierTypeRepr>(typeRepr))
+        if (auto *specifier = dyn_cast<SpecifierTypeRepr>(typeRepr)) {
+          if (isa<OwnershipTypeRepr>(specifier))
+            paramOptions |= TypeResolutionFlags::HasOwnership;
+
           typeRepr = specifier->getBase();
+        }
 
         if (auto *packExpansion = dyn_cast<VarargTypeRepr>(typeRepr)) {
           paramOptions.setContext(TypeResolverContext::VariadicFunctionInput);

lib/Sema/TypeCheckType.cpp

@@ -1990,6 +1990,8 @@ namespace {
     }
 
     bool diagnoseMoveOnly(TypeRepr *repr, Type genericArgTy);
+    bool diagnoseMoveOnlyMissingOwnership(TypeRepr *repr,
+                                          TypeResolutionOptions options);
 
     bool diagnoseDisallowedExistential(TypeRepr *repr);
 
@@ -2044,7 +2046,7 @@ namespace {
                                 Optional<SILResultInfo> &errorResult);
     NeverNullType resolveDeclRefTypeRepr(DeclRefTypeRepr *repr,
                                          TypeResolutionOptions options);
-    NeverNullType resolveSpecifierTypeRepr(SpecifierTypeRepr *repr,
+    NeverNullType resolveOwnershipTypeRepr(OwnershipTypeRepr *repr,
                                            TypeResolutionOptions options);
     NeverNullType resolveIsolatedTypeRepr(IsolatedTypeRepr *repr,
                                           TypeResolutionOptions options);
@@ -2250,6 +2252,51 @@ bool TypeResolver::diagnoseMoveOnly(TypeRepr *repr, Type genericArgTy) {
   return false;
 }
 
+/// Assuming this repr has resolved to a move-only / noncopyable type, checks
+/// to see if that resolution happened in a context requiring an ownership
+/// annotation. If it did and there was no ownership specified, emits a
+/// diagnostic.
+///
+/// \returns true if an error diagnostic was emitted
+bool TypeResolver::diagnoseMoveOnlyMissingOwnership(
+                                              TypeRepr *repr,
+                                              TypeResolutionOptions options) {
+  // only required on function inputs.
+  // we can ignore InoutFunctionInput since it's already got ownership.
+  if (!options.is(TypeResolverContext::FunctionInput))
+    return false;
+
+  // enum cases don't need to specify ownership for associated values
+  if (options.hasBase(TypeResolverContext::EnumElementDecl))
+    return false;
+
+  // otherwise, we require ownership.
+  if (options.contains(TypeResolutionFlags::HasOwnership))
+    return false;
+
+  diagnose(repr->getLoc(),
+           diag::moveonly_parameter_missing_ownership);
+
+  // FIXME: this should be 'borrowing'
+  diagnose(repr->getLoc(), diag::moveonly_parameter_ownership_suggestion,
+           "__shared", "for an immutable reference")
+      .fixItInsert(repr->getStartLoc(), "__shared ");
+
+  diagnose(repr->getLoc(), diag::moveonly_parameter_ownership_suggestion,
+           "inout", "for a mutable reference")
+      .fixItInsert(repr->getStartLoc(), "inout ");
+
+  // FIXME: this should be 'consuming'
+  diagnose(repr->getLoc(), diag::moveonly_parameter_ownership_suggestion,
+           "__owned", "to take the value from callers")
+      .fixItInsert(repr->getStartLoc(), "__owned ");
+
+  // to avoid duplicate diagnostics
+  repr->setInvalid();
+
+  return true;
+}
+
 NeverNullType TypeResolver::resolveType(TypeRepr *repr,
                                         TypeResolutionOptions options) {
   assert(repr && "Cannot validate null TypeReprs!");
@@ -2284,7 +2331,7 @@ NeverNullType TypeResolver::resolveType(TypeRepr *repr,
   case TypeReprKind::InOut:
   case TypeReprKind::Shared:
   case TypeReprKind::Owned:
-    return resolveSpecifierTypeRepr(cast<SpecifierTypeRepr>(repr), options);
+    return resolveOwnershipTypeRepr(cast<OwnershipTypeRepr>(repr), options);
 
   case TypeReprKind::Isolated:
     return resolveIsolatedTypeRepr(cast<IsolatedTypeRepr>(repr), options);
@@ -4146,6 +4193,10 @@ TypeResolver::resolveDeclRefTypeRepr(DeclRefTypeRepr *repr,
     }
   }
 
+  // move-only types must have an ownership specifier when used as a parameter of a function.
+  if (result->isPureMoveOnly())
+    diagnoseMoveOnlyMissingOwnership(repr, options);
+
   // Hack to apply context-specific @escaping to a typealias with an underlying
   // function type.
   if (result->is<FunctionType>())
@@ -4155,9 +4206,9 @@ TypeResolver::resolveDeclRefTypeRepr(DeclRefTypeRepr *repr,
 }
 
 NeverNullType
-TypeResolver::resolveSpecifierTypeRepr(SpecifierTypeRepr *repr,
+TypeResolver::resolveOwnershipTypeRepr(OwnershipTypeRepr *repr,
                                        TypeResolutionOptions options) {
-  // inout is only valid for (non-Subscript and non-EnumCaseDecl)
+  // ownership is only valid for (non-Subscript and non-EnumCaseDecl)
   // function parameters.
   if (!options.is(TypeResolverContext::FunctionInput) ||
       options.hasBase(TypeResolverContext::SubscriptDecl) ||
@@ -4177,10 +4228,10 @@ TypeResolver::resolveSpecifierTypeRepr(SpecifierTypeRepr *repr,
       name = "inout";
       break;
     case TypeReprKind::Shared:
-      name = "__shared";
+      name = "__shared"; // FIXME: use 'borrowing'
       break;
     case TypeReprKind::Owned:
-      name = "__owned";
+      name = "__owned";  // FIXME: use 'consuming'
       break;
     default:
       llvm_unreachable("unknown SpecifierTypeRepr kind");
@@ -4195,6 +4246,9 @@ TypeResolver::resolveSpecifierTypeRepr(SpecifierTypeRepr *repr,
     options.setContext(TypeResolverContext::InoutFunctionInput);
   }
 
+  // Remember that we've seen an ownership specifier for this base type.
+  options |= TypeResolutionFlags::HasOwnership;
+
   return resolveType(repr->getBase(), options);
 }
 

lib/Sema/TypeCheckType.h

@@ -81,6 +81,9 @@ enum class TypeResolutionFlags : uint16_t {
 
   /// Whether this is a resolution based on a pack reference.
   FromPackReference = 1 << 12,
+
+  /// Whether this resolution happens under an explicit ownership specifier.
+  HasOwnership = 1 << 13,
 };
 
 /// Type resolution contexts that require special handling.

test/Constraints/moveonly_constraints.swift

@@ -42,7 +42,7 @@ enum Maybe<T> {
   case just(T)
 }
 
-func takeConcrete(_ m: MO) {}
+func takeConcrete(_ m: __shared MO) {}
 func takeGeneric<T>(_ t: T) {}
 func takeGenericSendable<T>(_ t: T) where T: Sendable {}
 func takeMaybe<T>(_ m: Maybe<T>) {}
@@ -71,7 +71,7 @@ func testAny() {
   takeAny(MO()) // expected-error {{move-only type 'MO' cannot be used with generics yet}}
 }
 
-func testBasic(_ mo: MO) {
+func testBasic(_ mo: __shared MO) {
   takeConcrete(globalMO)
   takeConcrete(MO())
 
@@ -139,7 +139,7 @@ func checkMethodCalls() {
   takeMaybe(true ? .none : .just(MO())) // expected-error 3{{move-only type 'MO' cannot be used with generics yet}}
 }
 
-func checkCasting(_ b: any Box, _ mo: MO, _ a: Any) {
+func checkCasting(_ b: any Box, _ mo: __shared MO, _ a: Any) {
   // casting dynamically is allowed, but should always fail since you can't
   // construct such a type.
   let box = b as! ValBox<MO> // expected-error {{move-only type 'MO' cannot be used with generics yet}}
@@ -221,7 +221,7 @@ func checkCasting(_ b: any Box, _ mo: MO, _ a: Any) {
 
 }
 
-func checkStdlibTypes(_ mo: MO) {
+func checkStdlibTypes(_ mo: __shared MO) {
   let _: [MO] = // expected-error {{move-only type 'MO' cannot be used with generics yet}}
       [MO(), MO()]
   let _: [MO] = // expected-error {{move-only type 'MO' cannot be used with generics yet}}

test/Interpreter/moveonly_bufferview.swift

@@ -23,7 +23,7 @@ public struct BufferView<T> {
 }
 
 extension Array {
-    public mutating func withBufferView<U>(_ f: (BufferView<Element>) -> U) -> U {
+    public mutating func withBufferView<U>(_ f: (__shared BufferView<Element>) -> U) -> U {
         return withUnsafeBufferPointer {
             return f(BufferView(ptr: $0))
         }