Merge pull request #41693 from hborla/any-in-diagnositcs

[Diagnostics] Print `any` in diagnostics.

Author: hborla

include/swift/AST/DiagnosticsSema.def

@@ -142,7 +142,7 @@ ERROR(could_not_use_instance_member_on_type,none,
       "%select{| instance of nested}3 type %0",
       (Type, DeclNameRef, Type, bool))
 ERROR(could_not_use_member_on_existential,none,
-      "member %1 cannot be used on value of protocol type %0; consider using a"
+      "member %1 cannot be used on value of type %0; consider using a"
       " generic constraint instead",
       (Type, DeclNameRef))
 FIXIT(replace_with_type,"%0",(Type))
@@ -1945,9 +1945,8 @@ ERROR(type_cannot_conform_to_nsobject,none,
 ERROR(use_of_equal_instead_of_equality,none,
       "use of '=' in a boolean context, did you mean '=='?", ())
 ERROR(type_cannot_conform, none,
-      "%select{type %1|protocol %1 as a type}0 cannot conform to "
-      "%select{%3|the protocol itself}2",
-      (bool, Type, bool, Type))
+      "type %0 cannot conform to %1",
+      (Type, Type))
 NOTE(only_concrete_types_conform_to_protocols,none,
      "only concrete types such as structs, enums and classes can conform to protocols",
      ())
@@ -2455,8 +2454,8 @@ ERROR(protocol_composition_one_class,none,
       "contains class %1", (Type, Type))
 
 ERROR(requires_conformance_nonprotocol,none,
-      "type %0 constrained to non-protocol, non-class type '%1'",
-      (Type, StringRef))
+      "type %0 constrained to non-protocol, non-class type %1",
+      (Type, Type))
 NOTE(requires_conformance_nonprotocol_fixit,none,
      "use '%0 == %1' to require '%0' to be '%1'",
      (StringRef, StringRef))
@@ -2777,9 +2776,9 @@ WARNING(anyobject_class_inheritance_deprecated,Deprecation,
 ERROR(multiple_inheritance,none,
   "multiple inheritance from classes %0 and %1", (Type, Type))
 ERROR(inheritance_from_non_protocol_or_class,none,
-  "inheritance from non-protocol, non-class type '%0'", (StringRef))
+  "inheritance from non-protocol, non-class type %0", (Type))
 ERROR(inheritance_from_non_protocol,none,
-  "inheritance from non-protocol type '%0'", (StringRef))
+  "inheritance from non-protocol type %0", (Type))
 ERROR(inheritance_from_anyobject,none,
   "only protocols can inherit from 'AnyObject'", ())
 ERROR(inheritance_from_parameterized_protocol,none,
@@ -3589,7 +3588,7 @@ ERROR(construct_protocol_value,none,
       "value of type %0 is a protocol; it cannot be instantiated",
       (Type))
 ERROR(construct_protocol_by_name,none,
-      "protocol type %0 cannot be instantiated",
+      "type %0 cannot be instantiated",
       (Type))
 
 // Operators

include/swift/AST/PrintOptions.h

@@ -548,6 +548,13 @@ struct PrintOptions {
     return result;
   }
 
+  /// The print options used for formatting diagnostic arguments.
+  static PrintOptions forDiagnosticArguments() {
+    PrintOptions result;
+    result.PrintExplicitAny = true;
+    return result;
+  }
+
   /// Retrieve the set of options suitable for diagnostics printing.
   static PrintOptions printForDiagnostics(AccessLevel accessFilter,
                                           bool printFullConvention) {

include/swift/AST/Types.h

@@ -5330,7 +5330,7 @@ class ExistentialType final : public TypeBase {
         ConstraintType(constraintType) {}
 
 public:
-  static Type get(Type constraint);
+  static Type get(Type constraint, bool forceExistential = false);
 
   Type getConstraintType() const { return ConstraintType; }
 
@@ -6663,9 +6663,9 @@ inline bool TypeBase::hasSimpleTypeRepr() const {
     return false;
 
   case TypeKind::Metatype:
-  case TypeKind::ExistentialMetatype:
     return !cast<const AnyMetatypeType>(this)->hasRepresentation();
 
+  case TypeKind::ExistentialMetatype:
   case TypeKind::Existential:
     return false;
 

lib/AST/ASTContext.cpp

@@ -4260,15 +4260,17 @@ ProtocolType::ProtocolType(ProtocolDecl *TheDecl, Type Parent,
                            RecursiveTypeProperties properties)
   : NominalType(TypeKind::Protocol, &Ctx, TheDecl, Parent, properties) { }
 
-Type ExistentialType::get(Type constraint) {
+Type ExistentialType::get(Type constraint, bool forceExistential) {
   auto &C = constraint->getASTContext();
-  // FIXME: Any and AnyObject don't yet use ExistentialType.
-  if (constraint->isAny() || constraint->isAnyObject())
-    return constraint;
+  if (!forceExistential) {
+    // FIXME: Any and AnyObject don't yet use ExistentialType.
+    if (constraint->isAny() || constraint->isAnyObject())
+      return constraint;
 
-  // ExistentialMetatypeType is already an existential type.
-  if (constraint->is<ExistentialMetatypeType>())
-    return constraint;
+    // ExistentialMetatypeType is already an existential type.
+    if (constraint->is<ExistentialMetatypeType>())
+      return constraint;
+  }
 
   assert(constraint->isConstraintType());
 

lib/AST/ASTPrinter.cpp

@@ -5171,6 +5171,9 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
     } else if (auto existential = dyn_cast<ExistentialType>(T.getPointer())) {
       if (!Options.PrintExplicitAny)
         return isSimpleUnderPrintOptions(existential->getConstraintType());
+    } else if (auto existential = dyn_cast<ExistentialMetatypeType>(T.getPointer())) {
+      if (!Options.PrintExplicitAny)
+        return isSimpleUnderPrintOptions(existential->getInstanceType());
     }
     return T->hasSimpleTypeRepr();
   }
@@ -5591,10 +5594,32 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
       }
     }
 
-    if (T->is<ExistentialMetatypeType>() && Options.PrintExplicitAny)
-      Printer << "any ";
+    Type instanceType = T->getInstanceType();
+    if (Options.PrintExplicitAny) {
+      if (T->is<ExistentialMetatypeType>()) {
+        Printer << "any ";
+
+        // FIXME: We need to replace nested existential metatypes so that
+        // we don't print duplicate 'any'. This will be unnecessary once
+        // ExistentialMetatypeType is split into ExistentialType(MetatypeType).
+        instanceType = Type(instanceType).transform([](Type type) -> Type {
+          if (auto existential = type->getAs<ExistentialMetatypeType>())
+            return MetatypeType::get(existential->getInstanceType());
+
+          return type;
+        });
+      } else if (instanceType->isAny() || instanceType->isAnyObject()) {
+        // FIXME: 'any' is needed to distinguish between '(any Any).Type'
+        // and 'any Any.Type'. However, this combined with the above hack
+        // to replace nested existential metatypes with metatypes causes
+        // a bug in printing nested existential metatypes for Any and AnyObject,
+        // e.g. 'any (any Any).Type.Type'. This will be fixed by using
+        // ExistentialType for Any and AnyObject.
+        instanceType = ExistentialType::get(instanceType, /*forceExistential=*/true);
+      }
+    }
 
-    printWithParensIfNotSimple(T->getInstanceType());
+    printWithParensIfNotSimple(instanceType);
 
     // We spell normal metatypes of existential types as .Protocol.
     if (isa<MetatypeType>(T) &&
@@ -6294,7 +6319,11 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
       if (printNamedOpaque())
         return;
 
-      visit(T->getExistentialType());
+      auto constraint = T->getExistentialType();
+      if (auto existential = constraint->getAs<ExistentialType>())
+        constraint = existential->getConstraintType();
+
+      visit(constraint);
       return;
     }
     case PrintOptions::OpaqueReturnTypePrintingMode::StableReference: {

lib/AST/DiagnosticEngine.cpp

@@ -670,10 +670,8 @@ static void formatDiagnosticArgument(StringRef Modifier,
     Type type;
     bool needsQualification = false;
 
-    // TODO: We should use PrintOptions::printForDiagnostic here, or we should
-    // rename that method.
-    PrintOptions printOptions{};
-
+    // Compute the appropriate print options for this argument.
+    auto printOptions = PrintOptions::forDiagnosticArguments();
     if (Arg.getKind() == DiagnosticArgumentKind::Type) {
       type = Arg.getAsType()->getWithoutParens();
       if (type.isNull()) {

lib/AST/GenericSignatureBuilder.cpp

@@ -6307,16 +6307,8 @@ GenericSignatureBuilder::finalize(TypeArrayView<GenericTypeParamType> genericPar
       auto source = constraint.source;
       auto loc = source->getLoc();
 
-      // FIXME: The constraint string is printed directly here because
-      // the current default is to not print `any` for existential
-      // types, but this error message is super confusing without `any`
-      // if the user wrote it explicitly.
-      PrintOptions options;
-      options.PrintExplicitAny = true;
-      auto constraintString = constraintType.getString(options);
-
       Diags.diagnose(loc, diag::requires_conformance_nonprotocol,
-                     subjectType, constraintString);
+                     subjectType, constraintType);
 
       auto getNameWithoutSelf = [&](std::string subjectTypeName) {
         std::string selfSubstring = "Self.";
@@ -6331,10 +6323,12 @@ GenericSignatureBuilder::finalize(TypeArrayView<GenericTypeParamType> genericPar
       if (allowConcreteGenericParams ||
           (subjectType->is<DependentMemberType>() &&
            !source->isProtocolRequirement())) {
-        auto subjectTypeName = subjectType.getString();
+        auto options = PrintOptions::forDiagnosticArguments();
+        auto subjectTypeName = subjectType.getString(options);
         auto subjectTypeNameWithoutSelf = getNameWithoutSelf(subjectTypeName);
         Diags.diagnose(loc, diag::requires_conformance_nonprotocol_fixit,
-                       subjectTypeNameWithoutSelf, constraintString)
+                       subjectTypeNameWithoutSelf,
+                       constraintType.getString(options))
              .fixItReplace(loc, " == ");
       }
     }

lib/AST/RequirementMachine/RequirementLowering.cpp

@@ -547,16 +547,8 @@ bool swift::rewriting::diagnoseRequirementErrors(
       if (subjectType->hasError() || constraint->hasError())
         break;
 
-      // FIXME: The constraint string is printed directly here because
-      // the current default is to not print `any` for existential
-      // types, but this error message is super confusing without `any`
-      // if the user wrote it explicitly.
-      PrintOptions options;
-      options.PrintExplicitAny = true;
-      auto constraintString = constraint.getString(options);
-
       ctx.Diags.diagnose(loc, diag::requires_conformance_nonprotocol,
-                         subjectType, constraintString);
+                         subjectType, constraint);
       diagnosedError = true;
 
       auto getNameWithoutSelf = [&](std::string subjectTypeName) {
@@ -570,10 +562,12 @@ bool swift::rewriting::diagnoseRequirementErrors(
       };
 
       if (allowConcreteGenericParams) {
-        auto subjectTypeName = subjectType.getString();
+        auto options = PrintOptions::forDiagnosticArguments();
+        auto subjectTypeName = subjectType.getString(options);
         auto subjectTypeNameWithoutSelf = getNameWithoutSelf(subjectTypeName);
         ctx.Diags.diagnose(loc, diag::requires_conformance_nonprotocol_fixit,
-                           subjectTypeNameWithoutSelf, constraintString)
+                           subjectTypeNameWithoutSelf,
+                           constraint.getString(options))
              .fixItReplace(loc, " == ");
       }
 

lib/Sema/CSDiagnostics.cpp

@@ -568,10 +568,7 @@ bool MissingConformanceFailure::diagnoseTypeCannotConform(
     constraintType = existential->getConstraintType();
 
   emitDiagnostic(diag::type_cannot_conform,
-                 nonConformingType->isExistentialType(), 
-                 nonConformingType, 
-                 constraintType->isEqual(protocolType),
-                 protocolType);
+                 nonConformingType, protocolType);
 
   bool emittedSpecializedNote = false;
   if (auto protoType = protocolType->getAs<ProtocolType>()) {
@@ -2369,9 +2366,7 @@ bool ContextualFailure::diagnoseAsError() {
         if (auto existential = constraintType->getAs<ExistentialType>())
           constraintType = existential->getConstraintType();
 
-        emitDiagnostic(diag::type_cannot_conform,
-                       /*isExistentialType=*/true, fromType, 
-                       constraintType->isEqual(toType), toType);
+        emitDiagnostic(diag::type_cannot_conform, fromType, toType);
         emitDiagnostic(diag::only_concrete_types_conform_to_protocols);
         return true;
       }
@@ -3123,7 +3118,7 @@ bool ContextualFailure::tryProtocolConformanceFixIt(
   // Emit a diagnostic to inform the user that they need to conform to the
   // missing protocols.
   auto conformanceDiag =
-      emitDiagnostic(diag::assign_protocol_conformance_fix_it, unwrappedToType,
+      emitDiagnostic(diag::assign_protocol_conformance_fix_it, constraint,
                      nominal->getDescriptiveKind(), fromType);
   if (nominal->getInherited().size() > 0) {
     auto lastInherited = nominal->getInherited().back().getLoc();
@@ -3231,7 +3226,7 @@ bool ContextualFailure::isIntegerToStringIndexConversion() const {
 Optional<Diag<Type, Type>>
 ContextualFailure::getDiagnosticFor(ContextualTypePurpose context,
                                     Type contextualType) {
-  auto forProtocol = contextualType->isExistentialType();
+  auto forProtocol = contextualType->isConstraintType();
   switch (context) {
   case CTP_Initialization: {
     if (contextualType->isAnyObject())

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -335,19 +335,11 @@ static void checkInheritanceClause(
       }
     }
 
-    // FIXME: The inherited type is printed directly here because
-    // the current default is to not print `any` for existential
-    // types, but this error message is super confusing without `any`
-    // if the user wrote it explicitly.
-    PrintOptions options;
-    options.PrintExplicitAny = true;
-    auto inheritedTyString = inheritedTy.getString(options);
-
     // We can't inherit from a non-class, non-protocol type.
     decl->diagnose(canHaveSuperclass
                    ? diag::inheritance_from_non_protocol_or_class
                    : diag::inheritance_from_non_protocol,
-                   inheritedTyString);
+                   inheritedTy);
     // FIXME: Note pointing to the declaration 'inheritedTy' references?
   }
 }