Merge pull request #85528 from hnrklssn/diag-verify-fixes

[DiagnosticVerifier] improve output for expansions in clang attributes

Author: hnrklssn

lib/ClangImporter/ImportDecl.cpp

@@ -8786,7 +8786,9 @@ SourceFile &ClangImporter::Implementation::getClangSwiftAttrSourceFile(
   auto bufferID = sourceMgr.addMemBufferCopy(attributeText);
 
   auto info = GeneratedSourceInfo{GeneratedSourceInfo::AttributeFromClang,
-                                  CharSourceRange(),
+                                  // NB: This source range is not used by the diagnostic engine,
+                                  // but it is traversed by DiagnostciVerifier.
+                                  CharSourceRange(MappedDecl->getStartLoc(), 0),
                                   sourceMgr.getRangeForBuffer(bufferID)};
   info.astNode = static_cast<void *>(module);
   info.clangNode = MappedDecl->getClangNode();

lib/Frontend/DiagnosticVerifier.cpp

@@ -20,6 +20,7 @@
 #include "swift/Basic/ColorUtils.h"
 #include "swift/Basic/SourceManager.h"
 #include "swift/Parse/Lexer.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FormatVariadic.h"
@@ -386,19 +387,23 @@ static void autoApplyFixes(SourceManager &SM, unsigned BufferID,
 bool DiagnosticVerifier::verifyUnknown(
     std::vector<CapturedDiagnosticInfo> &CapturedDiagnostics) const {
   bool HadError = false;
-  for (unsigned i = 0, e = CapturedDiagnostics.size(); i != e; ++i) {
-    if (CapturedDiagnostics[i].Loc.isValid())
+  auto CapturedDiagIter = CapturedDiagnostics.begin();
+  while (CapturedDiagIter != CapturedDiagnostics.end()) {
+    if (CapturedDiagIter->Loc.isValid()) {
+      ++CapturedDiagIter;
       continue;
+    }
 
     HadError = true;
     std::string Message =
         ("unexpected " +
-         getDiagKindString(CapturedDiagnostics[i].Classification) +
-         " produced: " + CapturedDiagnostics[i].Message)
+         getDiagKindString(CapturedDiagIter->Classification) +
+         " produced: " + CapturedDiagIter->Message)
             .str();
 
     auto diag = SM.GetMessage({}, llvm::SourceMgr::DK_Error, Message, {}, {});
     printDiagnostic(diag);
+    CapturedDiagIter = CapturedDiagnostics.erase(CapturedDiagIter);
   }
 
   if (HadError) {
@@ -414,23 +419,35 @@ bool DiagnosticVerifier::verifyUnknown(
 bool DiagnosticVerifier::verifyUnrelated(
     std::vector<CapturedDiagnosticInfo> &CapturedDiagnostics) const {
   bool HadError = false;
-  for (unsigned i = 0, e = CapturedDiagnostics.size(); i != e; ++i) {
-    SourceLoc Loc = CapturedDiagnostics[i].Loc;
-    if (!Loc.isValid())
+  auto CapturedDiagIter = CapturedDiagnostics.begin();
+  while (CapturedDiagIter != CapturedDiagnostics.end()) {
+    SourceLoc Loc = CapturedDiagIter->Loc;
+    if (!Loc.isValid()) {
+      ++CapturedDiagIter;
       // checked by verifyUnknown
       continue;
+    }
 
     HadError = true;
     std::string Message =
         ("unexpected " +
-         getDiagKindString(CapturedDiagnostics[i].Classification) +
-         " produced: " + CapturedDiagnostics[i].Message)
+         getDiagKindString(CapturedDiagIter->Classification) +
+         " produced: " + CapturedDiagIter->Message)
             .str();
 
     auto diag = SM.GetMessage(Loc, llvm::SourceMgr::DK_Error, Message, {}, {});
     printDiagnostic(diag);
 
-    auto FileName = SM.getIdentifierForBuffer(SM.findBufferContainingLoc(Loc));
+    unsigned TopmostBufferID = SM.findBufferContainingLoc(Loc);
+    while (const GeneratedSourceInfo *GSI =
+               SM.getGeneratedSourceInfo(TopmostBufferID)) {
+      SourceLoc ParentLoc = GSI->originalSourceRange.getStart();
+      if (ParentLoc.isInvalid())
+        break;
+      TopmostBufferID = SM.findBufferContainingLoc(ParentLoc);
+      Loc = ParentLoc;
+    }
+    auto FileName = SM.getIdentifierForBuffer(TopmostBufferID);
     auto noteDiag =
         SM.GetMessage(Loc, llvm::SourceMgr::DK_Note,
                       ("file '" + FileName +
@@ -441,6 +458,7 @@ bool DiagnosticVerifier::verifyUnrelated(
                        "ignore diagnostics in this file"),
                       {}, {});
     printDiagnostic(noteDiag);
+    CapturedDiagIter = CapturedDiagnostics.erase(CapturedDiagIter);
   }
 
   return HadError;
@@ -476,7 +494,14 @@ void DiagnosticVerifier::printDiagnostic(const llvm::SMDiagnostic &Diag) const {
   ColoredStream coloredStream{stream};
   raw_ostream &out = UseColor ? coloredStream : stream;
   llvm::SourceMgr &Underlying = SM.getLLVMSourceMgr();
-  Underlying.PrintMessage(out, Diag);
+  if (Diag.getFilename().empty()) {
+    llvm::SMDiagnostic SubstDiag(
+        *Diag.getSourceMgr(), Diag.getLoc(), "<empty-filename>",
+        Diag.getLineNo(), Diag.getColumnNo(), Diag.getKind(), Diag.getMessage(),
+        Diag.getLineContents(), Diag.getRanges(), Diag.getFixIts());
+    Underlying.PrintMessage(out, SubstDiag);
+  } else
+    Underlying.PrintMessage(out, Diag);
 
   SourceLoc Loc = SourceLoc::getFromPointer(Diag.getLoc().getPointer());
   if (Loc.isInvalid())
@@ -1402,9 +1427,9 @@ DiagnosticVerifier::Result DiagnosticVerifier::verifyFile(unsigned BufferID) {
 
       // Diagnostics attached to generated sources originating in this
       // buffer also count as part of this buffer for this purpose.
-      const GeneratedSourceInfo *GSI =
-          SM.getGeneratedSourceInfo(CapturedDiagIter->SourceBufferID.value());
-      if (!GSI || llvm::find(GSI->ancestors, BufferID) == GSI->ancestors.end()) {
+      unsigned scratch;
+      llvm::ArrayRef<unsigned> ancestors = SM.getAncestors(CapturedDiagIter->SourceBufferID.value(), scratch);
+      if (llvm::find(ancestors, BufferID) == ancestors.end()) {
         ++CapturedDiagIter;
         continue;
       }

test/Frontend/DiagnosticVerifier/broken-c-module.swift

@@ -6,11 +6,9 @@
 // RUN: not %target-typecheck-verify-swift -I %S/Inputs/broken-c-module 2>&1 | %FileCheck %s --implicit-check-not error: --implicit-check-not note: --implicit-check-not warning:
 
 // CHECK: <unknown>:0: error: fatal error encountered while in -verify mode
-// CHECK: [[@LINE+7]]:8: error: unexpected error produced: could not build
+// CHECK: [[@LINE+5]]:8: error: unexpected error produced: could not build
 // CHECK: error: unexpected note produced: in file included from <module-includes>:1:
 // CHECK: note: file '<module-includes>' is not parsed for 'expected' statements. Use '-verify-additional-file <module-includes>' to enable, or '-verify-ignore-unrelated' to ignore diagnostics in this file
 // CHECK: error: unexpected error produced: expected function body after function declarator
 // CHECK: note: file '{{.*}}broken_c.h' is not parsed for 'expected' statements. Use '-verify-additional-file {{.*}}broken_c.h' to enable, or '-verify-ignore-unrelated' to ignore diagnostics in this file
-// CHECK: note: diagnostic produced elsewhere: in file included from <module-includes>
-// CHECK: broken_c.h:2:11: error: diagnostic produced elsewhere: expected function body after function declarator
 import BrokenCModule

test/Frontend/DiagnosticVerifier/clang-attribute.swift

@@ -0,0 +1,62 @@
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t
+
+// RUN: not %target-swift-frontend -typecheck -verify %t/test.swift -I %t -plugin-path %swift-plugin-dir -Rmacro-expansions 2>&1 | \
+// RUN: %FileCheck %s --implicit-check-not error: --implicit-check-not note: --implicit-check-not warning: --implicit-check-not remark: --match-full-lines --sanitize TEST_H=%t%{fs-sep}test.h
+
+//--- test.h
+void foo(int len, int *p) __attribute__((
+    swift_attr("@_SwiftifyImport(.countedBy(pointer: .param(2), count: \"len\"))")));
+
+//--- test.swift
+import TestClang
+
+func bar(x: UnsafeMutableBufferPointer<CInt>) {
+  foo(x) // trigger macro expansion
+}
+
+//--- module.modulemap
+module TestClang {
+  header "test.h"
+  export *
+}
+
+// CHECK: @__swiftmacro_So3foo15_SwiftifyImportfMp_.swift:1:1: error: unexpected remark produced: macro content: |/// This is an auto-generated wrapper for safer interop|
+// CHECK: TEST_H:1:25: note: in expansion from here
+// CHECK: TEST_H:1:25: note: file 'TEST_H' is not parsed for 'expected' statements. Use '-verify-additional-file TEST_H' to enable, or '-verify-ignore-unrelated' to ignore diagnostics in this file
+// CHECK: <empty-filename>:1:1: error: unexpected note produced: in expansion of macro '_SwiftifyImport' on global function 'foo' here
+//  // CHECK-NEXT: @_SwiftifyImport(.countedBy(pointer: .param(2), count: "len"))
+// CHECK: TEST_H:1:6: note: in expansion from here
+// CHECK: TEST_H:1:6: note: file 'TEST_H' is not parsed for 'expected' statements. Use '-verify-additional-file TEST_H' to enable, or '-verify-ignore-unrelated' to ignore diagnostics in this file 
+
+// CHECK: @__swiftmacro_So3foo15_SwiftifyImportfMp_.swift:2:1: error: unexpected remark produced: macro content: |@_alwaysEmitIntoClient @_disfavoredOverload public func foo(_ p: UnsafeMutableBufferPointer<Int32>) {|
+// CHECK: TEST_H:1:25: note: in expansion from here
+// CHECK: TEST_H:1:25: note: file 'TEST_H' is not parsed for 'expected' statements. Use '-verify-additional-file TEST_H' to enable, or '-verify-ignore-unrelated' to ignore diagnostics in this file
+// CHECK: <empty-filename>:1:1: error: unexpected note produced: in expansion of macro '_SwiftifyImport' on global function 'foo' here
+  // CHECK-NEXT: @_SwiftifyImport(.countedBy(pointer: .param(2), count: "len"))
+// CHECK: TEST_H:1:6: note: in expansion from here
+// CHECK: TEST_H:1:6: note: file 'TEST_H' is not parsed for 'expected' statements. Use '-verify-additional-file TEST_H' to enable, or '-verify-ignore-unrelated' to ignore diagnostics in this file 
+
+// CHECK: @__swiftmacro_So3foo15_SwiftifyImportfMp_.swift:3:1: error: unexpected remark produced: macro content: |    let len = Int32(exactly: p.count)!|
+// CHECK: TEST_H:1:25: note: in expansion from here
+// CHECK: TEST_H:1:25: note: file 'TEST_H' is not parsed for 'expected' statements. Use '-verify-additional-file TEST_H' to enable, or '-verify-ignore-unrelated' to ignore diagnostics in this file
+// CHECK: <empty-filename>:1:1: error: unexpected note produced: in expansion of macro '_SwiftifyImport' on global function 'foo' here
+  // CHECK-NEXT: @_SwiftifyImport(.countedBy(pointer: .param(2), count: "len"))
+// CHECK: TEST_H:1:6: note: in expansion from here
+// CHECK: TEST_H:1:6: note: file 'TEST_H' is not parsed for 'expected' statements. Use '-verify-additional-file TEST_H' to enable, or '-verify-ignore-unrelated' to ignore diagnostics in this file 
+
+// CHECK: @__swiftmacro_So3foo15_SwiftifyImportfMp_.swift:4:1: error: unexpected remark produced: macro content: |    return unsafe foo(len, p.baseAddress!)|
+// CHECK: TEST_H:1:25: note: in expansion from here
+// CHECK: TEST_H:1:25: note: file 'TEST_H' is not parsed for 'expected' statements. Use '-verify-additional-file TEST_H' to enable, or '-verify-ignore-unrelated' to ignore diagnostics in this file
+// CHECK: <empty-filename>:1:1: error: unexpected note produced: in expansion of macro '_SwiftifyImport' on global function 'foo' here
+  // CHECK-NEXT: @_SwiftifyImport(.countedBy(pointer: .param(2), count: "len"))
+// CHECK: TEST_H:1:6: note: in expansion from here
+// CHECK: TEST_H:1:6: note: file 'TEST_H' is not parsed for 'expected' statements. Use '-verify-additional-file TEST_H' to enable, or '-verify-ignore-unrelated' to ignore diagnostics in this file 
+
+// CHECK: @__swiftmacro_So3foo15_SwiftifyImportfMp_.swift:5:1: error: unexpected remark produced: macro content: |}|
+// CHECK: TEST_H:1:25: note: in expansion from here
+// CHECK: TEST_H:1:25: note: file 'TEST_H' is not parsed for 'expected' statements. Use '-verify-additional-file TEST_H' to enable, or '-verify-ignore-unrelated' to ignore diagnostics in this file
+// CHECK: <empty-filename>:1:1: error: unexpected note produced: in expansion of macro '_SwiftifyImport' on global function 'foo' here
+  // CHECK-NEXT: @_SwiftifyImport(.countedBy(pointer: .param(2), count: "len"))
+// CHECK: TEST_H:1:6: note: in expansion from here
+// CHECK: TEST_H:1:6: note: file 'TEST_H' is not parsed for 'expected' statements. Use '-verify-additional-file TEST_H' to enable, or '-verify-ignore-unrelated' to ignore diagnostics in this file 

test/Frontend/DiagnosticVerifier/verify.swift

@@ -34,4 +34,3 @@ fn(())    // expected-error {{argument passed to call that takes no arguments}}
 let x: Array<Int, Int>
 // CHECK: error: unexpected note produced: generic struct 'Array' declared here
 // CHECK: note: file 'Swift.Array' is not parsed for 'expected' statements. Use '-verify-additional-file Swift.Array' to enable, or '-verify-ignore-unrelated' to ignore diagnostics in this file
-// CHECK: note: diagnostic produced elsewhere: generic struct 'Array' declared here

test/Frontend/DiagnosticVerifier/verify2.swift

@@ -44,7 +44,6 @@ extension Crap {} // expected-error {{non-nominal type 'Crap' (aka '() -> ()') c
 
 // CHECK: error: unexpected note produced: 'Bool' declared here
 // CHECK: note: file 'Swift.Bool' is not parsed for 'expected' statements. Use '-verify-additional-file Swift.Bool' to enable, or '-verify-ignore-unrelated' to ignore diagnostics in this file
-// CHECK: note: diagnostic produced elsewhere: 'Bool' declared here
 
 // Verify the serialized diags have the right magic at the top.
 // CHECK-SERIALIZED: DIA

test/Inputs/clang-importer-sdk/usr/include/ObjCConcurrency.h

@@ -233,6 +233,9 @@ MAIN_ACTOR MAIN_ACTOR __attribute__((__swift_attr__("@MainActor"))) @protocol Tr
 
 SENDABLE @interface SendableClass : NSObject @end
 
+// expected-expansion@+3:13{{
+//   expected-note@1 5{{conformance of 'NonSendableClass' to 'Sendable' has been explicitly marked unavailable here}}
+// }}
 NONSENDABLE @interface NonSendableClass : NSObject @end // expected-note {{class 'NonSendableClass' does not conform to the 'Sendable' protocol}}
 
 ASSUME_NONSENDABLE_BEGIN

test/Interop/Cxx/templates/function-template-typechecker-errors.swift

@@ -1,5 +1,4 @@
-// RUN: not %target-typecheck-verify-swift -verify-ignore-unrelated -I %S/Inputs -enable-experimental-cxx-interop 2>&1 | %FileCheck %s
-
+// RUN: not %target-typecheck-verify-swift -verify-ignore-unrelated -verify-ignore-unknown -I %S/Inputs -enable-experimental-cxx-interop 2>&1 | %FileCheck %s
 // README: If you just added support for protocol composition to the
 // ClangTypeConverter, please update this test to use a different type that we
 // don't support so the error messages here are still tested.

test/Macros/accessor_macros.swift

@@ -102,6 +102,7 @@ struct MyBrokenStruct {
     /*
     expected-expansion@-2:25{{
       expected-error@1:1{{variable already has a getter}}
+      expected-error@5:1{{variable already has a setter}}
     }}
     */
     // CHECK-DIAGS: variable already has a getter

test/Macros/expand_peers_hang.swift

@@ -11,6 +11,10 @@ macro HangingMacro() = #externalMacro(module: "MacroDefinition", type: "HangingM
 class Foo {
   init() {}
 
+  // expected-expansion@+5:32{{
+  //   expected-error@1{{unexpected token '}' in expanded member list}}
+  //   expected-error@2{{expected declaration}}
+  // }}
   // expected-note@+1 2{{in expansion of macro 'HangingMacro' on property 'result' here}}
   @HangingMacro var result: Int // This comment makes it hang.
 }