Merge pull request swiftlang#32524 from OnyekachiSamuel/fix-confusing-protocol-diagnostic

hborla · web-flow · commit b87152817940 · 2020-07-02T13:01:19.000-07:00
[Diagnostics] Fix Confusing Protocol Diagnostic
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -1801,11 +1801,11 @@ 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{|value of protocol }0type %1 cannot conform to %2; "
-      "only struct/enum/class types can conform to protocols",
-      (bool, Type, Type))
+      "%select{type %1|protocol %1 as a type}0 cannot conform to "
+      "%select{%3|the protocol itself}2; "
+      "only concrete types such as structs, enums and classes can conform to protocols",
+      (bool, Type, bool, Type))
 NOTE(required_by_opaque_return,none,
     "required by opaque return type of %0 %1", (DescriptiveDeclKind, DeclName))
 NOTE(required_by_decl,none,
diff --git a/include/swift/AST/EducationalNotes.def b/include/swift/AST/EducationalNotes.def
@@ -65,5 +65,6 @@ EDUCATIONAL_NOTES(append_interpolation_static,
                   "string-interpolation-conformance.md")
 EDUCATIONAL_NOTES(append_interpolation_void_or_discardable,
                   "string-interpolation-conformance.md")
+EDUCATIONAL_NOTES(type_cannot_conform, "protocol-type-non-conformance.md")                
 
 #undef EDUCATIONAL_NOTES
diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp
@@ -500,7 +500,9 @@ bool MissingConformanceFailure::diagnoseTypeCannotConform(
   }
 
   emitDiagnostic(diag::type_cannot_conform,
-                 nonConformingType->isExistentialType(), nonConformingType,
+                 nonConformingType->isExistentialType(), 
+                 nonConformingType, 
+                 nonConformingType->isEqual(protocolType),
                  protocolType);
 
   if (auto *OTD = dyn_cast<OpaqueTypeDecl>(AffectedDecl)) {
@@ -2087,7 +2089,8 @@ bool ContextualFailure::diagnoseAsError() {
     if (CTP == CTP_ForEachStmt) {
       if (fromType->isAnyExistentialType()) {
         emitDiagnostic(diag::type_cannot_conform,
-                       /*isExistentialType=*/true, fromType, toType);
+                       /*isExistentialType=*/true, fromType, 
+                       fromType->isEqual(toType), toType);
         return true;
       }
 
diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp
@@ -4312,7 +4312,8 @@ void swift::diagnoseConformanceFailure(Type T,
 
     if (!T->isObjCExistentialType()) {
       diags.diagnose(ComplainLoc, diag::type_cannot_conform, true,
-                     T, Proto->getDeclaredType());
+                     T, T->isEqual(Proto->getDeclaredType()), 
+                     Proto->getDeclaredType());
       return;
     }
 
diff --git a/test/Constraints/diagnostics.swift b/test/Constraints/diagnostics.swift
@@ -51,9 +51,9 @@ f0(i, i, // expected-error@:7 {{cannot convert value of type 'Int' to expected a
 
 
 // Cannot conform to protocols.
-f5(f4)  // expected-error {{type '(Int) -> Int' cannot conform to 'P2'; only struct/enum/class types can conform to protocols}}
-f5((1, "hello"))  // expected-error {{type '(Int, String)' cannot conform to 'P2'; only struct/enum/class types can conform to protocols}}
-f5(Int.self) // expected-error {{type 'Int.Type' cannot conform to 'P2'; only struct/enum/class types can conform to protocols}}
+f5(f4)  // expected-error {{type '(Int) -> Int' cannot conform to 'P2'; only concrete types such as structs, enums and classes can conform to protocols}}
+f5((1, "hello"))  // expected-error {{type '(Int, String)' cannot conform to 'P2'; only concrete types such as structs, enums and classes can conform to protocols}}
+f5(Int.self) // expected-error {{type 'Int.Type' cannot conform to 'P2'; only concrete types such as structs, enums and classes can conform to protocols}}
 
 // Tuple element not convertible.
 f0(i,
@@ -104,7 +104,7 @@ func f8<T:P2>(_ n: T, _ f: @escaping (T) -> T) {}  // expected-note {{where 'T'
 f8(3, f4) // expected-error {{global function 'f8' requires that 'Int' conform to 'P2'}}
 typealias Tup = (Int, Double)
 func f9(_ x: Tup) -> Tup { return x }
-f8((1,2.0), f9) // expected-error {{type 'Tup' (aka '(Int, Double)') cannot conform to 'P2'; only struct/enum/class types can conform to protocols}}
+f8((1,2.0), f9) // expected-error {{type 'Tup' (aka '(Int, Double)') cannot conform to 'P2'; only concrete types such as structs, enums and classes can conform to protocols}}
 
 // <rdar://problem/19658691> QoI: Incorrect diagnostic for calling nonexistent members on literals
 1.doesntExist(0)  // expected-error {{value of type 'Int' has no member 'doesntExist'}}
diff --git a/test/Constraints/function_builder_diags.swift b/test/Constraints/function_builder_diags.swift
@@ -179,7 +179,7 @@ struct Label<L> : P where L : P { // expected-note 2 {{'L' declared as parameter
 }
 
 func test_51167632() -> some P {
-  AnyP(G { // expected-error {{type 'Label<_>.Type' cannot conform to 'P'; only struct/enum/class types can conform to protocols}}
+  AnyP(G { // expected-error {{type 'Label<_>.Type' cannot conform to 'P'; only concrete types such as structs, enums and classes can conform to protocols}}
     Text("hello")
     Label  // expected-error {{generic parameter 'L' could not be inferred}}
     // expected-note@-1 {{explicitly specify the generic arguments to fix this issue}} {{10-10=<<#L: P#>>}}
diff --git a/test/Constraints/generics.swift b/test/Constraints/generics.swift
@@ -188,7 +188,7 @@ func r22459135() {
 
 // <rdar://problem/19710848> QoI: Friendlier error message for "[] as Set"
 // <rdar://problem/22326930> QoI: "argument for generic parameter 'Element' could not be inferred" lacks context
-_ = [] as Set  // expected-error {{value of protocol type 'Any' cannot conform to 'Hashable'; only struct/enum/class types can conform to protocols}}
+_ = [] as Set  // expected-error {{protocol 'Any' as a type cannot conform to 'Hashable'; only concrete types such as structs, enums and classes can conform to protocols}}
 // expected-note@-1 {{required by generic struct 'Set' where 'Element' = 'Any'}}
 
 
diff --git a/test/Generics/conditional_conformances_literals.swift b/test/Generics/conditional_conformances_literals.swift
@@ -128,9 +128,9 @@ func combined() {
 
     // Needs self conforming protocols:
     let _: Conforms = [[0: [1 : [works]] as Conforms]]
-    // expected-error@-1 {{value of protocol type 'Conforms' cannot conform to 'Conforms'; only struct/enum/class types can conform to protocols}}
+    // expected-error@-1 {{protocol 'Conforms' as a type cannot conform to the protocol itself; only concrete types such as structs, enums and classes can conform to protocols}}
 
     let _: Conforms = [[0: [1 : [fails]] as Conforms]]
     // expected-error@-1 {{protocol 'Conforms' requires that 'Fails' conform to 'Conforms'}}
-    // expected-error@-2 {{value of protocol type 'Conforms' cannot conform to 'Conforms'; only struct/enum/class types can conform to protocols}}
+    // expected-error@-2 {{protocol 'Conforms' as a type cannot conform to the protocol itself; only concrete types such as structs, enums and classes can conform to protocols}}
 }
diff --git a/test/Generics/existential_restrictions.swift b/test/Generics/existential_restrictions.swift
@@ -23,7 +23,7 @@ func fAOE(_ t: AnyObject) { }
 func fT<T>(_ t: T) { }
 
 func testPassExistential(_ p: P, op: OP, opp: OP & P, cp: CP, sp: SP, any: Any, ao: AnyObject) {
-  fP(p) // expected-error{{value of protocol type 'P' cannot conform to 'P'; only struct/enum/class types can conform to protocols}}
+  fP(p) // expected-error{{protocol 'P' as a type cannot conform to the protocol itself; only concrete types such as structs, enums and classes can conform to protocols}}
   fAO(p) // expected-error{{global function 'fAO' requires that 'P' be a class type}}
   fAOE(p) // expected-error{{argument type 'P' expected to be an instance of a class or class-constrained type}}
   fT(p)
@@ -37,8 +37,8 @@ func testPassExistential(_ p: P, op: OP, opp: OP & P, cp: CP, sp: SP, any: Any,
   fAOE(cp)
   fT(cp)
 
-  fP(opp) // expected-error{{value of protocol type 'OP & P' cannot conform to 'P'; only struct/enum/class types can conform to protocols}}
-  fOP(opp) // expected-error{{value of protocol type 'OP & P' cannot conform to 'OP'; only struct/enum/class types can conform to protocols}}
+  fP(opp) // expected-error{{protocol 'OP & P' as a type cannot conform to 'P'; only concrete types such as structs, enums and classes can conform to protocols}}
+  fOP(opp) // expected-error{{protocol 'OP & P' as a type cannot conform to 'OP'; only concrete types such as structs, enums and classes can conform to protocols}}
   fAO(opp) // expected-error{{global function 'fAO' requires that 'OP & P' be a class type}}
   fAOE(opp)
   fT(opp)
@@ -64,9 +64,9 @@ class GAO<T : AnyObject> {} // expected-note 2{{requirement specified as 'T' : '
 func blackHole(_ t: Any) {}
 
 func testBindExistential() {
-  blackHole(GP<P>()) // expected-error{{value of protocol type 'P' cannot conform to 'P'; only struct/enum/class types can conform to protocols}}
+  blackHole(GP<P>()) // expected-error{{protocol 'P' as a type cannot conform to the protocol itself; only concrete types such as structs, enums and classes can conform to protocols}}
   blackHole(GOP<OP>())
-  blackHole(GCP<CP>()) // expected-error{{value of protocol type 'CP' cannot conform to 'CP'; only struct/enum/class types can conform to protocols}}
+  blackHole(GCP<CP>()) // expected-error{{protocol 'CP' as a type cannot conform to the protocol itself; only concrete types such as structs, enums and classes can conform to protocols}}
   blackHole(GAO<P>()) // expected-error{{'GAO' requires that 'P' be a class type}}
   blackHole(GAO<OP>())
   blackHole(GAO<CP>()) // expected-error{{'GAO' requires that 'CP' be a class type}}
@@ -92,5 +92,5 @@ func foo() {
   // generic no overloads error path. The error should actually talk
   // about the return type, and this can happen in other contexts as well;
   // <rdar://problem/21900971> tracks improving QoI here.
-  allMine.takeAll() // expected-error{{value of protocol type 'Mine' cannot conform to 'Mine'; only struct/enum/class types can conform to protocols}}
+  allMine.takeAll() // expected-error{{protocol 'Mine' as a type cannot conform to the protocol itself; only concrete types such as structs, enums and classes can conform to protocols}}
 }
diff --git a/test/Misc/misc_diagnostics.swift b/test/Misc/misc_diagnostics.swift
@@ -143,7 +143,7 @@ func test17875634() {
 func test20770032() {
   if case let 1...10 = (1, 1) { // expected-warning{{'let' pattern has no effect; sub-pattern didn't bind any variables}} {{11-15=}}
     // expected-error@-1 {{expression pattern of type 'ClosedRange<Int>' cannot match values of type '(Int, Int)'}}
-    // expected-error@-2 {{'(Int, Int)' cannot conform to 'Equatable'; only struct/enum/class types can conform to protocols}}
+    // expected-error@-2 {{type '(Int, Int)' cannot conform to 'Equatable'; only concrete types such as structs, enums and classes can conform to protocols}}
     // expected-note@-3 {{required by operator function '~=' where 'T' = '(Int, Int)'}}
   }
 }
diff --git a/test/Parse/confusables.swift b/test/Parse/confusables.swift
@@ -17,7 +17,7 @@ if (true ꝸꝸꝸ false) {} // expected-note {{identifier 'ꝸꝸꝸ' contains
 
 // expected-error @+3 {{invalid character in source file}}
 // expected-error @+2 {{expected ',' separator}}
-// expected-error @+1 {{type '(Int, Int)' cannot conform to 'BinaryInteger'; only struct/enum/class types can conform to protocols}}
+// expected-error @+1 {{type '(Int, Int)' cannot conform to 'BinaryInteger'; only concrete types such as structs, enums and classes can conform to protocols}}
 if (5 ‒ 5) == 0 {} // expected-note {{unicode character '‒' looks similar to '-'; did you mean to use '-'?}} {{7-10=-}}
 // expected-note @-1 {{operator function '=='}}
 
diff --git a/test/decl/protocol/conforms/error_self_conformance.swift b/test/decl/protocol/conforms/error_self_conformance.swift
@@ -11,15 +11,15 @@ func testSimple(error: Error) {
 
 protocol ErrorRefinement : Error {}
 func testErrorRefinment(error: ErrorRefinement) {
-  wantsError(error) // expected-error {{value of protocol type 'ErrorRefinement' cannot conform to 'Error'; only struct/enum/class types can conform to protocols}}
+  wantsError(error) // expected-error {{protocol 'ErrorRefinement' as a type cannot conform to 'Error'; only concrete types such as structs, enums and classes can conform to protocols}}
 }
 
 protocol OtherProtocol {}
 func testErrorComposition(error: Error & OtherProtocol) {
-  wantsError(error) // expected-error {{value of protocol type 'Error & OtherProtocol' cannot conform to 'Error'; only struct/enum/class types can conform to protocols}}
+  wantsError(error) // expected-error {{protocol 'Error & OtherProtocol' as a type cannot conform to 'Error'; only concrete types such as structs, enums and classes can conform to protocols}}
 }
 
 class C {}
 func testErrorCompositionWithClass(error: Error & C) {
-  wantsError(error) // expected-error {{value of protocol type 'C & Error' cannot conform to 'Error'; only struct/enum/class types can conform to protocols}}
+  wantsError(error) // expected-error {{protocol 'C & Error' as a type cannot conform to 'Error'; only concrete types such as structs, enums and classes can conform to protocols}}
 }
diff --git a/test/stmt/foreach.swift b/test/stmt/foreach.swift
@@ -177,7 +177,7 @@ func testOptionalSequence() {
 
 // Crash with (invalid) for each over an existential
 func testExistentialSequence(s: Sequence) { // expected-error {{protocol 'Sequence' can only be used as a generic constraint because it has Self or associated type requirements}}
-  for x in s { // expected-error {{value of protocol type 'Sequence' cannot conform to 'Sequence'; only struct/enum/class types can conform to protocols}}
+  for x in s { // expected-error {{protocol 'Sequence' as a type cannot conform to the protocol itself; only concrete types such as structs, enums and classes can conform to protocols}}
     _ = x
   }
 }
diff --git a/test/type/opaque.swift b/test/type/opaque.swift
@@ -383,7 +383,7 @@ protocol P_51641323 {
 func rdar_51641323() {
   struct Foo: P_51641323 {
     var foo: some P_51641323 { // expected-note {{required by opaque return type of property 'foo'}}
-      {} // expected-error {{type '() -> ()' cannot conform to 'P_51641323'; only struct/enum/class types can conform to protocols}}
+      {} // expected-error {{type '() -> ()' cannot conform to 'P_51641323'; only concrete types such as structs, enums and classes can conform to protocols}}
     }
   }
 }
diff --git a/test/type/subclass_composition.swift b/test/type/subclass_composition.swift
@@ -413,7 +413,7 @@ func conformsTo<T1 : P2, T2 : Base<Int> & P2>(
   // expected-error@-1 {{global function 'conformsToAnyObject' requires that 'P1' be a class type}}
 
   conformsToP1(p1)
-  // expected-error@-1 {{value of protocol type 'P1' cannot conform to 'P1'; only struct/enum/class types can conform to protocols}}
+  // expected-error@-1 {{protocol 'P1' as a type cannot conform to the protocol itself; only concrete types such as structs, enums and classes can conform to protocols}}
 
   // FIXME: Following diagnostics are not great because when
   // `conformsTo*` methods are re-typechecked, they loose information
diff --git a/userdocs/diagnostics/protocol-type-non-conformance.md b/userdocs/diagnostics/protocol-type-non-conformance.md
@@ -0,0 +1,46 @@
+# Protocol type not conforming to itself
+Protocols in Swift may be used as types. Protocols as types are sometimes called existential types.
+
+
+```swift 
+protocol P {}
+
+struct S: P {}
+
+var s: P = S() // This creates existential type because the protocol P is used as a type
+```
+
+However, a protocol type does not conform to protocols - not even the protocol itself. 
+Allowing existential types to conform to protocols is unsound. For protocols with static method, initializer, or associated type requirements, the implementation of these requirements cannot be accessed from the protocol type - accessing these kinds of requirements must be done using a concrete type.
+
+Let's walk through the example below:
+
+```swift
+protocol Word: Hashable {
+    var word: String { get }
+}
+
+struct Singular: Word {
+    var word: String
+}
+
+struct Plural: Word {
+    var word: String
+}
+
+let singularWord = Singular(word: "mango")
+let pluralWord = Plural(word: "mangoes")
+
+let wordPairDict: [Word: Word] = [singularWord: pluralWord] // Error
+```
+
+One workaround to fix this problem is to use type erasure for the protocol `Word`. Think of type erasure as a way to hide an object's type. Since `Word` is of type `Hashable`, we already have `AnyHashable` type erasure available in the standard library which we can easily use here.
+
+```swift 
+// The fix
+let wordPairDict: [AnyHashable: AnyHashable] = [singularWord: pluralWord]
+```
+
+# Exceptions
+`@objc` protocol type with no static requirements however do conform to its own protocol. Another exception is the `Error` Swift protocol.
+
diff --git a/validation-test/Sema/type_checker_crashers_fixed/rdar27830834.swift b/validation-test/Sema/type_checker_crashers_fixed/rdar27830834.swift
@@ -2,5 +2,5 @@
 
 var d = [String:String]()
 _ = "\(d.map{ [$0 : $0] })"
-// expected-error@-1 {{type 'Dictionary<String, String>.Element' (aka '(key: String, value: String)') cannot conform to 'Hashable'; only struct/enum/class types can conform to protocols}}
+// expected-error@-1 {{type 'Dictionary<String, String>.Element' (aka '(key: String, value: String)') cannot conform to 'Hashable'; only concrete types such as structs, enums and classes can conform to protocols}}
 // expected-note@-2 {{required by generic struct 'Dictionary' where 'Key' = 'Dictionary<String, String>.Element' (aka '(key: String, value: String)')}}
diff --git a/validation-test/compiler_crashers_2_fixed/0196-rdar48937223.swift b/validation-test/compiler_crashers_2_fixed/0196-rdar48937223.swift
@@ -6,7 +6,7 @@ func fn<T, U: P>(_ arg1: T, arg2: (T) -> U) {}
 // expected-note@-1 {{required by global function 'fn(_:arg2:)' where 'U' = '()'}}
 
 func test(str: String) {
-  fn(str) { arg in // expected-error {{type '()' cannot conform to 'P'; only struct/enum/class types can conform to protocols}}
+  fn(str) { arg in // expected-error {{type '()' cannot conform to 'P'; only concrete types such as structs, enums and classes can conform to protocols}}
     <#FOO#> // expected-error {{editor placeholder in source file}}
   }
 }
diff --git a/validation-test/compiler_crashers_fixed/00017-llvm-foldingset-llvm-attributesetnode-nodeequals.swift b/validation-test/compiler_crashers_fixed/00017-llvm-foldingset-llvm-attributesetnode-nodeequals.swift
@@ -20,4 +20,4 @@ extension Bool : BooleanProtocol {
 func f<T : BooleanProtocol>(_ b: T) {}
 // expected-note@-1 {{required by global function 'f' where 'T' = 'BooleanProtocol'}}
 
-f(true as BooleanProtocol) // expected-error {{value of protocol type 'BooleanProtocol' cannot conform to 'BooleanProtocol'; only struct/enum/class types can conform to protocols}}
+f(true as BooleanProtocol) // expected-error {{protocol 'BooleanProtocol' as a type cannot conform to the protocol itself; only concrete types such as structs, enums and classes can conform to protocols}}

Original file line number	Diff line number	Diff line change
`@@ -4312,7 +4312,8 @@ void swift::diagnoseConformanceFailure(Type T,`
`4312`	`4312`
`4313`	`4313`	`if (!T->isObjCExistentialType()) {`
`4314`	`4314`	`diags.diagnose(ComplainLoc, diag::type_cannot_conform, true,`
`4315`		`- T, Proto->getDeclaredType());`
	`4315`	`+ T, T->isEqual(Proto->getDeclaredType()),`
	`4316`	`+ Proto->getDeclaredType());`
`4316`	`4317`	`return;`
`4317`	`4318`	`}`
`4318`	`4319`
Original file line number	Diff line number	Diff line change
`@@ -179,7 +179,7 @@ struct Label<L> : P where L : P { // expected-note 2 {{'L' declared as parameter`
`179`	`179`	`}`
`180`	`180`
`181`	`181`	`func test_51167632() -> some P {`
`182`		`- AnyP(G { // expected-error {{type 'Label<_>.Type' cannot conform to 'P'; only struct/enum/class types can conform to protocols}}`
	`182`	`+ AnyP(G { // expected-error {{type 'Label<_>.Type' cannot conform to 'P'; only concrete types such as structs, enums and classes can conform to protocols}}`
`183`	`183`	`Text("hello")`
`184`	`184`	`Label // expected-error {{generic parameter 'L' could not be inferred}}`
`185`	`185`	`// expected-note@-1 {{explicitly specify the generic arguments to fix this issue}} {{10-10=<<#L: P#>>}}`
Original file line number	Diff line number	Diff line change
`@@ -143,7 +143,7 @@ func test17875634() {`
`143`	`143`	`func test20770032() {`
`144`	`144`	`if case let 1...10 = (1, 1) { // expected-warning{{'let' pattern has no effect; sub-pattern didn't bind any variables}} {{11-15=}}`
`145`	`145`	`// expected-error@-1 {{expression pattern of type 'ClosedRange<Int>' cannot match values of type '(Int, Int)'}}`
`146`		`- // expected-error@-2 {{'(Int, Int)' cannot conform to 'Equatable'; only struct/enum/class types can conform to protocols}}`
	`146`	`+ // expected-error@-2 {{type '(Int, Int)' cannot conform to 'Equatable'; only concrete types such as structs, enums and classes can conform to protocols}}`
`147`	`147`	`// expected-note@-3 {{required by operator function '~=' where 'T' = '(Int, Int)'}}`
`148`	`148`	`}`
`149`	`149`	`}`