Avoid computing generated source notes on child notes

This could cause us to double up on the same note
for nested macro expansions, as we'd generate the
note, then when recursing back through the function
we'd compute the same note again. Also remove the
seemingly unused `lastBufferID` param.

Author: hamishknight

include/swift/AST/DiagnosticEngine.h

@@ -1169,8 +1169,7 @@ namespace swift {
 
     /// Retrieve the set of child notes that describe how the generated
     /// source buffer was derived, e.g., a macro expansion backtrace.
-    std::vector<Diagnostic> getGeneratedSourceBufferNotes(
-        SourceLoc loc, Optional<unsigned> &lastBufferID);
+    std::vector<Diagnostic> getGeneratedSourceBufferNotes(SourceLoc loc);
 
     /// Handle a new diagnostic, which will either be emitted, or added to an
     /// active transaction.

lib/AST/DiagnosticEngine.cpp

@@ -1272,9 +1272,8 @@ DiagnosticEngine::diagnosticInfoForDiagnostic(const Diagnostic &diagnostic) {
       diagnostic.isChildNote());
 }
 
-std::vector<Diagnostic> DiagnosticEngine::getGeneratedSourceBufferNotes(
-    SourceLoc loc, Optional<unsigned> &lastBufferID
-) {
+std::vector<Diagnostic>
+DiagnosticEngine::getGeneratedSourceBufferNotes(SourceLoc loc) {
   // The set of child notes we're building up.
   std::vector<Diagnostic> childNotes;
 
@@ -1285,12 +1284,6 @@ std::vector<Diagnostic> DiagnosticEngine::getGeneratedSourceBufferNotes(
   // If we already emitted these notes for a prior part of the diagnostic,
   // don't do so again.
   auto currentBufferID = SourceMgr.findBufferContainingLoc(loc);
-  if (currentBufferID == lastBufferID)
-    return childNotes;
-
-  // Keep track of the last buffer ID we considered.
-  lastBufferID = currentBufferID;
-
   SourceLoc currentLoc = loc;
   do {
     auto generatedInfo = SourceMgr.getGeneratedSourceInfo(currentBufferID);
@@ -1337,15 +1330,18 @@ std::vector<Diagnostic> DiagnosticEngine::getGeneratedSourceBufferNotes(
 }
 
 void DiagnosticEngine::emitDiagnostic(const Diagnostic &diagnostic) {
-  Optional<unsigned> lastBufferID;
 
   ArrayRef<Diagnostic> childNotes = diagnostic.getChildNotes();
   std::vector<Diagnostic> extendedChildNotes;
 
   if (auto info = diagnosticInfoForDiagnostic(diagnostic)) {
     // If the diagnostic location is within a buffer containing generated
     // source code, add child notes showing where the generation occurred.
-    extendedChildNotes = getGeneratedSourceBufferNotes(info->Loc, lastBufferID);
+    // We need to avoid doing this if this is itself a child note, as otherwise
+    // we'd end up doubling up on notes.
+    if (!info->IsChildNote) {
+      extendedChildNotes = getGeneratedSourceBufferNotes(info->Loc);
+    }
     if (!extendedChildNotes.empty()) {
       extendedChildNotes.insert(extendedChildNotes.end(),
                                 childNotes.begin(), childNotes.end());

test/Macros/macro_expand_other.swift

@@ -31,7 +31,7 @@ func testFreestandingExpansionOfOther() {
 
   #if TEST_DIAGNOSTICS
 #recurseThrough(true)
-  // expected-note@-1 2{{in expansion of macro 'recurseThrough' here}}
+  // expected-note@-1 {{in expansion of macro 'recurseThrough' here}}
   #endif
 }
 

test/SourceKit/Macros/diags.swift.response

@@ -84,20 +84,6 @@
             }
           ]
         },
-        {
-          key.line: 17,
-          key.column: 11,
-          key.filepath: diags.swift,
-          key.severity: source.diagnostic.severity.note,
-          key.id: "in_macro_expansion",
-          key.description: "in expansion of macro 'stringify' here",
-          key.ranges: [
-            {
-              key.offset: 674,
-              key.length: 29
-            }
-          ]
-        },
         {
           key.line: 17,
           key.column: 11,