prevent uses of Copyable and bogus errors when ~Copyable appears

Rather than try to omit explicit uses of `Copyable` in the output module
for backwards compatability, we instead make it an error to use `Copyable`
as a type when not suppressed. While it's straightforward to omit it for
generic parameters, for uses of `Copyable` as an existential, we'd need to
emit `Any` in its place. We'd also need to find and omit or modify
explicit type constraints or same-type requirements involving Copyable.

All of that sounded rather gross, and not really needed right now since
any uses of `Copyable` explicitly is just not needed.

Author: kavon

include/swift/AST/DiagnosticsSema.def

@@ -6898,8 +6898,11 @@ ERROR(noncopyable_objc_enum, none,
 ERROR(suppress_cannot_suppress,none,
       "cannot suppress implicit conformance to %0", (Type))
 
+ERROR(suppress_cannot_suppress_here,none,
+      "suppression of %0 conformance cannot appear here", (Type))
+
 ERROR(suppress_on_wrong_decl,none,
-      "cannot suppress conformance on %0", (DescriptiveDeclKind))
+      "cannot suppress conformance to %0 on %1", (Type, DescriptiveDeclKind))
 
 ERROR(suppress_duplicate,none,
       "implicit conformance to %0 already suppressed", (Type))
@@ -6909,6 +6912,14 @@ NOTE(suppress_previously_here,none,
 ERROR(suppress_not_protocol,none,
       "suppression of non-protocol type %0", (Type))
 
+ERROR(copyable_only_suppression,none,
+      "'Copyable' can only be suppressed via '~Copyable' at this time",
+      ())
+
+ERROR(suppression_illegal_here,none,
+      "suppression cannot appear here",
+      ())
+
 //------------------------------------------------------------------------------
 // MARK: Type inference from default expressions
 //------------------------------------------------------------------------------

include/swift/Parse/Parser.h

@@ -1200,7 +1200,6 @@ class Parser {
     PI_Default               = 0,
     PI_AllowClassRequirement = 1 << 1,
     PI_AllowAnyObject        = 1 << 2,
-    PI_AllowSuppression      = 1 << 3,
   };
 
   using ParseInheritanceOptions = OptionSet<ParseInheritanceFlags>;

lib/AST/RequirementMachine/RequirementLowering.cpp

@@ -726,7 +726,6 @@ void swift::rewriting::realizeInheritedRequirements(
     // handle suppressed requirements
     if (inheritedEntries[index].isSuppressed) {
       // TODO: we should be adding a requirement for Copyable if not suppressed.
-      assert(false && "unexpected suppressed entry");
       continue;
     }
 

lib/Parse/ParseDecl.cpp

@@ -5686,31 +5686,12 @@ ParserStatus Parser::parseInheritance(
       continue;
     }
 
-    // parse the "without" operator
-    SourceLoc withoutLoc;
-    if (Tok.isTilde()) {
-      auto loc = consumeToken();
-
-      if (!options.contains(PI_AllowSuppression))
-        diagnose(loc, diag::suppress_illegal_here);
-      else
-        withoutLoc = loc;
-    }
-
     auto ParsedTypeResult = parseType();
     Status |= ParsedTypeResult;
 
-    // If it's a single type, record the type under the appropriate entry.
-    if (ParsedTypeResult.isNonNull()) {
-      TypeRepr *typeRepr = ParsedTypeResult.get();
+    if (ParsedTypeResult.isNonNull())
+      Inherited.push_back(InheritedEntry(ParsedTypeResult.get()));
 
-      // is the parsed type meant to be a suppressed entry?
-      if (withoutLoc) {
-        typeRepr = new (Context) SuppressedTypeRepr(typeRepr, withoutLoc);
-      }
-
-      Inherited.push_back(InheritedEntry(typeRepr));
-    }
   } while (HasNextType);
 
   return Status;
@@ -8171,7 +8152,7 @@ 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;
-    Status |= parseInheritance(Inherited, {PI_AllowSuppression});
+    Status |= parseInheritance(Inherited, {});
     ED->setAllInheritedEntries(Context.AllocateCopy(Inherited));
   }
 
@@ -8432,7 +8413,7 @@ 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;
-    Status |= parseInheritance(Inherited, {PI_AllowSuppression});
+    Status |= parseInheritance(Inherited, {});
     SD->setAllInheritedEntries(Context.AllocateCopy(Inherited));
   }
 
@@ -8523,7 +8504,7 @@ ParserResult<ClassDecl> Parser::parseDeclClass(ParseDeclOptions Flags,
   // Parse optional inheritance clause within the context of the class.
   if (Tok.is(tok::colon)) {
     SmallVector<InheritedEntry, 2> Inherited;
-    Status |= parseInheritance(Inherited, {PI_AllowSuppression});
+    Status |= parseInheritance(Inherited, {});
     CD->setAllInheritedEntries(Context.AllocateCopy(Inherited));
 
   // Parse python style inheritance clause and replace parentheses with a colon

lib/Parse/ParseGeneric.cpp

@@ -99,7 +99,7 @@ Parser::parseGenericParametersBeforeWhere(SourceLoc LAngleLoc,
       ParserResult<TypeRepr> Ty;
 
       if (Tok.isAny(tok::identifier, tok::code_complete, tok::kw_protocol,
-                    tok::kw_Any)) {
+                    tok::kw_Any) || Tok.isTilde()) {
         Ty = parseType();
       } else if (Tok.is(tok::kw_class)) {
         diagnose(Tok, diag::unexpected_class_constraint);

lib/Parse/ParseType.cpp

@@ -417,6 +417,11 @@ ParserResult<TypeRepr> Parser::parseTypeScalar(
     return parseSILBoxType(generics, attrs);
   }
 
+  // parse the "without" operator
+  SourceLoc withoutLoc;
+  if (Tok.isTilde())
+    withoutLoc = consumeToken();
+
   ParserResult<TypeRepr> ty = parseTypeSimpleOrComposition(MessageID, reason);
   status |= ParserStatus(ty);
   if (ty.isNull())
@@ -566,6 +571,11 @@ ParserResult<TypeRepr> Parser::parseTypeScalar(
       tyR->walk(walker);
   }
 
+  // Wrap the type more tightly to "without" than other attributes.
+  // TODO: require parens around the composition if we saw a "without".
+  if (withoutLoc)
+    tyR = new(Context) SuppressedTypeRepr(tyR, withoutLoc);
+
   return makeParserResult(
       status,
       applyAttributeToType(tyR, attrs, specifier, specifierLoc, isolatedLoc,

lib/Sema/TypeCheckAccess.cpp

@@ -413,9 +413,9 @@ void AccessControlCheckerBase::checkGenericParamAccess(
       if (allInheritedEntries.empty())
         continue;
       assert(allInheritedEntries.size() == 1);
-      assert(allInheritedEntries.front().isSuppressed == false);
-      checkTypeAccessImpl(allInheritedEntries.front().getType(),
-                          allInheritedEntries.front().getTypeRepr(),
+      auto entry = allInheritedEntries.front();
+      checkTypeAccessImpl(entry.getType(),
+                          entry.getTypeRepr(),
                           accessScope, DC, /*mayBeInferred*/false,
                           callback);
     }
@@ -1923,9 +1923,8 @@ class DeclAvailabilityChecker : public DeclVisitor<DeclAvailabilityChecker> {
         if (allInheritedEntries.empty())
           continue;
         assert(allInheritedEntries.size() == 1);
-        auto inherited = allInheritedEntries.front();
-        assert(!inherited.isSuppressed);
-        checkType(inherited.getType(), inherited.getTypeRepr(), ownerDecl);
+        auto entry = allInheritedEntries.front();
+        checkType(entry.getType(), entry.getTypeRepr(), ownerDecl);
       }
     }
 

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -90,69 +90,71 @@ static Type containsParameterizedProtocolType(Type inheritedTy) {
 // implicit conformances.
 static void checkSuppressedEntries(
     llvm::PointerUnion<const TypeDecl *, const ExtensionDecl *> declUnion) {
-  if (declUnion.is<const ExtensionDecl*>())
-    return; // nothing to check; extensions don't carry suppressed conformances
-
   auto typeDecl = declUnion.dyn_cast<const TypeDecl *>();
-  assert(typeDecl);
+  auto extnDecl = declUnion.dyn_cast<const ExtensionDecl*>();
 
-#ifndef NDEBUG
-  if (isa<AssociatedTypeDecl>(typeDecl)
-      || isa<GenericTypeParamDecl>(typeDecl)) {
-    assert(typeDecl->getSuppressed().empty()
-               && "unexpected suppressed conformances");
-  }
-#endif
+  ASTContext &ctx =
+      typeDecl ? typeDecl->getASTContext() : extnDecl->getASTContext();
+
+  ArrayRef<InheritedEntry> allEntries =
+      typeDecl ? typeDecl->getAllInheritedEntries()
+               : extnDecl->getAllInheritedEntries();
 
-  ASTContext &ctx = typeDecl->getASTContext();
   auto &diags = ctx.Diags;
+  bool haveMoveOnlyClasses = ctx.LangOpts.hasFeature(Feature::MoveOnlyClasses);
 
   using EntryTracker = std::unordered_map<KnownProtocolKind,
                                           InheritedEntry const *>;
   EntryTracker seenEntries;
 
-  bool haveMoveOnlyClasses =
-      typeDecl->getASTContext().LangOpts.hasFeature(Feature::MoveOnlyClasses);
-
   // TODO: this pattern of iterating over the indicies of all entries is
   //  an all too common pattern throughout the type-checker. It's a symptom of
   //  an overdue refactoring that allows you to iterate over the entries and
   //  either automatically perform these type resolution requests, or know
   //  the requests have already been performed very early on. It's the primary
   //  thing preventing us from using the InheritedEntryRange accessors.
-  ArrayRef<InheritedEntry> allEntries = typeDecl->getAllInheritedEntries();
   for (auto i : indices(allEntries)) {
     auto &suppressed = allEntries[i];
 
     if (!suppressed.isSuppressed)
       continue;
 
     // Resolve and validate the type if needed.
-    InheritedTypeRequest request{typeDecl, i, TypeResolutionStage::Interface};
+    InheritedTypeRequest request{declUnion, i, TypeResolutionStage::Interface};
     Type suppressedTy = evaluateOrDefault(ctx.evaluator, request, Type());
 
     // If we couldn't resolve the type or it contains an error, ignore it.
     if (!suppressedTy || suppressedTy->hasError())
       continue;
 
     // only structs, enums, and some classes can have suppressed entries
-    if (!(isa<StructDecl>(typeDecl) || isa<EnumDecl>(typeDecl)
-        || (isa<ClassDecl>(typeDecl) && haveMoveOnlyClasses))) {
+    if (typeDecl) {
+      if (!(isa<StructDecl>(typeDecl) || isa<EnumDecl>(typeDecl)
+          || (isa<ClassDecl>(typeDecl) && haveMoveOnlyClasses))) {
+        diags.diagnose(suppressed.getLoc(),
+                       diag::suppress_on_wrong_decl,
+                       suppressedTy,
+                       typeDecl->getDescriptiveKind());
+        // FIXME: a great fix-it would be to remove the entire entry,
+        //  including any trailing comma. See checkInheritedEntry
+        //  for an example of the logic to do that. You'll need to update it
+        //  using MergedArrayRef so it understands source locations for both
+        //  suppressed and inherited entries.
+        continue;
+      }
+    } else {
+      assert(extnDecl);
       diags.diagnose(suppressed.getLoc(),
                      diag::suppress_on_wrong_decl,
-                     typeDecl->getDescriptiveKind());
-      // FIXME: a great fix-it would be to remove the entire entry,
-      //  including any trailing comma. See checkInheritedEntry
-      //  for an example of the logic to do that. You'll need to update it
-      //  using MergedArrayRef so it understands source locations for both
-      //  suppressed and inherited entries.
+                     suppressedTy,
+                     extnDecl->getDescriptiveKind());
       continue;
     }
 
     // Ensure it is a suppressible protocol.
-    if (auto protocolTy = dyn_cast<ProtocolType>(suppressedTy))
-      if (auto protocolDecl = protocolTy->getDecl())
-        if (auto knownProtocol = protocolDecl->getKnownProtocolKind())
+    if (auto protocolTy = dyn_cast<ProtocolType>(suppressedTy)) {
+      if (auto protocolDecl = protocolTy->getDecl()) {
+        if (auto knownProtocol = protocolDecl->getKnownProtocolKind()) {
           if (isSuppressibleProtocol(*knownProtocol)) {
             // remember that we've seen this suppression for the type
             EntryTracker::iterator iter;
@@ -178,20 +180,16 @@ static void checkSuppressedEntries(
 
             continue;
           }
-
-    // At this point, the suppressed type is wrong. Tailor the error messages.
-
-    if (suppressedTy->isConstraintType()) {
-      diags.diagnose(suppressed.getLoc(),
-                     diag::suppress_cannot_suppress,
-                     suppressedTy);
-      // FIXME: a great fix-it would be to remove the entire entry.
-    } else {
-      diags.diagnose(suppressed.getLoc(),
-                     diag::suppress_not_protocol,
-                     suppressedTy);
-      // FIXME: a great fix-it would be to remove the entire entry.
+        }
+      }
     }
+
+    // At this point, the suppressed type is wrong.
+    assert(suppressedTy->isConstraintType());
+    diags.diagnose(suppressed.getLoc(),
+                   diag::suppress_cannot_suppress,
+                   suppressedTy);
+    // FIXME: a great fix-it would be to remove the entire entry.
   }
 }
 
@@ -2912,7 +2910,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
           return;
 
         // go over the all types directly conformed-to by the extension
-        for (auto entry : extension->getAllInheritedEntries()) {
+        for (auto entry : extension->getInherited()) {
           diagnoseIncompatibleWithMoveOnlyType(extension->getLoc(), nomDecl,
                                                entry.getType());
         }

lib/Sema/TypeCheckType.cpp

@@ -2085,6 +2085,8 @@ namespace {
                                           TypeResolutionOptions options);
     NeverNullType resolveCompileTimeConstTypeRepr(CompileTimeConstTypeRepr *repr,
                                                   TypeResolutionOptions options);
+    NeverNullType resolveSuppressedTypeRepr(SuppressedTypeRepr *repr,
+                                            TypeResolutionOptions options);
     NeverNullType resolveArrayType(ArrayTypeRepr *repr,
                                    TypeResolutionOptions options);
     NeverNullType resolveDictionaryType(DictionaryTypeRepr *repr,
@@ -2374,9 +2376,7 @@ NeverNullType TypeResolver::resolveType(TypeRepr *repr,
       return resolveCompileTimeConstTypeRepr(cast<CompileTimeConstTypeRepr>(repr),
                                              options);
   case TypeReprKind::Suppressed:
-      // Currently, parsing and other type checking code prevents invalid
-      // suppressed types from being formed, so just resolve the base.
-      return resolveType(cast<SuppressedTypeRepr>(repr)->getBase(), options);
+    return resolveSuppressedTypeRepr(cast<SuppressedTypeRepr>(repr), options);
 
   case TypeReprKind::SimpleIdent:
   case TypeReprKind::GenericIdent:
@@ -3391,7 +3391,11 @@ TypeResolver::resolveASTFunctionTypeParams(TupleTypeRepr *inputRepr,
           nestedRepr = specifierRepr->getBase();
           continue;
         case TypeReprKind::Suppressed:
-          llvm_unreachable("unexpected suppressed type for function param");
+          // Can't have a suppressed type as a function param.
+          diagnose(specifierRepr->getLoc(), diag::suppression_illegal_here);
+          specifierRepr->setInvalid();
+          nestedRepr = specifierRepr->getBase();
+          break;
         default:
           break;
         }
@@ -4177,6 +4181,19 @@ TypeResolver::resolveDeclRefTypeRepr(DeclRefTypeRepr *repr,
     return ErrorType::get(getASTContext());
   }
 
+  // Do not allow unsuppressed uses of Copyable anywhere.
+  if (!options.contains(TypeResolutionFlags::IsSuppressed)) {
+    if (auto protoTy = result->getAs<ProtocolType>()) {
+      if (auto protoDecl = protoTy->getDecl()) {
+        if (protoDecl->isSpecificProtocol(KnownProtocolKind::Copyable)) {
+          diagnose(repr->getLoc(), diag::copyable_only_suppression);
+          repr->setInvalid();
+          return ErrorType::get(getASTContext());
+        }
+      }
+    }
+  }
+
     auto *dc = getDeclContext();
     auto &ctx = getASTContext();
 
@@ -4321,6 +4338,39 @@ TypeResolver::resolveCompileTimeConstTypeRepr(CompileTimeConstTypeRepr *repr,
   return resolveType(repr->getBase(), options);
 }
 
+NeverNullType
+TypeResolver::resolveSuppressedTypeRepr(SuppressedTypeRepr *repr,
+                                        TypeResolutionOptions options) {
+  auto baseType = resolveType(repr->getBase(),
+                              options | TypeResolutionFlags::IsSuppressed);
+  if (baseType->hasError())
+    return baseType;
+
+  // Can't suppress outside of an inheritance clause.
+  if (!options.isInheritanceClause()) {
+    if (baseType->isConstraintType()) {
+      // Give a more detailed message if they tried to suppress a constraint
+      // in an illegal spot. This specifically excludes existentials because
+      // we don't want to complain about an 'any Equatable' as that's not a
+      // constraint.
+      diagnose(repr->getLoc(), diag::suppress_cannot_suppress_here, baseType);
+    } else {
+      diagnose(repr->getLoc(), diag::suppression_illegal_here);
+    }
+    repr->setInvalid();
+    return ErrorType::get(getASTContext());
+  }
+
+  // Can't suppress a non-constraint type in an inheritance clause.
+  if (!baseType->isConstraintType()) {
+    diagnose(repr->getLoc(), diag::suppress_not_protocol, baseType);
+    repr->setInvalid();
+    return ErrorType::get(getASTContext());
+  }
+
+  return baseType;
+}
+
 NeverNullType TypeResolver::resolveArrayType(ArrayTypeRepr *repr,
                                              TypeResolutionOptions options) {
   auto baseTy = resolveType(repr->getBase(), options.withoutContext());