[Evaluator] Avoid emitting duplicate "through reference here" notes

Filter out any duplicate notes to help cut down on the noise for
request cycle diagnostics. Some of the note locations here still aren't
great, but this at least stops us from repeating them for each
intermediate request.

Author: hamishknight

include/swift/AST/DiagnosticEngine.h

@@ -1462,6 +1462,13 @@ namespace swift {
     /// Retrieve the underlying engine which will receive the diagnostics.
     DiagnosticEngine &getUnderlyingDiags() const { return UnderlyingEngine; }
 
+    /// Iterates over each captured diagnostic, running a lambda with it.
+    void forEach(llvm::function_ref<void(const Diagnostic &)> body) const;
+
+    /// Filters the queued diagnostics, dropping any where the predicate
+    /// returns \c false.
+    void filter(llvm::function_ref<bool(const Diagnostic &)> predicate);
+
     /// Clear this queue and erase all diagnostics recorded.
     void clear() {
       assert(QueueEngine.TransactionCount == 1 &&

lib/AST/DiagnosticEngine.cpp

@@ -1784,6 +1784,20 @@ void DiagnosticEngine::onTentativeDiagnosticFlush(Diagnostic &diagnostic) {
   }
 }
 
+void DiagnosticQueue::forEach(
+    llvm::function_ref<void(const Diagnostic &)> body) const {
+  for (auto &activeDiag : QueueEngine.TentativeDiagnostics)
+    body(activeDiag.Diag);
+}
+
+void DiagnosticQueue::filter(
+    llvm::function_ref<bool(const Diagnostic &)> predicate) {
+  llvm::erase_if(QueueEngine.TentativeDiagnostics,
+                 [&](detail::ActiveDiagnostic &activeDiag) {
+                   return !predicate(activeDiag.Diag);
+                 });
+}
+
 EncodedDiagnosticMessage::EncodedDiagnosticMessage(StringRef S)
     : Message(Lexer::getEncodedStringSegment(S, Buf, /*IsFirstSegment=*/true,
                                              /*IsLastSegment=*/true,

lib/AST/Evaluator.cpp

@@ -19,6 +19,7 @@
 #include "swift/AST/DiagnosticEngine.h"
 #include "swift/AST/TypeCheckRequests.h" // for ResolveMacroRequest
 #include "swift/Basic/Assertions.h"
+#include "swift/Basic/Defer.h"
 #include "swift/Basic/LangOptions.h"
 #include "swift/Basic/Range.h"
 #include "swift/Basic/SourceManager.h"
@@ -123,17 +124,40 @@ void Evaluator::diagnoseCycle(const ActiveRequest &request) {
     OS << "\n";
   }
 
-  request.diagnoseCycle(diags);
-  for (const auto &step : llvm::reverse(activeRequests)) {
-    if (step == request) return;
-
-    // Reporting the lifetime dependence location generates a redundant
-    // diagnostic.
-    if (step.getAs<LifetimeDependenceInfoRequest>()) {
-      continue;
-    }
+  // Perform some filtering to avoid emitting duplicate 'through reference here'
+  // notes.
+  DiagnosticQueue cycleDiags(diags, /*emitOnDestruction*/ true);
+  SWIFT_DEFER {
+    auto isDefaultStepNote = [](const Diagnostic &diag) -> bool {
+      return diag.getID() == diag::circular_reference_through.ID;
+    };
+    // First populate seen locs for everything except default step notes. If
+    // we have a diagnostic or custom note at a given location we want to prefer
+    // that over the default step note.
+    llvm::DenseSet<SourceLoc> seenLocs;
+    cycleDiags.forEach([&](const Diagnostic &diag) {
+      if (isDefaultStepNote(diag))
+        return;
+
+      if (auto loc = diag.getLocOrDeclLoc())
+        seenLocs.insert(loc);
+    });
+    // Then we can filter out unnecessary default step notes.
+    cycleDiags.filter([&](const Diagnostic &diag) -> bool {
+      if (!isDefaultStepNote(diag))
+        return true;
+
+      auto loc = diag.getLocOrDeclLoc();
+      return loc && seenLocs.insert(loc).second;
+    });
+  };
+
+  request.diagnoseCycle(cycleDiags.getDiags());
 
-    step.noteCycleStep(diags);
+  for (const auto &step : llvm::reverse(activeRequests)) {
+    if (step == request)
+      return;
+    step.noteCycleStep(cycleDiags.getDiags());
   }
 
   llvm_unreachable("Diagnosed a cycle but it wasn't represented in the stack");

test/CircularReferences/global_typealias.swift

@@ -1,6 +1,6 @@
 // RUN: %target-typecheck-verify-swift
 
-typealias A = B // expected-error {{type alias 'A' references itself}} expected-note {{while resolving type 'B'}} expected-note {{through reference here}}
-typealias C = D // expected-note {{through reference here}} expected-note {{while resolving type 'D'}} expected-note {{through reference here}}
-typealias D = (A, Int) // expected-note {{through reference here}} expected-note {{while resolving type '(A, Int)'}} expected-note {{through reference here}}
-typealias B = C // expected-note {{through reference here}} expected-note {{while resolving type 'C'}} expected-note {{through reference here}}
+typealias A = B // expected-error {{type alias 'A' references itself}} expected-note {{while resolving type 'B'}}
+typealias C = D // expected-note {{through reference here}} expected-note {{while resolving type 'D'}}
+typealias D = (A, Int) // expected-note {{through reference here}} expected-note {{while resolving type '(A, Int)'}}
+typealias B = C // expected-note {{through reference here}} expected-note {{while resolving type 'C'}}

test/Generics/issue-69318.swift

@@ -15,6 +15,7 @@ public protocol ChildlessView: View where NodeChildren == EmptyViewGraphNodeChil
 // expected-error@-1 {{circular reference}}
 
 public struct EmptyViewGraphNodeChildren<ParentView: ChildlessView>: ViewGraphNodeChildren {}
-// expected-note@-1 3{{through reference here}}
-// expected-error@-2 {{type 'EmptyViewGraphNodeChildren<ParentView>' does not conform to protocol 'ViewGraphNodeChildren'}}
-// expected-note@-3 {{add stubs for conformance}}
+// expected-note@-1:15 {{through reference here}}
+// expected-note@-2:70 {{through reference here}}
+// expected-error@-3 {{type 'EmptyViewGraphNodeChildren<ParentView>' does not conform to protocol 'ViewGraphNodeChildren'}}
+// expected-note@-4 {{add stubs for conformance}}

test/Generics/rdar123013710.swift

@@ -8,9 +8,9 @@ public protocol P {
     static func f() -> Any?
 }
 
-protocol Q: P where B == Never {} // expected-error {{circular reference}}
+protocol Q: P where B == Never {} // expected-error@:10 {{circular reference}}
 
-extension Never: Q, P { // expected-note 2{{through reference here}}
+extension Never: Q, P { // expected-note@:1 {{through reference here}}
    public typealias A = Never
    public static func f() -> Any? { nil }
 }

test/Generics/rdar94848868.swift

@@ -3,12 +3,13 @@
 // This is too circular to work, but it shouldn't crash.
 
 protocol MyCollectionProtocol: Collection where Iterator == MyCollectionIterator<Self> {}
-// expected-error@-1 {{circular reference}}
+// expected-error@-1:10 {{circular reference}}
 
 struct MyCollectionIterator<MyCollection: MyCollectionProtocol>: IteratorProtocol {
-// expected-note@-1 3{{through reference here}}
-// expected-error@-2 {{type 'MyCollectionIterator<MyCollection>' does not conform to protocol 'IteratorProtocol'}}
-// expected-note@-3 {{add stubs for conformance}}
+  // expected-note@-1:8 {{through reference here}}
+  // expected-note@-2:66 {{through reference here}}
+  // expected-error@-3 {{type 'MyCollectionIterator<MyCollection>' does not conform to protocol 'IteratorProtocol'}}
+  // expected-note@-4 {{add stubs for conformance}}
     mutating func next() -> MyCollection.Element? {
     // expected-error@-1 {{'Element' is not a member type of type 'MyCollection'}}
         return nil

test/NameLookup/name_lookup.swift

@@ -637,17 +637,14 @@ struct PatternBindingWithTwoVars2 { var x = y, y = 3 }
 // expected-error@-1 {{cannot use instance member 'y' within property initializer; property initializers run before 'self' is available}}
 
 struct PatternBindingWithTwoVars3 { var x = y, y = x }
-// expected-error@-1 {{circular reference}}
-// expected-note@-2 {{through reference here}}
-// expected-note@-3 {{through reference here}}
-// expected-note@-4 {{through reference here}}
-// expected-note@-5 {{through reference here}}
-// expected-note@-6 {{through reference here}}
+// expected-error@-1:41 {{circular reference}}
+// expected-note@-2:37 {{through reference here}}
+// expected-note@-3:48 {{through reference here}}
 
 // https://github.com/apple/swift/issues/51518
 do {
-  let closure1 = { closure2() } // expected-error {{circular reference}} expected-note {{through reference here}} expected-note {{through reference here}}
-  let closure2 = { closure1() } // expected-note {{through reference here}} expected-note {{through reference here}} expected-note {{through reference here}}
+  let closure1 = { closure2() } // expected-error {{circular reference}} expected-note {{through reference here}}
+  let closure2 = { closure1() } // expected-note {{through reference here}} expected-note {{through reference here}}
 }
 
 func color(with value: Int) -> Int {

test/Sema/diag_typealias.swift

@@ -2,4 +2,4 @@
 
 struct S {}
 
-typealias S = S // expected-error {{type alias 'S' references itself}} expected-note {{while resolving type 'S'}} expected-note {{through reference here}}
+typealias S = S // expected-error {{type alias 'S' references itself}} expected-note {{while resolving type 'S'}}

test/decl/circularity.swift

@@ -68,12 +68,11 @@ class C1 {
     func run(a: Int) {}
 }
 
-class C2: C1, P {
-    // expected-note@-1 2{{through reference here}}
+class C2: C1, P { // expected-note@:7 {{through reference here}}
     override func run(a: A) {}
-    // expected-error@-1 {{circular reference}}
-    // expected-note@-2 {{while resolving type 'A'}}
-    // expected-note@-3 2{{through reference here}}
+    // expected-error@-1:19 {{circular reference}}
+    // expected-note@-2:26 {{while resolving type 'A'}}
+    // expected-note@-3:23 {{through reference here}}
 }
 
 // Another crash to the above
@@ -84,12 +83,12 @@ open class G1<A> {
 class C3: G1<A>, P {
     // expected-error@-1 {{type 'C3' does not conform to protocol 'P'}}
     // expected-error@-2 {{cannot find type 'A' in scope}}
-    // expected-note@-3 2{{through reference here}}
+    // expected-note@-3:7 {{through reference here}}
     // expected-note@-4 {{add stubs for conformance}}
     override func run(a: A) {}
-    // expected-error@-1 {{circular reference}}
-    // expected-note@-2 2 {{through reference here}}
-    // expected-note@-3 {{while resolving type 'A'}}
+    // expected-error@-1:19 {{circular reference}}
+    // expected-note@-2:26 {{while resolving type 'A'}}
+    // expected-note@-3:23 {{through reference here}}
 }
 
 // Another case that triggers circular override checking.
@@ -102,10 +101,10 @@ class C4 {
   required init(x: Int) {}
 }
 
-class D4 : C4, P1 { // expected-note 4 {{through reference here}}
-  required init(x: X) { // expected-error {{circular reference}}
-    // expected-note@-1 {{while resolving type 'X'}}
-    // expected-note@-2 2{{through reference here}}
+class D4 : C4, P1 { // expected-note@:7 {{through reference here}}
+  required init(x: X) { // expected-error@:12 {{circular reference}}
+    // expected-note@-1:20 {{while resolving type 'X'}}
+    // expected-note@-2:17 {{through reference here}}
     super.init(x: x)
   }
 }
@@ -114,6 +113,6 @@ class D4 : C4, P1 { // expected-note 4 {{through reference here}}
 // N.B. This used to compile in 5.1.
 protocol P_54662 { }
 class C_54662 { // expected-note {{through reference here}}
-    typealias Nest = P_54662 // expected-error {{circular reference}} expected-note {{through reference here}}
+    typealias Nest = P_54662 // expected-error {{circular reference}}
 }
 extension C_54662: C_54662.Nest { }