[NCGenerics] allow feature-guarded uses of ~Copyable

We need to delay the pre-NCGenerics errors about
illegal uses of ~Copyable from Parse until Sema.
Otherwise, such uses can't appear within a false

Author: kavon

include/swift/AST/DiagnosticsParse.def

@@ -961,8 +961,8 @@ ERROR(cannot_suppress_here,none,
 ERROR(only_suppress_copyable,none,
       "can only suppress 'Copyable'", ())
 
-ERROR(already_suppressed,none,
-      "duplicate suppression of %0", (Identifier))
+ERROR(already_suppressed_copyable,none,
+      "duplicate suppression of 'Copyable'", ())
 
 //------------------------------------------------------------------------------
 // MARK: Pattern parsing diagnostics

include/swift/Parse/Parser.h

@@ -42,6 +42,7 @@ namespace llvm {
 
 namespace swift {
   class IdentTypeRepr;
+  class ErrorTypeRepr;
   class CodeCompletionCallbacks;
   class DoneParsingCallback;
   class IDEInspectionCallbacksFactory;
@@ -2035,11 +2036,6 @@ class Parser {
   void performIDEInspectionSecondPassImpl(
       IDEInspectionDelayedDeclState &info);
 
-  /// Returns true if the caller should skip calling `parseType` afterwards.
-  bool parseLegacyTildeCopyable(SourceLoc *parseTildeCopyable,
-                                ParserStatus &Status,
-                                SourceLoc &TildeCopyableLoc);
-
   //===--------------------------------------------------------------------===//
   // ASTGen support.
 

lib/Parse/ParseDecl.cpp

@@ -6426,51 +6426,6 @@ static void addMoveOnlyAttrIf(SourceLoc const &parsedTildeCopyable,
   attrs.add(new(Context) MoveOnlyAttr(/*IsImplicit=*/true));
 }
 
-bool Parser::parseLegacyTildeCopyable(SourceLoc *parseTildeCopyable,
-                                      ParserStatus &Status,
-                                      SourceLoc &TildeCopyableLoc) {
-  // 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;
-
-        return true;
-      } else if (nextTok.is(tok::code_complete)) {
-        consumeToken(); // consume '~'
-        Status.setHasCodeCompletionAndIsError();
-        if (CodeCompletionCallbacks) {
-          CodeCompletionCallbacks->completeWithoutConstraintType();
-        }
-        consumeToken(tok::code_complete);
-      }
-
-      // 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();
-  }
-
-  return false;
-}
-
 /// Parse an inheritance clause.
 ///
 /// \verbatim
@@ -6546,11 +6501,47 @@ ParserStatus Parser::parseInheritance(
       continue;
     }
 
-    if (!EnabledNoncopyableGenerics) {
-      if (parseLegacyTildeCopyable(parseTildeCopyable,
-                                   Status,
-                                   TildeCopyableLoc))
-        continue;
+    if (!EnabledNoncopyableGenerics && Tok.isTilde()) {
+      ErrorTypeRepr *error = nullptr;
+      if (parseTildeCopyable) {
+        const auto &nextTok = peekToken(); // lookahead
+        if (isIdentifier(nextTok, Context.Id_Copyable.str())) {
+          auto tildeLoc = consumeToken();
+          consumeToken(); // the 'Copyable' token
+
+          if (TildeCopyableLoc)
+            Inherited.push_back(InheritedEntry(
+                ErrorTypeRepr::create(Context, tildeLoc,
+                                      diag::already_suppressed_copyable)));
+          else
+            TildeCopyableLoc = tildeLoc;
+
+          continue; // success
+        }
+
+        if (nextTok.is(tok::code_complete)) {
+          consumeToken(); // consume '~'
+          Status.setHasCodeCompletionAndIsError();
+          if (CodeCompletionCallbacks) {
+            CodeCompletionCallbacks->completeWithoutConstraintType();
+          }
+          consumeToken(tok::code_complete);
+        }
+
+        // can't suppress whatever is between '~' and ',' or '{'.
+        error = ErrorTypeRepr::create(Context, consumeToken(),
+                                      diag::only_suppress_copyable);
+      } else {
+        // Otherwise, a suppression isn't allowed here unless noncopyable
+        // generics is enabled, so record a delayed error diagnostic and
+        // eat the token to prevent further parsing errors.
+        error = ErrorTypeRepr::create(Context, consumeToken(),
+                                      diag::cannot_suppress_here);
+      }
+
+      // Record the error parsing ~Copyable, but continue on to parseType.
+      if (error)
+        Inherited.push_back(InheritedEntry(error));
     }
 
     auto ParsedTypeResult = parseType();

lib/Parse/ParseType.cpp

@@ -172,9 +172,6 @@ ParserResult<TypeRepr> Parser::parseTypeSimple(
   SourceLoc tildeLoc;
   if (Tok.isTilde() && !isInSILMode()) {
     tildeLoc = consumeToken();
-    if (!EnabledNoncopyableGenerics)
-      diagnose(tildeLoc, diag::cannot_suppress_here)
-          .fixItRemoveChars(tildeLoc, tildeLoc);
   }
 
   switch (Tok.getKind()) {
@@ -310,9 +307,15 @@ ParserResult<TypeRepr> Parser::parseTypeSimple(
   }
 
   // Wrap in an InverseTypeRepr if needed.
-  if (EnabledNoncopyableGenerics && tildeLoc) {
-    ty = makeParserResult(ty,
-                          new(Context) InverseTypeRepr(tildeLoc, ty.get()));
+  if (tildeLoc) {
+    TypeRepr *repr;
+    if (EnabledNoncopyableGenerics)
+      repr = new (Context) InverseTypeRepr(tildeLoc, ty.get());
+    else
+      repr =
+          ErrorTypeRepr::create(Context, tildeLoc, diag::cannot_suppress_here);
+
+    ty = makeParserResult(ty, repr);
   }
 
   return ty;

test/Parse/inverses_legacy_ifdef.swift

@@ -0,0 +1,61 @@
+// RUN: %swift-frontend -typecheck %s
+// RUN: %swift-frontend -typecheck %s -DHAVE_NCGENERICS -enable-experimental-feature NoncopyableGenerics
+// RUN: %swift-frontend -typecheck %s -verify -DHAVE_NCGENERICS
+
+// REQUIRES: asserts
+
+/// This test checks that you can write ~Copyable in places that were illegal
+/// in Swift 5.9, as long as those illegal appearances are guarded within
+/// a disabled #if block.
+///
+/// We previously would emit errors during Parse for those illegal appearances,
+/// which would make it impossible to feature-guard new, legal uses of those
+/// annotations. Now we delay those diagnostics until Sema.
+///
+/// So, the first RUN line here checks that there are no error diagnostics emitted
+/// when not providing HAVE_NCGENERICS. Then, we enable that experimental feature to
+/// ensure that what's in the #if has no unexpected errors. Finally, we run without that
+/// feature to ensure we get the expected errors when the #if-block is allowed to be
+/// typechecked.
+
+struct NC: ~Copyable {}
+
+#if HAVE_NCGENERICS
+
+struct Blah: ~Copyable, ~Copyable {}
+// expected-error@-1 {{duplicate suppression of 'Copyable'}}
+
+protocol Compare where Self: ~Copyable { // expected-error {{cannot suppress conformances here}}
+  func lessThan(_ other: borrowing Self) -> Bool
+}
+
+protocol Sortable<Element>: ~Copyable { // expected-error {{cannot suppress conformances here}}
+  associatedtype Element: ~Copyable, Compare // expected-error {{cannot suppress conformances here}} // expected-note {{protocol requires nested type 'Element'; add nested type 'Element' for conformance}}
+
+  mutating func sort()
+}
+
+extension NC: Compare { // expected-error {{noncopyable struct 'NC' cannot conform to 'Compare'}}
+  func lessThan(_ other: borrowing Self) -> Bool { false }
+}
+
+extension NC: Sortable { // expected-error {{noncopyable struct 'NC' cannot conform to 'Sortable'}}
+                         // expected-error@-1 {{type 'NC' does not conform to protocol 'Sortable'}}
+  typealias Element = NC // expected-note {{possibly intended match 'NC.Element' (aka 'NC') does not conform to 'Copyable'}}
+
+  mutating func sort() { }
+}
+
+func f<T>(_ t: inout T) // expected-note {{where 'T.Element' = 'NC.Element'}}
+  where T: ~Copyable,  // expected-error {{cannot suppress conformances here}}
+        T: Sortable,
+        T.Element == NC {
+  t.sort()
+}
+
+do {
+  var x = NC()
+  f(&x) // expected-error {{global function 'f' requires the types 'NC.Element' and 'NC' be equivalent}}
+}
+
+#endif