Merge pull request #65472 from hamishknight/buffering

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());

lib/Frontend/SerializedDiagnosticConsumer.cpp

@@ -231,6 +231,8 @@ unsigned SerializedDiagnosticConsumer::getEmitFile(
   // NOTE: Using Filename.data() here relies on SourceMgr using
   // const char* as buffer identifiers.  This is fast, but may
   // be brittle.  We can always switch over to using a StringMap.
+  // Note that the logic in EditorDiagConsumer::getBufferInfo
+  // will also need changing.
   unsigned &existingEntry = State->Files[Filename.data()];
   if (existingEntry)
     return existingEntry;

test/Macros/Inputs/syntax_macro_definitions.swift

@@ -916,6 +916,15 @@ public struct InvalidMacro: PeerMacro, DeclarationMacro {
   }
 }
 
+public struct CoerceToIntMacro: ExpressionMacro {
+  public static func expansion(
+    of node: some FreestandingMacroExpansionSyntax,
+    in context: some MacroExpansionContext
+  ) -> ExprSyntax {
+    "\(node.argumentList.first!.expression) as Int"
+  }
+}
+
 public struct WrapInType: PeerMacro {
   public static func expansion(
     of node: AttributeSyntax,

test/Macros/macro_expand.swift

@@ -188,7 +188,7 @@ func testDiscardableStringify(x: Int) {
 func testNested() {
   struct Nested { }
   _ = #stringify(#assertAny(Nested()))
-  // expected-note@-1 2 {{in expansion of macro 'stringify' here}}
+  // expected-note@-1 {{in expansion of macro 'stringify' here}}
 // CHECK-DIAGS-NOT: error: cannot convert value of type 'Nested' to expected argument type 'Bool'
 // CHECK-DIAGS: @__swiftmacro_9MacroUser10testNestedyyF9stringifyfMf_9assertAnyfMf_.swift:1:8: error: cannot convert value of type 'Nested' to expected argument type 'Bool'
 // CHECK-DIAGS-NOT: error: cannot convert value of type 'Nested' to expected argument type 'Bool'

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/Diagnostics/diag_in_c_header.swift

@@ -0,0 +1,44 @@
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t
+// RUN: %sourcekitd-test -req=diags %t/main.swift -- %t/main.swift -I %t | %FileCheck %s
+
+//--- Header.h
+#define FOO(x) #x
+
+//--- module.modulemap
+module HeaderWithMacro {
+    header "Header.h"
+}
+
+//--- main.swift
+import HeaderWithMacro
+
+_ = FOO(5)
+
+// rdar://107281079 – Make sure the note points in the .h file
+// CHECK:      key.diagnostics: [
+// CHECK-NEXT:   {
+// CHECK-NEXT:     key.line: 3,
+// CHECK-NEXT:     key.column: 5,
+// CHECK-NEXT:     key.filepath: "{{.*}}main.swift",
+// CHECK-NEXT:     key.severity: source.diagnostic.severity.error,
+// CHECK-NEXT:     key.id: "cannot_find_in_scope",
+// CHECK-NEXT:     key.description: "cannot find 'FOO' in scope",
+// CHECK-NEXT:     key.ranges: [
+// CHECK-NEXT:       {
+// CHECK-NEXT:         key.offset: 28,
+// CHECK-NEXT:         key.length: 3
+// CHECK-NEXT:       }
+// CHECK-NEXT:     ],
+// CHECK-NEXT:     key.diagnostics: [
+// CHECK-NEXT:       {
+// CHECK-NEXT:         key.line: 1,
+// CHECK-NEXT:         key.column: 9,
+// CHECK-NEXT:         key.filepath: "{{.*}}Header.h",
+// CHECK-NEXT:         key.severity: source.diagnostic.severity.note,
+// CHECK-NEXT:         key.id: "macro_not_imported_function_like",
+// CHECK-NEXT:         key.description: "macro 'FOO' unavailable: function like macros not supported"
+// CHECK-NEXT:       }
+// CHECK-NEXT:     ]
+// CHECK-NEXT:   }
+// CHECK-NEXT: ]

test/SourceKit/Inputs/sourcekitd_path_sanitize.py

@@ -12,21 +12,35 @@
 import re
 import sys
 
-SWIFTMODULE_BUNDLE_RE = re.compile(
-    r'key.filepath: ".*[/\\](.*)\.swiftmodule[/\\].*\.swiftmodule"')
-SWIFTMODULE_RE = re.compile(r'key.filepath: ".*[/\\](.*)\.swiftmodule"')
-SWIFT_RE = re.compile(r'key.filepath: ".*[/\\](.*)\.swift"')
-PCM_RE = re.compile(r'key.filepath: ".*[/\\](.*)-[0-9A-Z]*\.pcm"')
-HEADER_RE = re.compile(r' file=\\".*[/\\](.*)\.h\\"')
+# I'm sorry dear reader, unfortunately my knowledge of regex trumps my knowledge of
+# Python. This pattern allows us to clean up file paths by stripping them down to
+# just the relevant file name.
+RE = re.compile(
+    # The key can either be 'filepath' or 'buffer_name'. Also apply this logic to
+    # the file name in XML, which is written 'file=\"...\"'.
+    r'(key\.(?:filepath|buffer_name): |file=)'
+
+    # Open delimiter with optional escape.
+    r'\\?"'
+
+    # Lazily match characters until we hit a slash, then match any non-slash that
+    # ends in the right file extension, capturing the result.
+    r'.*?[/\\]+([^/\\]*\.(?:swiftmodule|swift|pcm|h))'
+
+    # For swiftmodule bundles, we want to match against the directory name, so
+    # optionally match the .swiftmodule filename here. The lazy matching of the
+    # previous logic means we'll prefer to match the previous '.swiftmodule' as the
+    # directory.
+    r'(?:[/\\]+[^/\\]*\.swiftmodule)?'
+
+    # Close delimiter with optional escape.
+    r'\\?"'
+)
 
 try:
     for line in sys.stdin.readlines():
-        line = re.sub(SWIFTMODULE_BUNDLE_RE,
-                      r'key.filepath: \1.swiftmodule', line)
-        line = re.sub(SWIFTMODULE_RE, r'key.filepath: \1.swiftmodule', line)
-        line = re.sub(SWIFT_RE, r'key.filepath: \1.swift', line)
-        line = re.sub(PCM_RE, r'key.filepath: \1.pcm', line)
-        line = re.sub(HEADER_RE, r' file=\1.h', line)
+        # We substitute in both the key and the matched filename.
+        line = re.sub(RE, r'\1\2', line)
         sys.stdout.write(line)
 except KeyboardInterrupt:
     sys.stdout.flush()

test/SourceKit/Macros/diags.swift

@@ -0,0 +1,27 @@
+@freestanding(expression)
+macro coerceToInt<T>(_: T) -> Int = #externalMacro(module: "MacroDefinition", type: "CoerceToIntMacro")
+
+@freestanding(expression)
+macro stringify<T>(_: T) -> (T, String) = #externalMacro(module: "MacroDefinition", type: "StringifyMacro")
+
+@attached(peer)
+macro Invalid() = #externalMacro(module: "MacroDefinition", type: "InvalidMacro")
+
+func foo() {
+  let _ = #coerceToInt("a")
+  let _ = #coerceToInt("b")
+  let _ = #stringify(#coerceToInt("c"))
+}
+
+@Invalid
+struct Bad {}
+
+// REQUIRES: swift_swift_parser
+
+// RUN: %empty-directory(%t)
+
+//##-- Prepare the macro plugin.
+// RUN: %host-build-swift -swift-version 5 -emit-library -o %t/%target-library-name(MacroDefinition) -module-name=MacroDefinition %S/../../Macros/Inputs/syntax_macro_definitions.swift -g -no-toolchain-stdlib-rpath
+
+// RUN: %sourcekitd-test -req=diags %s -- -swift-version 5 -load-plugin-library %t/%target-library-name(MacroDefinition) -module-name MacroUser %s | %sed_clean > %t.response
+// RUN: %diff -u %s.response %t.response