Merge pull request #69821 from kavon/ncgenerics-feature-guard-parsing

[NCGenerics] allow feature-guarded uses of ~Copyable

Author: kavon

include/swift/AST/DiagnosticEngine.h

@@ -509,6 +509,9 @@ namespace swift {
     void addChildNote(Diagnostic &&D);
     void insertChildNote(unsigned beforeIndex, Diagnostic &&D);
   };
+
+  /// A diagnostic that has no input arguments, so it is trivially-destructable.
+  using ZeroArgDiagnostic = Diag<>;
 
   /// Describes an in-flight diagnostic, which is currently active
   /// within the diagnostic engine and can be augmented within additional

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/AST/TypeRepr.h

@@ -19,6 +19,7 @@
 
 #include "swift/AST/Attr.h"
 #include "swift/AST/DeclContext.h"
+#include "swift/AST/DiagnosticEngine.h"
 #include "swift/AST/GenericSignature.h"
 #include "swift/AST/Identifier.h"
 #include "swift/AST/Type.h"
@@ -195,17 +196,35 @@ class alignas(1 << TypeReprAlignInBits) TypeRepr
 /// A TypeRepr for a type with a syntax error.  Can be used both as a
 /// top-level TypeRepr and as a part of other TypeRepr.
 ///
-/// The client should make sure to emit a diagnostic at the construction time
-/// (in the parser).  All uses of this type should be ignored and not
-/// re-diagnosed.
+/// The client can either emit a detailed diagnostic at the construction time
+/// (in the parser) or store a zero-arg diagnostic in this TypeRepr to be
+/// emitted after parsing, during type resolution.
+///
+/// All uses of this type should be ignored and not re-diagnosed.
 class ErrorTypeRepr : public TypeRepr {
   SourceRange Range;
+  llvm::Optional<ZeroArgDiagnostic> DelayedDiag;
+
+  ErrorTypeRepr(SourceRange Range, llvm::Optional<ZeroArgDiagnostic> Diag)
+      : TypeRepr(TypeReprKind::Error), Range(Range), DelayedDiag(Diag) {}
 
 public:
-  ErrorTypeRepr() : TypeRepr(TypeReprKind::Error) {}
-  ErrorTypeRepr(SourceLoc Loc) : TypeRepr(TypeReprKind::Error), Range(Loc) {}
-  ErrorTypeRepr(SourceRange Range)
-      : TypeRepr(TypeReprKind::Error), Range(Range) {}
+  static ErrorTypeRepr *
+  create(ASTContext &Context, SourceRange Range,
+         llvm::Optional<ZeroArgDiagnostic> DelayedDiag = llvm::None) {
+    assert((!DelayedDiag || Range) && "diagnostic needs a location");
+    return new (Context) ErrorTypeRepr(Range, DelayedDiag);
+  }
+
+  static ErrorTypeRepr *
+  create(ASTContext &Context, SourceLoc Loc = SourceLoc(),
+         llvm::Optional<ZeroArgDiagnostic> DelayedDiag = llvm::None) {
+    return create(Context, SourceRange(Loc), DelayedDiag);
+  }
+
+  /// If there is a delayed diagnostic stored in this TypeRepr, consumes and
+  /// emits that diagnostic.
+  void dischargeDiagnostic(ASTContext &Context);
 
   static bool classof(const TypeRepr *T) {
     return T->getKind() == TypeReprKind::Error;

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/AST/DiagnosticEngine.cpp

@@ -42,6 +42,9 @@
 
 using namespace swift;
 
+static_assert(IsTriviallyDestructible<ZeroArgDiagnostic>::value,
+              "ZeroArgDiagnostic is meant to be trivially destructable");
+
 namespace {
 enum class DiagnosticOptions {
   /// No options.

lib/AST/TypeRepr.cpp

@@ -653,6 +653,15 @@ void SILBoxTypeRepr::printImpl(ASTPrinter &Printer,
   Printer.printKeyword("sil_box", Opts);
 }
 
+void ErrorTypeRepr::dischargeDiagnostic(swift::ASTContext &Context) {
+  if (!DelayedDiag)
+    return;
+
+  // Consume and emit the diagnostic.
+  Context.Diags.diagnose(Range.Start, *DelayedDiag).highlight(Range);
+  DelayedDiag = llvm::None;
+}
+
 // See swift/Basic/Statistic.h for declaration: this enables tracing
 // TypeReprs, is defined here to avoid too much layering violation / circular
 // linkage dependency.

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();
@@ -9665,7 +9656,7 @@ Parser::parseDeclSubscript(SourceLoc StaticLoc,
 
     if (ElementTy.isNull()) {
       // Always set an element type.
-      ElementTy = makeParserResult(ElementTy, new (Context) ErrorTypeRepr());
+      ElementTy = makeParserResult(ElementTy, ErrorTypeRepr::create(Context));
     }
   }
 

lib/Parse/ParseGeneric.cpp

@@ -339,7 +339,7 @@ ParserStatus Parser::parseGenericWhereClause(
         ParserResult<TypeRepr> Protocol = parseType();
         Status |= Protocol;
         if (Protocol.isNull())
-          Protocol = makeParserResult(new (Context) ErrorTypeRepr(PreviousLoc));
+          Protocol = makeParserResult(ErrorTypeRepr::create(Context, PreviousLoc));
 
         // Add the requirement.
         Requirements.push_back(RequirementRepr::getTypeConstraint(
@@ -358,7 +358,7 @@ ParserStatus Parser::parseGenericWhereClause(
       ParserResult<TypeRepr> SecondType = parseType();
       Status |= SecondType;
       if (SecondType.isNull())
-        SecondType = makeParserResult(new (Context) ErrorTypeRepr(PreviousLoc));
+        SecondType = makeParserResult(ErrorTypeRepr::create(Context, PreviousLoc));
 
       // Add the requirement
       if (FirstType.hasCodeCompletion()) {
@@ -373,7 +373,7 @@ ParserStatus Parser::parseGenericWhereClause(
         // completion token in the TypeRepr.
         Requirements.push_back(RequirementRepr::getTypeConstraint(
             FirstType.get(), EqualLoc,
-            new (Context) ErrorTypeRepr(SecondType.get()->getLoc()),
+            ErrorTypeRepr::create(Context, SecondType.get()->getLoc()),
             isRequirementExpansion));
       } else {
         Requirements.push_back(RequirementRepr::getSameType(
@@ -383,7 +383,7 @@ ParserStatus Parser::parseGenericWhereClause(
     } else if (FirstType.hasCodeCompletion()) {
       // Recover by adding dummy constraint.
       Requirements.push_back(RequirementRepr::getTypeConstraint(
-          FirstType.get(), PreviousLoc, new (Context) ErrorTypeRepr(PreviousLoc),
+          FirstType.get(), PreviousLoc, ErrorTypeRepr::create(Context, PreviousLoc),
           isRequirementExpansion));
     } else {
       diagnose(Tok, diag::expected_requirement_delim);

lib/Parse/ParsePattern.cpp

@@ -1053,7 +1053,7 @@ ParserResult<Pattern> Parser::parseTypedPattern() {
         }
       }
     } else {
-      Ty = makeParserResult(new (Context) ErrorTypeRepr(PreviousLoc));
+      Ty = makeParserResult(ErrorTypeRepr::create(Context, PreviousLoc));
     }
 
     result = makeParserResult(result,
@@ -1278,7 +1278,7 @@ parseOptionalPatternTypeAnnotation(ParserResult<Pattern> result) {
 
   TypeRepr *repr = Ty.getPtrOrNull();
   if (!repr)
-    repr = new (Context) ErrorTypeRepr(PreviousLoc);
+    repr = ErrorTypeRepr::create(Context, PreviousLoc);
 
   return makeParserResult(status, new (Context) TypedPattern(P, repr));
 }

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()) {
@@ -232,7 +229,7 @@ ParserResult<TypeRepr> Parser::parseTypeSimple(
       CodeCompletionCallbacks->completeTypeSimpleBeginning();
     }
     return makeParserCodeCompletionResult<TypeRepr>(
-        new (Context) ErrorTypeRepr(consumeToken(tok::code_complete)));
+        ErrorTypeRepr::create(Context, consumeToken(tok::code_complete)));
   case tok::l_square: {
     ty = parseTypeCollection();
     break;
@@ -255,7 +252,7 @@ ParserResult<TypeRepr> Parser::parseTypeSimple(
         diag.fixItInsert(getEndOfPreviousLoc(), " <#type#>");
     }
     if (Tok.isKeyword() && !Tok.isAtStartOfLine()) {
-      ty = makeParserErrorResult(new (Context) ErrorTypeRepr(Tok.getLoc()));
+      ty = makeParserErrorResult(ErrorTypeRepr::create(Context, Tok.getLoc()));
       consumeToken();
       return ty;
     }
@@ -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;
@@ -685,8 +688,8 @@ ParserResult<TypeRepr> Parser::parseDeclResultType(Diag<> MessageID) {
       auto diag = diagnose(Tok, diag::extra_rbracket);
       diag.fixItInsert(result.get()->getStartLoc(), getTokenText(tok::l_square));
       consumeToken();
-      return makeParserErrorResult(new (Context)
-                                       ErrorTypeRepr(getTypeErrorLoc()));
+      return makeParserErrorResult(ErrorTypeRepr::create(Context,
+                                                         getTypeErrorLoc()));
     }
 
     if (Tok.is(tok::colon)) {
@@ -702,8 +705,8 @@ ParserResult<TypeRepr> Parser::parseDeclResultType(Diag<> MessageID) {
           diag.fixItInsertAfter(secondType.get()->getEndLoc(), getTokenText(tok::r_square));
         }
       }
-      return makeParserErrorResult(new (Context)
-                                       ErrorTypeRepr(getTypeErrorLoc()));
+      return makeParserErrorResult(ErrorTypeRepr::create(Context,
+                                                         getTypeErrorLoc()));
     }
   }
   return result;