require an ownership specifier for noncopyable parameters

Per the current proposal, these are to be specified
explicitly, as they form an important part of the API.

Bonus: This commit includes a fix to make
`CompileTimeConstTypeRepr` a proper `isa<>` subtype of
`SpecifierTypeRepr`, since we forgot to add it to
that type's `classof` function.

resolves rdar://105480354

Author: kavon

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/Sema/moveonly_require_ownership_specifier.swift

@@ -0,0 +1,86 @@
+// RUN: %target-typecheck-verify-swift -enable-experimental-move-only
+
+// Coverage for check that requires some ownership specifier to be written
+// when a move-only / noncopyable type appears as a parameter of a function.
+
+@_moveOnly
+struct MO {
+    var x = 0
+}
+
+struct Box<T> { var val: T }
+
+@_moveOnly
+public struct NoncopyableWrapper<T> {
+  var x: T
+}
+
+class Inspector {
+  func inspect(_ hasIt: inout MO, _ mo: MO, _ hasItAgain: __owned MO) {}
+  // expected-error@-1 {{noncopyable parameter must specify its ownership}}
+  // expected-note@-2 {{add '__shared' for an immutable reference}}{{41-41=__shared }}
+  // expected-note@-3 {{add 'inout' for a mutable reference}}{{41-41=inout }}
+  // expected-note@-4 {{add '__owned' to take the value from callers}}{{41-41=__owned }}
+}
+
+// expected-error@+4 {{noncopyable parameter must specify its ownership}}
+// expected-note@+3 {{add '__shared' for an immutable reference}}{{20-20=__shared }}
+// expected-note@+2 {{add 'inout' for a mutable reference}}{{20-20=inout }}
+// expected-note@+1 {{add '__owned' to take the value from callers}}{{20-20=__owned }}
+func applier(_ f: (MO) -> (),
+             _ v: MO) {}
+// expected-error@-1 {{noncopyable parameter must specify its ownership}}
+// expected-note@-2 {{add '__shared' for an immutable reference}}{{19-19=__shared }}
+// expected-note@-3 {{add 'inout' for a mutable reference}}{{19-19=inout }}
+// expected-note@-4 {{add '__owned' to take the value from callers}}{{19-19=__owned }}
+
+func caller() {
+  let f = { (_ mo1: MO, _ mo2: MO) in () }
+  // expected-error@-1 2{{noncopyable parameter must specify its ownership}}
+  // expected-note@-2 {{add '__shared' for an immutable reference}}{{21-21=__shared }}
+  // expected-note@-3 {{add '__shared' for an immutable reference}}{{32-32=__shared }}
+  // expected-note@-4 {{add 'inout' for a mutable reference}}{{21-21=inout }}
+  // expected-note@-5 {{add 'inout' for a mutable reference}}{{32-32=inout }}
+  // expected-note@-6 {{add '__owned' to take the value from callers}}{{21-21=__owned }}
+  // expected-note@-7 {{add '__owned' to take the value from callers}}{{32-32=__owned }}
+
+  let g: (MO, MO) -> () = f
+  // expected-error@-1 2{{noncopyable parameter must specify its ownership}}
+  // expected-note@-2 {{add '__shared' for an immutable reference}}{{11-11=__shared }}
+  // expected-note@-3 {{add '__shared' for an immutable reference}}{{15-15=__shared }}
+  // expected-note@-4 {{add 'inout' for a mutable reference}}{{11-11=inout }}
+  // expected-note@-5 {{add 'inout' for a mutable reference}}{{15-15=inout }}
+  // expected-note@-6 {{add '__owned' to take the value from callers}}{{11-11=__owned }}
+  // expected-note@-7 {{add '__owned' to take the value from callers}}{{15-15=__owned }}
+
+  let partialG = { g($0, MO()) }
+
+  let _: Box<(MO) -> ()> = Box(val: partialG)
+  // expected-error@-1 {{noncopyable parameter must specify its ownership}}
+  // expected-note@-2 {{add '__shared' for an immutable reference}}{{15-15=__shared }}
+  // expected-note@-3 {{add 'inout' for a mutable reference}}{{15-15=inout }}
+  // expected-note@-4 {{add '__owned' to take the value from callers}}{{15-15=__owned }}
+
+  let _: Box<(inout MO) -> ()>? = nil
+  let _: Box<(__shared MO) -> ()>? = nil
+
+  let _: Box<(MO) -> ()>? = nil
+  // expected-error@-1 {{noncopyable parameter must specify its ownership}}
+  // expected-note@-2 {{add '__shared' for an immutable reference}}{{15-15=__shared }}
+  // expected-note@-3 {{add 'inout' for a mutable reference}}{{15-15=inout }}
+  // expected-note@-4 {{add '__owned' to take the value from callers}}{{15-15=__owned }}
+
+  applier(partialG, MO())
+}
+
+func takeGeneric<T>(_ x: NoncopyableWrapper<T>) {}
+// expected-error@-1 {{noncopyable parameter must specify its ownership}}
+// expected-note@-2 {{add '__shared' for an immutable reference}}{{26-26=__shared }}
+// expected-note@-3 {{add 'inout' for a mutable reference}}{{26-26=inout }}
+// expected-note@-4 {{add '__owned' to take the value from callers}}{{26-26=__owned }}
+
+func takeInstantiated(_ x: NoncopyableWrapper<Int>) {}
+// expected-error@-1 {{noncopyable parameter must specify its ownership}}
+// expected-note@-2 {{add '__shared' for an immutable reference}}{{28-28=__shared }}
+// expected-note@-3 {{add 'inout' for a mutable reference}}{{28-28=inout }}
+// expected-note@-4 {{add '__owned' to take the value from callers}}{{28-28=__owned }}