Sema: Clean up printing of generic signatures in missing witness diagnostics

Instead of stripping out requirements, let's pass the right PrintOptions.

Author: slavapestov

include/swift/AST/DiagnosticEngine.h

@@ -105,6 +105,14 @@ namespace swift {
     Type getType() const { return t; }
   };
 
+  struct WitnessType {
+    Type t;
+
+    WitnessType(Type t) : t(t) {}
+
+    Type getType() { return t; }
+  };
+
   /// Describes the kind of diagnostic argument we're storing.
   ///
   enum class DiagnosticArgumentKind {
@@ -117,6 +125,7 @@ namespace swift {
     Type,
     TypeRepr,
     FullyQualifiedType,
+    WitnessType,
     DescriptivePatternKind,
     SelfAccessKind,
     ReferenceOwnership,
@@ -151,6 +160,7 @@ namespace swift {
       Type TypeVal;
       TypeRepr *TyR;
       FullyQualified<Type> FullyQualifiedTypeVal;
+      WitnessType WitnessTypeVal;
       DescriptivePatternKind DescriptivePatternKindVal;
       SelfAccessKind SelfAccessKindVal;
       ReferenceOwnership ReferenceOwnershipVal;
@@ -214,6 +224,10 @@ namespace swift {
         : Kind(DiagnosticArgumentKind::FullyQualifiedType),
           FullyQualifiedTypeVal(FQT) {}
 
+    DiagnosticArgument(WitnessType WT)
+        : Kind(DiagnosticArgumentKind::WitnessType),
+          WitnessTypeVal(WT) {}
+
     DiagnosticArgument(const TypeLoc &TL) {
       if (TypeRepr *tyR = TL.getTypeRepr()) {
         Kind = DiagnosticArgumentKind::TypeRepr;
@@ -329,6 +343,11 @@ namespace swift {
       return FullyQualifiedTypeVal;
     }
 
+    WitnessType getAsWitnessType() const {
+      assert(Kind == DiagnosticArgumentKind::WitnessType);
+      return WitnessTypeVal;
+    }
+
     DescriptivePatternKind getAsDescriptivePatternKind() const {
       assert(Kind == DiagnosticArgumentKind::DescriptivePatternKind);
       return DescriptivePatternKindVal;

include/swift/AST/DiagnosticsSema.def

@@ -2883,19 +2883,19 @@ NOTE(no_witnesses,none,
      "protocol requires "
      "%select{initializer %1|function %1|property %1|subscript}0 with type %2"
      "%select{|; add a stub for conformance}3",
-     (RequirementKind, const ValueDecl *, Type, bool))
+     (RequirementKind, const ValueDecl *, WitnessType, bool))
 NOTE(missing_witnesses_general,none, "add stubs for conformance",
      ())
 NOTE(ambiguous_witnesses,none,
      "multiple matching "
      "%select{initializers named %1|functions named %1|properties named %1|"
      "subscript operators}0 with type %2",
-     (RequirementKind, const ValueDecl *, Type))
+     (RequirementKind, const ValueDecl *, WitnessType))
 NOTE(ambiguous_witnesses_wrong_name,none,
      "multiple matching "
      "%select{initializers named %1|functions named %1|properties named %1|"
      "subscript operators}0 with type %2",
-     (RequirementKind, const ValueDecl *, Type))
+     (RequirementKind, const ValueDecl *, WitnessType))
 NOTE(no_witnesses_type,none,
      "protocol requires nested type %0; add nested type %0 for conformance",
      (const AssociatedTypeDecl *))
@@ -2982,7 +2982,7 @@ NOTE(protocol_witness_kind_conflict,none,
      "candidate is not %select{an initializer|a function|a variable|"
      "a subscript}0", (RequirementKind))
 NOTE(protocol_witness_type_conflict,none,
-     "candidate has non-matching type %0%1", (Type, StringRef))
+     "candidate has non-matching type %0%1", (WitnessType, StringRef))
 NOTE(protocol_witness_missing_requirement,none,
      "candidate would match if %0 %select{conformed to|subclassed|"
      "was the same type as|%error|%error}2 %1", (Type, Type, unsigned))

lib/AST/DiagnosticEngine.cpp

@@ -755,7 +755,8 @@ static void formatDiagnosticArgument(StringRef Modifier,
   }
 
   case DiagnosticArgumentKind::FullyQualifiedType:
-  case DiagnosticArgumentKind::Type: {
+  case DiagnosticArgumentKind::Type:
+  case DiagnosticArgumentKind::WitnessType: {
     assert(Modifier.empty() && "Improper modifier for Type argument");
 
     // Strip extraneous parentheses; they add no value.
@@ -777,8 +778,7 @@ static void formatDiagnosticArgument(StringRef Modifier,
         printOptions.PrintFunctionRepresentationAttrs =
             PrintOptions::FunctionRepresentationMode::Full;
       needsQualification = typeSpellingIsAmbiguous(type, Args, printOptions);
-    } else {
-      assert(Arg.getKind() == DiagnosticArgumentKind::FullyQualifiedType);
+    } else if (Arg.getKind() == DiagnosticArgumentKind::FullyQualifiedType) {
       type = Arg.getAsFullyQualifiedType().getType()->getWithoutParens();
       if (type.isNull()) {
         // FIXME: We should never receive a nullptr here, but this is causing
@@ -791,6 +791,19 @@ static void formatDiagnosticArgument(StringRef Modifier,
         printOptions.PrintFunctionRepresentationAttrs =
             PrintOptions::FunctionRepresentationMode::Full;
       needsQualification = true;
+    } else {
+      assert(Arg.getKind() == DiagnosticArgumentKind::WitnessType);
+      type = Arg.getAsWitnessType().getType()->getWithoutParens();
+      if (type.isNull()) {
+        // FIXME: We should never receive a nullptr here, but this is causing
+        // crashes (rdar://75740683). Remove once ParenType never contains
+        // nullptr as the underlying type.
+        Out << "<null>";
+        break;
+      }
+      printOptions.PrintGenericRequirements = false;
+      printOptions.PrintInverseRequirements = false;
+      needsQualification = typeSpellingIsAmbiguous(type, Args, printOptions);
     }
 
     // If a type has an unresolved type, print it with syntax sugar removed for

lib/Sema/TypeCheckProtocol.cpp

@@ -2493,24 +2493,18 @@ static Type getTypeForDisplay(ModuleDecl *module, ValueDecl *decl) {
   if (!decl->getDeclContext()->isTypeContext())
     return type;
 
-  GenericSignature sigWithoutReqts;
-  if (auto genericFn = type->getAs<GenericFunctionType>()) {
-    // For generic functions, build a new generic function... but strip off
-    // the requirements. They don't add value.
-    sigWithoutReqts
-      = GenericSignature::get(genericFn->getGenericParams(), {});
-  }
-
   // For functions, strip off the 'Self' parameter clause.
-  if (isa<AbstractFunctionDecl>(decl))
-    type = type->castTo<AnyFunctionType>()->getResult();
+  if (isa<AbstractFunctionDecl>(decl)) {
+    if (auto genericFn = type->getAs<GenericFunctionType>()) {
+      auto sig = genericFn->getGenericSignature();
+      auto resultFn = genericFn->getResult()->castTo<FunctionType>();
+      return GenericFunctionType::get(sig,
+                                      resultFn->getParams(),
+                                      resultFn->getResult(),
+                                      resultFn->getExtInfo());
+    }
 
-  if (sigWithoutReqts) {
-    auto resultFn = type->castTo<AnyFunctionType>();
-    return GenericFunctionType::get(sigWithoutReqts,
-                                    resultFn->getParams(),
-                                    resultFn->getResult(),
-                                    resultFn->getExtInfo());
+    return type->castTo<FunctionType>()->getResult();
   }
 
   return type;
@@ -2800,18 +2794,18 @@ diagnoseMatch(ModuleDecl *module, NormalProtocolConformance *conformance,
     break;
 
   case MatchKind::TypeConflict: {
+    auto witnessType = getTypeForDisplay(module, match.Witness);
+
     if (!isa<TypeDecl>(req) && !isa<EnumElementDecl>(match.Witness)) {
       computeFixitsForOverriddenDeclaration(match.Witness, req, [&](bool){
         return diags.diagnose(match.Witness,
                               diag::protocol_witness_type_conflict,
-                              getTypeForDisplay(module, match.Witness),
-                              withAssocTypes);
+                              witnessType, withAssocTypes);
       });
     } else {
       diags.diagnose(match.Witness,
                      diag::protocol_witness_type_conflict,
-                     getTypeForDisplay(module, match.Witness),
-                     withAssocTypes);
+                     witnessType, withAssocTypes);
     }
     break;
   }
@@ -4367,6 +4361,7 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
       // Determine the type that the requirement is expected to have.
       Type reqType = getRequirementTypeForDisplay(dc->getParentModule(),
                                                   conformance, requirement);
+
       auto &diags = dc->getASTContext().Diags;
       auto diagnosticMessage = diag::ambiguous_witnesses;
       if (ignoringNames) {

test/Constraints/generic_protocol_witness.swift

@@ -25,7 +25,7 @@ class EqualConstraints : NeedsGenericMethods {
   func twoArgsOneVar<U>(x: U, y: U) {}
   func twoArgsTwoVars<V, U>(x: U, y: V) {}
 }
-func •<T : Fungible>(x: EqualConstraints, y: T) {} // expected-note {{candidate has non-matching type '<T where T : Fungible> (EqualConstraints, T) -> ()'}}
+func •<T : Fungible>(x: EqualConstraints, y: T) {} // expected-note {{candidate has non-matching type '<T> (EqualConstraints, T) -> ()'}}
 
 class LooseConstraints : NeedsGenericMethods {
   func oneArgNoConstraints<U>(x: U) {}