[Typechecker] Fix duplicate diagnostics when using attributes (swiftlang#28888)

* [Typechecker] resolveAttributedType() should invalidate the type repr after emitting a diagnostic, to prevent duplicate diagnostics later

* [Test] Update '@NoEscape' attribute diagnostics

* [Typechecker] Add a special diagnose method to TypeResolver that accepts a TypeRepr and invalidates it

* [Typechecker] Rename the special 'diagnose' method to 'diagnoseInvalid' for clarity

Author: theblixguy

lib/Sema/TypeCheckType.cpp

@@ -1782,6 +1782,14 @@ namespace {
       return diags.diagnose(std::forward<ArgTypes>(Args)...);
     }
 
+    template <typename... ArgTypes>
+    InFlightDiagnostic diagnoseInvalid(TypeRepr *repr,
+                                       ArgTypes &&... Args) const {
+      auto &diags = Context.Diags;
+      repr->setInvalid();
+      return diags.diagnose(std::forward<ArgTypes>(Args)...);
+    }
+
     Type resolveAttributedType(AttributedTypeRepr *repr,
                                TypeResolutionOptions options);
     Type resolveAttributedType(TypeAttributes &attrs, TypeRepr *repr,
@@ -2080,18 +2088,21 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
 
           // Check for @thick.
           if (attrs.has(TAK_thick)) {
-            if (storedRepr)
-              diagnose(repr->getStartLoc(), diag::sil_metatype_multiple_reprs);
-              
+            if (storedRepr) {
+              diagnoseInvalid(repr, repr->getStartLoc(),
+                              diag::sil_metatype_multiple_reprs);
+            }
+
             storedRepr = MetatypeRepresentation::Thick;
             attrs.clearAttribute(TAK_thick);
           }
 
           // Check for @objc_metatype.
           if (attrs.has(TAK_objc_metatype)) {
-            if (storedRepr)
-              diagnose(repr->getStartLoc(), diag::sil_metatype_multiple_reprs);
-              
+            if (storedRepr) {
+              diagnoseInvalid(repr, repr->getStartLoc(),
+                              diag::sil_metatype_multiple_reprs);
+            }
             storedRepr = MetatypeRepresentation::ObjC;
             attrs.clearAttribute(TAK_objc_metatype);
           }
@@ -2119,8 +2130,8 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
 
   auto checkUnsupportedAttr = [&](TypeAttrKind attr) {
     if (attrs.has(attr)) {
-      diagnose(attrs.getLoc(attr), diag::unknown_attribute,
-               TypeAttributes::getAttrName(attr));
+      diagnoseInvalid(repr, attrs.getLoc(attr), diag::unknown_attribute,
+                      TypeAttributes::getAttrName(attr));
       attrs.clearAttribute(attr);
     }
   };
@@ -2170,8 +2181,8 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
       auto calleeConvention = ParameterConvention::Direct_Unowned;
       if (attrs.has(TAK_callee_owned)) {
         if (attrs.has(TAK_callee_guaranteed)) {
-          diagnose(attrs.getLoc(TAK_callee_owned),
-                   diag::sil_function_repeat_convention, /*callee*/ 2);
+          diagnoseInvalid(repr, attrs.getLoc(TAK_callee_owned),
+                          diag::sil_function_repeat_convention, /*callee*/ 2);
         }
         calleeConvention = ParameterConvention::Direct_Owned;
       } else if (attrs.has(TAK_callee_guaranteed)) {
@@ -2197,8 +2208,9 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
                       SILFunctionType::Representation::WitnessMethod)
                 .Default(None);
         if (!parsedRep) {
-          diagnose(attrs.getLoc(TAK_convention),
-                   diag::unsupported_sil_convention, attrs.getConventionName());
+          diagnoseInvalid(repr, attrs.getLoc(TAK_convention),
+                          diag::unsupported_sil_convention,
+                          attrs.getConventionName());
           rep = SILFunctionType::Representation::Thin;
         } else {
           rep = *parsedRep;
@@ -2214,8 +2226,8 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
 
       if (attrs.has(TAK_differentiable) &&
           !Context.LangOpts.EnableExperimentalDifferentiableProgramming) {
-        diagnose(attrs.getLoc(TAK_differentiable),
-                 diag::experimental_differentiable_programming_disabled);
+        diagnoseInvalid(repr, attrs.getLoc(TAK_differentiable),
+                        diag::experimental_differentiable_programming_disabled);
       }
 
       DifferentiabilityKind diffKind = DifferentiabilityKind::NonDifferentiable;
@@ -2246,8 +2258,9 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
                 .Case("c", FunctionType::Representation::CFunctionPointer)
                 .Default(None);
         if (!parsedRep) {
-          diagnose(attrs.getLoc(TAK_convention), diag::unsupported_convention,
-                   attrs.getConventionName());
+          diagnoseInvalid(repr, attrs.getLoc(TAK_convention),
+                          diag::unsupported_convention,
+                          attrs.getConventionName());
           rep = FunctionType::Representation::Swift;
         } else {
           rep = *parsedRep;
@@ -2256,8 +2269,8 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
 
       if (attrs.has(TAK_differentiable) &&
           !Context.LangOpts.EnableExperimentalDifferentiableProgramming) {
-        diagnose(attrs.getLoc(TAK_differentiable),
-                 diag::experimental_differentiable_programming_disabled);
+        diagnoseInvalid(repr, attrs.getLoc(TAK_differentiable),
+                        diag::experimental_differentiable_programming_disabled);
       }
 
       DifferentiabilityKind diffKind = DifferentiabilityKind::NonDifferentiable;
@@ -2279,21 +2292,21 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
     if (attrs.hasConvention()) {
       if (attrs.getConventionName() == "c" ||
           attrs.getConventionName() == "block") {
-        diagnose(attrs.getLoc(TAK_convention),
-                 diag::invalid_autoclosure_and_convention_attributes,
-                 attrs.getConventionName());
+        diagnoseInvalid(repr, attrs.getLoc(TAK_convention),
+                        diag::invalid_autoclosure_and_convention_attributes,
+                        attrs.getConventionName());
         attrs.clearAttribute(TAK_convention);
         didDiagnose = true;
       }
     } else if (options.is(TypeResolverContext::VariadicFunctionInput) &&
                !options.hasBase(TypeResolverContext::EnumElementDecl)) {
-      diagnose(attrs.getLoc(TAK_autoclosure),
-               diag::attr_not_on_variadic_parameters, "@autoclosure");
+      diagnoseInvalid(repr, attrs.getLoc(TAK_autoclosure),
+                      diag::attr_not_on_variadic_parameters, "@autoclosure");
       attrs.clearAttribute(TAK_autoclosure);
       didDiagnose = true;
     } else if (!options.is(TypeResolverContext::FunctionInput)) {
-      diagnose(attrs.getLoc(TAK_autoclosure), diag::attr_only_on_parameters,
-               "@autoclosure");
+      diagnoseInvalid(repr, attrs.getLoc(TAK_autoclosure),
+                      diag::attr_only_on_parameters, "@autoclosure");
       attrs.clearAttribute(TAK_autoclosure);
       didDiagnose = true;
     }
@@ -2328,12 +2341,13 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
         auto loc = attrs.getLoc(TAK_escaping);
         auto attrRange = getTypeAttrRangeWithAt(Context, loc);
 
-        diagnose(loc, diag::escaping_non_function_parameter)
-          .fixItRemove(attrRange);
+        diagnoseInvalid(repr, loc, diag::escaping_non_function_parameter)
+            .fixItRemove(attrRange);
 
         // Try to find a helpful note based on how the type is being used
         if (options.is(TypeResolverContext::ImmediateOptionalTypeArgument)) {
-          diagnose(repr->getLoc(), diag::escaping_optional_type_argument);
+          diagnoseInvalid(repr, repr->getLoc(),
+                          diag::escaping_optional_type_argument);
         }
       }
 
@@ -2349,16 +2363,17 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
       // @autoclosure is going to be diagnosed when type of
       // the parameter is validated, because that attribute
       // applies to the declaration now.
+      repr->setInvalid();
       attrs.clearAttribute(TAK_autoclosure);
     }
 
     for (auto i : FunctionAttrs) {
       if (!attrs.has(i))
         continue;
 
-      auto diag = diagnose(attrs.getLoc(i),
-                           diag::attribute_requires_function_type,
-                           TypeAttributes::getAttrName(i));
+      auto diag = diagnoseInvalid(repr, attrs.getLoc(i),
+                                  diag::attribute_requires_function_type,
+                                  TypeAttributes::getAttrName(i));
 
       // If we see @escaping among the attributes on this type, because it isn't
       // a function type, we'll remove it.
@@ -2368,7 +2383,8 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
         // Specialize the diagnostic for Optionals.
         if (ty->getOptionalObjectType()) {
           diag.flush();
-          diagnose(repr->getLoc(), diag::escaping_optional_type_argument);
+          diagnoseInvalid(repr, repr->getLoc(),
+                          diag::escaping_optional_type_argument);
         }
       }
       attrs.clearAttribute(i);
@@ -2398,7 +2414,8 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
   // In SIL, handle @opened (n), which creates an existential archetype.
   if (attrs.has(TAK_opened)) {
     if (!ty->isExistentialType()) {
-      diagnose(attrs.getLoc(TAK_opened), diag::opened_non_protocol, ty);
+      diagnoseInvalid(repr, attrs.getLoc(TAK_opened), diag::opened_non_protocol,
+                      ty);
     } else {
       ty = OpenedArchetypeType::get(ty, attrs.OpenedID);
     }
@@ -2438,9 +2455,10 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
   }
 
   for (unsigned i = 0; i != TypeAttrKind::TAK_Count; ++i)
-    if (attrs.has((TypeAttrKind)i))
-      diagnose(attrs.getLoc((TypeAttrKind)i),
-               diag::attribute_does_not_apply_to_type);
+    if (attrs.has((TypeAttrKind)i)) {
+      diagnoseInvalid(repr, attrs.getLoc((TypeAttrKind)i),
+                      diag::attribute_does_not_apply_to_type);
+    }
 
   return ty;
 }
@@ -3080,8 +3098,7 @@ Type TypeResolver::resolveSpecifierTypeRepr(SpecifierTypeRepr *repr,
     default:
       llvm_unreachable("unknown SpecifierTypeRepr kind");
     }
-    diagnose(repr->getSpecifierLoc(), diagID, name);
-    repr->setInvalid();
+    diagnoseInvalid(repr, repr->getSpecifierLoc(), diagID, name);
     return ErrorType::get(Context);
   }
 

test/attr/attr_autoclosure.swift

@@ -292,3 +292,6 @@ func sr_11938_3(_ x: [@autoclosure String]) {} // expected-error {{'@autoclosure
 
 protocol SR_11938_P {}
 struct SR_11938_S : @autoclosure SR_11938_P {} // expected-error {{'@autoclosure' may only be used on parameters}}
+
+// SR-9178
+func bar<T>(_ x: @autoclosure T) {} // expected-error 1{{@autoclosure attribute only applies to function types}}

test/attr/attr_escaping.swift

@@ -226,3 +226,6 @@ extension SR_9760 {
   func fiz<T>(_: T, _: @escaping F) {} // Ok
   func baz<T>(_: @escaping G<T>) {} // Ok
 }
+
+// SR-9178
+func foo<T>(_ x: @escaping T) {} // expected-error 1{{@escaping attribute only applies to function types}}

test/attr/attr_noescape.swift

@@ -6,7 +6,7 @@ func conflictingAttrs(_ fn: @noescape @escaping () -> Int) {} // expected-error
 
 func doesEscape(_ fn : @escaping () -> Int) {}
 
-func takesGenericClosure<T>(_ a : Int, _ fn : @noescape () -> T) {} // expected-error 2{{unknown attribute 'noescape'}}
+func takesGenericClosure<T>(_ a : Int, _ fn : @noescape () -> T) {} // expected-error {{unknown attribute 'noescape'}}
 
 
 var globalAny: Any = 0
@@ -23,8 +23,8 @@ func takesVariadic(_ fns: () -> Int...) {
   doesEscape(fns[0]) // Okay - variadic-of-function parameters are escaping
 }
 
-func takesNoEscapeClosure(_ fn : () -> Int) { // expected-note 2 {{parameter 'fn' is implicitly non-escaping}} {{34-34=@escaping }}
-  // expected-note@-1 5{{parameter 'fn' is implicitly non-escaping}} {{34-34=@escaping }}
+func takesNoEscapeClosure(_ fn : () -> Int) { // expected-note 1 {{parameter 'fn' is implicitly non-escaping}} {{34-34=@escaping }}
+  // expected-note@-1 6{{parameter 'fn' is implicitly non-escaping}} {{34-34=@escaping }}
   takesNoEscapeClosure { 4 }  // ok
 
   _ = fn()  // ok
@@ -195,10 +195,7 @@ func testAutoclosure(_ a : @autoclosure () -> Int) { // expected-note{{parameter
 
 
 // <rdar://problem/19470858> QoI: @autoclosure implies @noescape, so you shouldn't be allowed to specify both
-func redundant(_ fn : @noescape
-               @autoclosure () -> Int) {
- // expected-error@-2{{unknown attribute 'noescape'}}
-}
+func redundant(_ fn : @noescape @autoclosure () -> Int) {} // expected-error {{unknown attribute 'noescape'}}
 
 
 protocol P1 {
@@ -215,9 +212,7 @@ func overloadedEach<P: P2, T>(_ source: P, _ transform: @escaping (P.Element) ->
 struct S : P2 {
   typealias Element = Int
   func each(_ transform: @noescape (Int) -> ()) { // expected-error{{unknown attribute 'noescape'}}
-                                                  // expected-note@-1 {{parameter 'transform' is implicitly non-escaping}}
     overloadedEach(self, transform, 1)
-    // expected-error@-1 {{passing non-escaping parameter 'transform' to function expecting an @escaping closure}}
   }
 }
 
@@ -261,22 +256,22 @@ public func XCTAssert(_ expression: @autoclosure () -> Bool, _ message: String =
 
 
 /// SR-770 - Currying and `noescape`/`rethrows` don't work together anymore
-func curriedFlatMap<A, B>(_ x: [A]) -> (@noescape (A) -> [B]) -> [B] { // expected-error 2{{unknown attribute 'noescape'}}
+func curriedFlatMap<A, B>(_ x: [A]) -> (@noescape (A) -> [B]) -> [B] { // expected-error 1{{unknown attribute 'noescape'}}
   return { f in
     x.flatMap(f)
   }
 }
 
-func curriedFlatMap2<A, B>(_ x: [A]) -> (@noescape (A) -> [B]) -> [B] { // expected-error 2{{unknown attribute 'noescape'}}
-  return { (f : @noescape (A) -> [B]) in // expected-error{{unknown attribute 'noescape'}}
+func curriedFlatMap2<A, B>(_ x: [A]) -> (@noescape (A) -> [B]) -> [B] { // expected-error {{unknown attribute 'noescape'}}
+  return { (f : @noescape (A) -> [B]) in
     x.flatMap(f)
   }
 }
 
 func bad(_ a : @escaping (Int)-> Int) -> Int { return 42 }
 func escapeNoEscapeResult(_ x: [Int]) -> (@noescape (Int) -> Int) -> Int { // expected-error{{unknown attribute 'noescape'}}
-  return { f in // expected-note {{parameter 'f' is implicitly non-escaping}}
-    bad(f) // expected-error {{passing non-escaping parameter 'f' to function expecting an @escaping closure}}
+  return { f in
+    bad(f)
   }
 }
 
@@ -296,24 +291,21 @@ typealias CompletionHandler = (_ success: Bool) -> ()
 var escape : CompletionHandlerNE
 var escapeOther : CompletionHandler
 func doThing1(_ completion: (_ success: Bool) -> ()) {
-  // expected-note@-1 {{parameter 'completion' is implicitly non-escaping}}
-  escape = completion // expected-error {{assigning non-escaping parameter 'completion' to an @escaping closure}}
+  escape = completion
 }
 func doThing2(_ completion: CompletionHandlerNE) {
-  // expected-note@-1 {{parameter 'completion' is implicitly non-escaping}}
-  escape = completion // expected-error {{assigning non-escaping parameter 'completion' to an @escaping closure}}
+  escape = completion
 }
 func doThing3(_ completion: CompletionHandler) {
-  // expected-note@-1 {{parameter 'completion' is implicitly non-escaping}}
-  escape = completion // expected-error {{assigning non-escaping parameter 'completion' to an @escaping closure}}
+  escape = completion
 }
 func doThing4(_ completion: @escaping CompletionHandler) {
   escapeOther = completion
 }
 
 // <rdar://problem/19997680> @noescape doesn't work on parameters of function type
 func apply<T, U>(_ f: @noescape (T) -> U, g: @noescape (@noescape (T) -> U) -> U) -> U { 
-  // expected-error@-1 6{{unknown attribute 'noescape'}}
+  // expected-error@-1 2{{unknown attribute 'noescape'}}
   return g(f)
 }
 
@@ -323,7 +315,7 @@ enum r19997577Type {
   case Function(() -> r19997577Type, () -> r19997577Type)
   case Sum(() -> r19997577Type, () -> r19997577Type)
 
-  func reduce<Result>(_ initial: Result, _ combine: @noescape (Result, r19997577Type) -> Result) -> Result { // expected-error 2{{unknown attribute 'noescape'}}
+  func reduce<Result>(_ initial: Result, _ combine: @noescape (Result, r19997577Type) -> Result) -> Result { // expected-error 1{{unknown attribute 'noescape'}}
     let binary: @noescape (r19997577Type, r19997577Type) -> Result = { combine(combine(combine(initial, self), $0), $1) } // expected-error{{unknown attribute 'noescape'}}
     switch self {
     case .Unit: