[Diagnostics] Support diagnostics with no location in the new formatter

include/swift/AST/DiagnosticBridge.h

@@ -22,6 +22,7 @@
 #include "swift/Basic/SourceManager.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/Support/raw_ostream.h"
+#include <optional>
 
 namespace swift {
 /// Declare the bridge between swift-syntax and swift-frontend for diagnostics
@@ -44,7 +45,7 @@ class DiagnosticBridge {
 public:
   /// Enqueue diagnostics.
   void enqueueDiagnostic(SourceManager &SM, const DiagnosticInfo &Info,
-                         unsigned innermostBufferID);
+                         std::optional<unsigned> innermostBufferID);
 
   /// Flush all enqueued diagnostics.
   void flush(llvm::raw_ostream &OS, bool includeTrailingBreak,

lib/AST/DiagnosticBridge.cpp

@@ -87,9 +87,10 @@ static void addQueueDiagnostic(void *queuedDiagnostics,
   }
 }
 
-void DiagnosticBridge::enqueueDiagnostic(SourceManager &SM,
-                                         const DiagnosticInfo &Info,
-                                         unsigned innermostBufferID) {
+void DiagnosticBridge::enqueueDiagnostic(
+    SourceManager &SM,
+    const DiagnosticInfo &Info,
+    std::optional<unsigned> innermostBufferID) {
   // If we didn't have per-frontend state before, create it now.
   if (!perFrontendState) {
     perFrontendState = swift_ASTGen_createPerFrontendDiagnosticState();
@@ -100,7 +101,8 @@ void DiagnosticBridge::enqueueDiagnostic(SourceManager &SM,
   if (!queuedDiagnostics)
     queuedDiagnostics = swift_ASTGen_createQueuedDiagnostics();
 
-  queueBuffer(SM, innermostBufferID);
+  if (innermostBufferID)
+    queueBuffer(SM, *innermostBufferID);
   addQueueDiagnostic(queuedDiagnostics, perFrontendState, Info, SM);
 }
 

lib/ASTGen/Sources/ASTGen/DiagnosticsBridge.swift

@@ -25,15 +25,19 @@ fileprivate func emitDiagnosticParts(
   sourceFileBuffer: UnsafeBufferPointer<UInt8>,
   message: String,
   severity: DiagnosticSeverity,
-  position: AbsolutePosition,
+  position: AbsolutePosition?,
   offset: Int,
   highlights: [Syntax] = [],
   edits: [SourceEdit] = []
 ) {
   // Map severity
   let bridgedSeverity = severity.bridged
 
-  func bridgedSourceLoc(at position: AbsolutePosition) -> BridgedSourceLoc {
+  func bridgedSourceLoc(at position: AbsolutePosition?) -> BridgedSourceLoc {
+    guard let position else {
+      return nil
+    }
+
     return BridgedSourceLoc(at: position.advanced(by: offset), in: sourceFileBuffer)
   }
 
@@ -266,95 +270,125 @@ public func addQueuedDiagnostic(
     to: PerFrontendDiagnosticState.self
   )
 
-  guard let rawPosition = cLoc.getOpaquePointerValue() else {
-    return
-  }
+  let rawPosition = cLoc.getOpaquePointerValue()
+  var sourceFile: ExportedSourceFile? = nil
+  if let rawPosition {
+    sourceFile = queuedDiagnostics.pointee.sourceFiles.first { sf in
+      guard let baseAddress = sf.buffer.baseAddress else {
+        return false
+      }
 
-  // Find the source file that contains this location.
-  let sourceFile = queuedDiagnostics.pointee.sourceFiles.first { sf in
-    guard let baseAddress = sf.buffer.baseAddress else {
-      return false
+      return rawPosition >= baseAddress && rawPosition <= baseAddress + sf.buffer.count
     }
-
-    return rawPosition >= baseAddress && rawPosition <= baseAddress + sf.buffer.count
-  }
-  guard let sourceFile = sourceFile else {
-    // FIXME: Hard to report an error here...
-    return
   }
 
-  let sourceFileBaseAddress = UnsafeRawPointer(sourceFile.buffer.baseAddress!)
-  let sourceFileEndAddress = sourceFileBaseAddress + sourceFile.buffer.count
-  let offset = rawPosition - sourceFileBaseAddress
-  let position = AbsolutePosition(utf8Offset: offset)
-
-  // Find the token at that offset.
-  let node: Syntax
-  if let token = sourceFile.syntax.token(at: position) {
-    node = Syntax(token)
-  } else if position == sourceFile.syntax.endPosition {
-    // FIXME: EOF token is not included in '.token(at: position)'
-    // We might want to include it, but want to avoid special handling.
-    // Also 'sourceFile.syntax' is not guaranteed to be 'SourceFileSyntax'.
-    if let token = sourceFile.syntax.lastToken(viewMode: .all) {
+  let node: Syntax?
+  let position: AbsolutePosition?
+  var highlights: [Syntax] = []
+  let fixIts: [FixIt]
+  if let sourceFile, let rawPosition {
+    let sourceFileBaseAddress = UnsafeRawPointer(sourceFile.buffer.baseAddress!)
+    let sourceFileEndAddress = sourceFileBaseAddress + sourceFile.buffer.count
+    let offset = rawPosition - sourceFileBaseAddress
+    let myPosition = AbsolutePosition(utf8Offset: offset)
+    position = myPosition
+
+    // Find the token at that offset.
+    if let token = sourceFile.syntax.token(at: myPosition) {
       node = Syntax(token)
+    } else if myPosition == sourceFile.syntax.endPosition {
+      // FIXME: EOF token is not included in '.token(at: position)'
+      // We might want to include it, but want to avoid special handling.
+      // Also 'sourceFile.syntax' is not guaranteed to be 'SourceFileSyntax'.
+      if let token = sourceFile.syntax.lastToken(viewMode: .all) {
+        node = Syntax(token)
+      } else {
+        node = sourceFile.syntax
+      }
     } else {
-      node = sourceFile.syntax
+      node = nil
     }
-  } else {
-    // position out of range.
-    return
-  }
 
-  // Map the highlights.
-  var highlights: [Syntax] = []
-  let highlightRanges = UnsafeBufferPointer<BridgedCharSourceRange>(
-    start: highlightRangesPtr,
-    count: numHighlightRanges
-  )
-  for index in 0..<numHighlightRanges {
-    let range = highlightRanges[index]
-
-    // Make sure both the start and the end land within this source file.
-    guard let start = range.start.getOpaquePointerValue(),
-      let end = range.start.advanced(by: range.byteLength).getOpaquePointerValue()
-    else {
-      continue
-    }
+    // Map the highlights.
+    let highlightRanges = UnsafeBufferPointer<BridgedCharSourceRange>(
+      start: highlightRangesPtr,
+      count: numHighlightRanges
+    )
+    for index in 0..<numHighlightRanges {
+      let range = highlightRanges[index]
 
-    guard start >= sourceFileBaseAddress && start < sourceFileEndAddress,
-      end >= sourceFileBaseAddress && end <= sourceFileEndAddress
-    else {
-      continue
-    }
+      // Make sure both the start and the end land within this source file.
+      guard let start = range.start.getOpaquePointerValue(),
+        let end = range.start.advanced(by: range.byteLength).getOpaquePointerValue()
+      else {
+        continue
+      }
 
-    // Find start tokens in the source file.
-    let startPos = AbsolutePosition(utf8Offset: start - sourceFileBaseAddress)
-    guard let startToken = sourceFile.syntax.token(at: startPos) else {
-      continue
-    }
+      guard start >= sourceFileBaseAddress && start < sourceFileEndAddress,
+        end >= sourceFileBaseAddress && end <= sourceFileEndAddress
+      else {
+        continue
+      }
 
-    // Walk up from the start token until we find a syntax node that matches
-    // the highlight range.
-    let endPos = AbsolutePosition(utf8Offset: end - sourceFileBaseAddress)
-    var highlightSyntax = Syntax(startToken)
-    while true {
-      // If this syntax matches our starting/ending positions, add the
-      // highlight and we're done.
-      if highlightSyntax.positionAfterSkippingLeadingTrivia == startPos
-        && highlightSyntax.endPositionBeforeTrailingTrivia == endPos
-      {
-        highlights.append(highlightSyntax)
-        break
+      // Find start tokens in the source file.
+      let startPos = AbsolutePosition(utf8Offset: start - sourceFileBaseAddress)
+      guard let startToken = sourceFile.syntax.token(at: startPos) else {
+        continue
       }
 
-      // Go up to the parent.
-      guard let parent = highlightSyntax.parent else {
-        break
+      // Walk up from the start token until we find a syntax node that matches
+      // the highlight range.
+      let endPos = AbsolutePosition(utf8Offset: end - sourceFileBaseAddress)
+      var highlightSyntax = Syntax(startToken)
+      while true {
+        // If this syntax matches our starting/ending positions, add the
+        // highlight and we're done.
+        if highlightSyntax.positionAfterSkippingLeadingTrivia == startPos
+          && highlightSyntax.endPositionBeforeTrailingTrivia == endPos
+        {
+          highlights.append(highlightSyntax)
+          break
+        }
+
+        // Go up to the parent.
+        guard let parent = highlightSyntax.parent else {
+          break
+        }
+
+        highlightSyntax = parent
       }
+    }
 
-      highlightSyntax = parent
+    // Map the Fix-Its
+    let fixItChanges: [FixIt.Change] = fixItsUntyped.withElements(ofType: BridgedFixIt.self) { fixIts in
+      fixIts.compactMap { fixIt in
+        guard let startPos = sourceFile.position(of: fixIt.replacementRange.start),
+              let endPos = sourceFile.position(
+                of: fixIt.replacementRange.start.advanced(
+                  by: fixIt.replacementRange.byteLength)) else {
+          return nil
+        }
+
+        return FixIt.Change.replaceText(
+          range: startPos..<endPos,
+          with: String(bridged: fixIt.replacementText),
+          in: sourceFile.syntax
+        )
+      }
     }
+
+    fixIts = fixItChanges.isEmpty
+        ? []
+        : [
+            FixIt(
+              message: BridgedFixItMessage(),
+              changes: fixItChanges
+            )
+          ]
+  } else {
+    node = nil
+    position = nil
+    fixIts = []
   }
 
   let documentationPath = String(bridged: documentationPath)
@@ -383,33 +417,6 @@ public func addQueuedDiagnostic(
     diagnosticState.pointee.referencedCategories.insert(category)
   }
 
-  // Map the Fix-Its
-  let fixItChanges: [FixIt.Change] = fixItsUntyped.withElements(ofType: BridgedFixIt.self) { fixIts in
-    fixIts.compactMap { fixIt in
-      guard let startPos = sourceFile.position(of: fixIt.replacementRange.start),
-            let endPos = sourceFile.position(
-              of: fixIt.replacementRange.start.advanced(
-                by: fixIt.replacementRange.byteLength)) else {
-        return nil
-      }
-
-      return FixIt.Change.replaceText(
-        range: startPos..<endPos,
-        with: String(bridged: fixIt.replacementText),
-        in: sourceFile.syntax
-      )
-    }
-  }
-
-  let fixIts: [FixIt] = fixItChanges.isEmpty
-      ? []
-      : [
-          FixIt(
-            message: BridgedFixItMessage(),
-            changes: fixItChanges
-          )
-        ]
-
   let diagnostic = Diagnostic(
     node: node,
     position: position,

lib/ASTGen/Sources/ASTGen/SourceFile.swift

@@ -219,7 +219,8 @@ public func emitParserDiagnostics(
     let configuredRegions = sourceFile.pointee.configuredRegions(astContext: ctx)
     for diag in diags {
       // If the diagnostic is in an unparsed #if region, don't emit it.
-      if configuredRegions.isActive(diag.node) == .unparsed {
+      if let diagNode = diag.node,
+          configuredRegions.isActive(diagNode) == .unparsed {
         continue
       }
 

lib/ASTGen/Sources/MacroEvaluation/SourceManager.swift

@@ -99,9 +99,13 @@ extension SourceManager {
   /// Produce the C++ source location for a given position based on a
   /// syntax node.
   func bridgedSourceLoc<Node: SyntaxProtocol>(
-    for node: Node,
+    for node: Node?,
     at position: AbsolutePosition? = nil
   ) -> BridgedSourceLoc {
+    guard let node else {
+      return nil
+    }
+
     // Find the source file and this node's position within it.
     let (rootNode, rootPosition) = rootSyntax(of: node)
 
@@ -124,8 +128,8 @@ extension SourceManager {
   private func diagnoseSingle<Node: SyntaxProtocol>(
     message: String,
     severity: DiagnosticSeverity,
-    node: Node,
-    position: AbsolutePosition,
+    node: Node?,
+    position: AbsolutePosition?,
     highlights: [Syntax] = [],
     fixItChanges: [FixIt.Change] = []
   ) {

lib/Frontend/PrintingDiagnosticConsumer.cpp

@@ -47,21 +47,31 @@ void PrintingDiagnosticConsumer::handleDiagnostic(SourceManager &SM,
   case DiagnosticOptions::FormattingStyle::Swift: {
 #if SWIFT_BUILD_SWIFT_SYNTAX
     // Use the swift-syntax formatter.
+    if (Info.Kind != DiagnosticKind::Note) {
+      DiagBridge.flush(Stream, /*includeTrailingBreak=*/true,
+                       /*forceColors=*/ForceColors);
+    }
+
     auto bufferStack = DiagnosticBridge::getSourceBufferStack(SM, Info.Loc);
+    std::optional<unsigned> innermostBufferID;
     if (!bufferStack.empty()) {
-      if (Info.Kind != DiagnosticKind::Note)
-        DiagBridge.flush(Stream, /*includeTrailingBreak=*/true,
-                         /*forceColors=*/ForceColors);
+      innermostBufferID = bufferStack.front();
+    }
+
+    DiagBridge.enqueueDiagnostic(SM, Info, innermostBufferID);
 
-      DiagBridge.enqueueDiagnostic(SM, Info, bufferStack.front());
-      break;
+    // If there was no source-location information, immediately flush the
+    // enqueued diagnostic. It won't ever be grouped with anything.
+    if (bufferStack.empty()) {
+      DiagBridge.flush(Stream, /*includeTrailingBreak=*/true,
+                       /*forceColors=*/ForceColors);
     }
-#endif
 
+    break;
+#else
     // Fall through when we don't have the new diagnostics renderer available.
-    // This also happens if the location of the diagnostic is invalid, because
-    // the new rendered cannot cope with that.
     LLVM_FALLTHROUGH;
+#endif
   }
 
   case DiagnosticOptions::FormattingStyle::LLVM:

test/Unsafe/unsafe_command_line.swift

@@ -1,4 +1,4 @@
 // RUN: %target-swift-frontend -typecheck -strict-memory-safety -Ounchecked -disable-access-control %s 2>&1 | %FileCheck %s
 
 // CHECK: warning: '-Ounchecked' is not memory-safe
-// CHECK: warning: '-disable-access-control' is not memory-safe
+// CHECK: warning: '-disable-access-control' is not memory-safe and should not be combined with strict memory safety checking [#StrictMemorySafety]