Merge pull request #65556 from kavon/barebones-suppress-copyable

Build support for `~Copyable` atop `@_moveOnly`

Author: kavon

include/swift/AST/DiagnosticsParse.def

@@ -944,6 +944,19 @@ ERROR(expected_rparen_layout_constraint,none,
 ERROR(layout_constraints_only_inside_specialize_attr,none,
       "layout constraints are only allowed inside '_specialize' attributes", ())
 
+//------------------------------------------------------------------------------
+// MARK: Conformance suppression diagnostics
+//------------------------------------------------------------------------------
+
+ERROR(cannot_suppress_here,none,
+      "cannot suppress conformances here", ())
+
+ERROR(only_suppress_copyable,none,
+      "can only suppress 'Copyable'", ())
+
+ERROR(already_suppressed,none,
+      "duplicate suppression of %0", (Identifier))
+
 //------------------------------------------------------------------------------
 // MARK: Pattern parsing diagnostics
 //------------------------------------------------------------------------------

include/swift/AST/KnownIdentifiers.def

@@ -56,6 +56,7 @@ IDENTIFIER(Context)
 IDENTIFIER(CoreGraphics)
 IDENTIFIER(CoreMedia)
 IDENTIFIER(CGFloat)
+IDENTIFIER(Copyable)
 IDENTIFIER(CoreFoundation)
 IDENTIFIER(count)
 IDENTIFIER(CVarArg)

include/swift/Parse/Parser.h

@@ -1195,9 +1195,18 @@ class Parser {
 
   ParserResult<ImportDecl> parseDeclImport(ParseDeclOptions Flags,
                                            DeclAttributes &Attributes);
+
+  /// Parse an inheritance clause into a vector of InheritedEntry's.
+  ///
+  /// \param allowClassRequirement whether to permit parsing of 'class'
+  /// \param allowAnyObject whether to permit parsing of 'AnyObject'
+  /// \param parseTildeCopyable if non-null, permits parsing of `~Copyable`
+  ///   and writes out a valid source location if it was parsed. If null, then a
+  ///   parsing error will be emitted upon the appearance of `~` in the clause.
   ParserStatus parseInheritance(SmallVectorImpl<InheritedEntry> &Inherited,
                                 bool allowClassRequirement,
-                                bool allowAnyObject);
+                                bool allowAnyObject,
+                                SourceLoc *parseTildeCopyable = nullptr);
   ParserStatus parseDeclItem(bool &PreviousHadSemi,
                              ParseDeclOptions Options,
                              llvm::function_ref<void(Decl*)> handler);

include/swift/Parse/Token.h

@@ -124,6 +124,10 @@ class Token {
     return !isEllipsis();
   }
 
+  bool isTilde() const {
+    return isAnyOperator() && Text == "~";
+  }
+
   /// Determine whether this token occurred at the start of a line.
   bool isAtStartOfLine() const { return AtStartOfLine; }
 

lib/Parse/ParseDecl.cpp

@@ -5678,6 +5678,29 @@ ParserResult<ImportDecl> Parser::parseDeclImport(ParseDeclOptions Flags,
   return DCC.fixupParserResult(ID);
 }
 
+static void addMoveOnlyAttrIf(SourceLoc const &parsedTildeCopyable,
+                              ASTContext &Context,
+                              Decl *decl) {
+  if (parsedTildeCopyable.isInvalid())
+    return;
+
+  auto &attrs = decl->getAttrs();
+
+  // Don't add if it's already explicitly written on the decl, but error about
+  // the duplication and point to the `~Copyable`.
+  if (auto attr = attrs.getAttribute<MoveOnlyAttr>()) {
+    const bool sayModifier = false;
+    Context.Diags.diagnose(attr->getLocation(), diag::duplicate_attribute,
+                           sayModifier)
+        .fixItRemove(attr->getRange());
+    Context.Diags.diagnose(parsedTildeCopyable, diag::previous_attribute,
+                           sayModifier);
+    return;
+  }
+
+  attrs.add(new(Context) MoveOnlyAttr(/*IsImplicit=*/true));
+}
+
 /// Parse an inheritance clause.
 ///
 /// \verbatim
@@ -5687,15 +5710,19 @@ ParserResult<ImportDecl> Parser::parseDeclImport(ParseDeclOptions Flags,
 ///   inherited:
 ///     'class'
 ///     type-identifier
+///     '~' 'Copyable'
 /// \endverbatim
 ParserStatus Parser::parseInheritance(
     SmallVectorImpl<InheritedEntry> &Inherited,
-    bool allowClassRequirement, bool allowAnyObject) {
+    bool allowClassRequirement,
+    bool allowAnyObject,
+    SourceLoc *parseTildeCopyable) {
   consumeToken(tok::colon);
 
   SourceLoc classRequirementLoc;
 
   ParserStatus Status;
+  SourceLoc TildeCopyableLoc;
   SourceLoc prevComma;
   bool HasNextType;
   do {
@@ -5749,6 +5776,38 @@ ParserStatus Parser::parseInheritance(
       continue;
     }
 
+    // Is suppression permitted?
+    if (parseTildeCopyable) {
+      // Try to find '~' 'Copyable'
+      //
+      // We do this knowing that Copyable is not a real type as of now, so we
+      // can't rely on parseType.
+      if (Tok.isTilde()) {
+        const auto &nextTok = peekToken(); // lookahead
+        if (isIdentifier(nextTok, Context.Id_Copyable.str())) {
+          auto tildeLoc = consumeToken();
+          consumeToken(); // the 'Copyable' token
+
+          if (TildeCopyableLoc)
+            diagnose(tildeLoc, diag::already_suppressed, Context.Id_Copyable);
+          else
+            TildeCopyableLoc = tildeLoc;
+
+          continue;
+        }
+
+        // can't suppress whatever is between '~' and ',' or '{'.
+        diagnose(Tok, diag::only_suppress_copyable);
+        consumeToken();
+      }
+
+    } else if (Tok.isTilde()) {
+      // a suppression isn't allowed here, so emit an error eat the token to
+      // prevent further parsing errors.
+      diagnose(Tok, diag::cannot_suppress_here);
+      consumeToken();
+    }
+
     auto ParsedTypeResult = parseType();
     Status |= ParsedTypeResult;
 
@@ -5757,6 +5816,9 @@ ParserStatus Parser::parseInheritance(
       Inherited.push_back(InheritedEntry(ParsedTypeResult.get()));
   } while (HasNextType);
 
+  if (parseTildeCopyable)
+    *parseTildeCopyable = TildeCopyableLoc;
+
   return Status;
 }
 
@@ -8219,10 +8281,14 @@ ParserResult<EnumDecl> Parser::parseDeclEnum(ParseDeclOptions Flags,
   // Parse optional inheritance clause within the context of the enum.
   if (Tok.is(tok::colon)) {
     SmallVector<InheritedEntry, 2> Inherited;
+    SourceLoc parsedTildeCopyable;
     Status |= parseInheritance(Inherited,
                                /*allowClassRequirement=*/false,
-                               /*allowAnyObject=*/false);
+                               /*allowAnyObject=*/false,
+                               &parsedTildeCopyable);
     ED->setInherited(Context.AllocateCopy(Inherited));
+
+    addMoveOnlyAttrIf(parsedTildeCopyable, Context, ED);
   }
 
   diagnoseWhereClauseInGenericParamList(GenericParams);
@@ -8482,10 +8548,14 @@ ParserResult<StructDecl> Parser::parseDeclStruct(ParseDeclOptions Flags,
   // Parse optional inheritance clause within the context of the struct.
   if (Tok.is(tok::colon)) {
     SmallVector<InheritedEntry, 2> Inherited;
+    SourceLoc parsedTildeCopyable;
     Status |= parseInheritance(Inherited,
                                /*allowClassRequirement=*/false,
-                               /*allowAnyObject=*/false);
+                               /*allowAnyObject=*/false,
+                               &parsedTildeCopyable);
     SD->setInherited(Context.AllocateCopy(Inherited));
+
+    addMoveOnlyAttrIf(parsedTildeCopyable, Context, SD);
   }
 
   diagnoseWhereClauseInGenericParamList(GenericParams);

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -2754,11 +2754,15 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
   /// check to see if a move-only type can ever conform to the given type.
   /// \returns true iff a diagnostic was emitted because it was not compatible
   static bool diagnoseIncompatibleWithMoveOnlyType(SourceLoc loc,
-                                                 NominalTypeDecl *moveonlyType,
-                                                 Type type) {
+                                                   NominalTypeDecl *moveonlyType,
+                                                   Type type) {
     assert(type && "got an empty type?");
     assert(moveonlyType->isMoveOnly());
 
+    // no need to emit a diagnostic if the type itself is already problematic.
+    if (type->hasError())
+      return false;
+
     auto canType = type->getCanonicalType();
     if (auto prot = canType->getAs<ProtocolType>()) {
       // Permit conformance to marker protocol Sendable.

test/ModuleInterface/moveonly_interface_flag.swift

@@ -8,9 +8,15 @@
 // CHECK:       #if compiler(>=5.3) && $MoveOnly
 // CHECK-NEXT:    @_moveOnly public struct MoveOnlyStruct {
 
+// CHECK:       #if compiler(>=5.3) && $MoveOnly
+// CHECK-NEXT:    @_moveOnly public struct MoveOnlyStructSupp {
+
 // CHECK:      #if compiler(>=5.3) && $MoveOnly
 // CHECK-NEXT:   @_moveOnly public enum MoveOnlyEnum {
 
+// CHECK:      #if compiler(>=5.3) && $MoveOnly
+// CHECK-NEXT:   @_moveOnly public enum MoveOnlyEnumSupp {
+
 // CHECK:      #if compiler(>=5.3) && $MoveOnly
 // CHECK-NEXT:   public func someFn() -> Library.MoveOnlyEnum
 
@@ -25,10 +31,18 @@
   let x = 0
 }
 
+public struct MoveOnlyStructSupp: ~Copyable {
+  let x = 0
+}
+
 @_moveOnly public enum MoveOnlyEnum {
   case depth
 }
 
+public enum MoveOnlyEnumSupp: ~Copyable {
+  case depth
+}
+
 public func someFn() -> MoveOnlyEnum { return .depth }
 
 public class What {

test/Parse/without_copyable.swift

@@ -0,0 +1,47 @@
+// RUN: %target-typecheck-verify-swift
+
+protocol Sando { func make() } // expected-note 2{{protocol requires function 'make()'}}
+
+struct S: ~U, // expected-error {{can only suppress 'Copyable'}}
+              // expected-error@-1 {{inheritance from non-protocol type 'U'}}
+          ~Copyable {}
+
+struct U: // expected-error {{move-only struct 'U' cannot conform to 'Sando'}}
+          // expected-error@-1 {{type 'U' does not conform to protocol 'Sando'}}
+          ~Copyable,
+          Sando,
+          ~Copyable // expected-error {{duplicate suppression of 'Copyable'}}
+          {}
+
+
+// The expected behavior for '~' in the inheritance clause of a decl not supporting
+// suppression is to emit an error and then to treat it as if it's inheriting from
+// the type, rather than suppressing. That is, it treats it like the '~' wasn't there
+// after emitting an error.
+
+class C: // expected-error {{type 'C' does not conform to protocol 'Sando'}}
+         ~Copyable, // expected-error {{cannot suppress conformances here}}
+                    // expected-error@-1 {{cannot find type 'Copyable' in scope}}
+         ~Sando // expected-error {{cannot suppress conformances here}}
+         {}
+
+protocol Rope<Element>: ~Copyable { // expected-error {{cannot suppress conformances here}}
+                                    // expected-error@-1 {{cannot find type 'Copyable' in scope}}
+
+  associatedtype Element: ~Copyable // expected-error {{cannot suppress conformances here}}
+                                    // expected-error@-1 {{cannot find type 'Copyable' in scope}}
+}
+
+extension S: ~Copyable {} // expected-error {{cannot suppress conformances here}}
+                          // expected-error@-1 {{cannot find type 'Copyable' in scope}}
+
+func takeNoncopyableGeneric<T: ~Copyable>(_ t: T) {} // expected-error {{expected a class type or protocol-constrained type restricting 'T'}}
+
+@_moveOnly struct ExtraNonCopyable:         // expected-error {{duplicate attribute}}{{1-12=}}
+                                  ~Copyable // expected-note {{attribute already specified here}}
+                                  {}
+
+// basic test to ensure it's viewed as a noncopyable struct:
+struct HasADeinit: ~Copyable {
+  deinit {}
+}