[llvm-branch-commits] [llvm] [Support] Check verify directives in buffers without diagnostics (PR #233307)

[via llvm-branch-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-support

Author: Alexander Richardson (arichardson)

<details>
<summary>Changes</summary>

Buffers added after the verifier was created (e.g. `.include`d files)
were only scanned when a diagnostic was reported in them, so their
directives were never checked if they produced no diagnostics. Scan all
buffers in `verify()` and when looking for `@<!-- -->unknown` directives, and
make calling `verify()` again not report the same directives twice.

Assisted-by: AI tooling

---
Full diff: https://github.com/llvm/llvm-project/pull/233307.diff


4 Files Affected:

- (modified) llvm/include/llvm/Support/SourceMgrDiagnosticVerifier.h (+2-2) 
- (modified) llvm/lib/Support/SourceMgrDiagnosticVerifier.cpp (+12-6) 
- (modified) llvm/test/tools/llvm-mc/verify/basic.s (+12) 
- (modified) llvm/unittests/Support/SourceMgrDiagnosticVerifierTest.cpp (+17) 


``````````diff
diff --git a/llvm/include/llvm/Support/SourceMgrDiagnosticVerifier.h b/llvm/include/llvm/Support/SourceMgrDiagnosticVerifier.h
index 359aba7f3fcae..fc501d8c49bf2 100644
--- a/llvm/include/llvm/Support/SourceMgrDiagnosticVerifier.h
+++ b/llvm/include/llvm/Support/SourceMgrDiagnosticVerifier.h
@@ -63,8 +63,8 @@ class LLVM_ABI SourceMgrDiagnosticVerifier {
   };
 
   /// Scans all buffers currently registered with \p Mgr for expected
-  /// diagnostics. Buffers added later are scanned the first time a
-  /// diagnostic is reported in them. Verifier errors (near misses, unmatched
+  /// diagnostics. Buffers added later are scanned when they are first needed,
+  /// at the latest by \c verify(). Verifier errors (near misses, unmatched
   /// expected diagnostics, malformed directives) are printed to \p OS,
   /// bypassing \p Mgr's diagnostic handler.
   ///
diff --git a/llvm/lib/Support/SourceMgrDiagnosticVerifier.cpp b/llvm/lib/Support/SourceMgrDiagnosticVerifier.cpp
index ef0b4373b5cc7..a005487ed3122 100644
--- a/llvm/lib/Support/SourceMgrDiagnosticVerifier.cpp
+++ b/llvm/lib/Support/SourceMgrDiagnosticVerifier.cpp
@@ -255,6 +255,10 @@ SourceMgrDiagnosticVerifier::process(SourceMgr::DiagKind Kind,
     // with a location in an unknown file can never match anything.
     Diags = computeExpectedDiags(Loc->Buf);
   } else {
+    // '@unknown' directives may be in any buffer, including ones added after
+    // the verifier was created.
+    for (unsigned I = 0, E = Mgr.getNumBuffers(); I != E; ++I)
+      (void)computeExpectedDiags(Mgr.getMemoryBuffer(I + 1));
     Diags = ExpectedUnknownLocDiags;
   }
 
@@ -325,16 +329,18 @@ bool SourceMgrDiagnosticVerifier::verify() {
       emitError(Diag, "expected " + getDiagKindStr(Diag.Kind) + " \"" +
                           Diag.Substring + "\" was not produced");
   };
-  // Check buffers in registration order so that the output is deterministic.
+  // Check all buffers in registration order, including ones that were added
+  // after the verifier was created and had no diagnostics (e.g. included
+  // files). Clear the lists rather than the map so that calling verify()
+  // again does not rescan the buffers and report their directives again.
   for (unsigned I = 0, E = Mgr.getNumBuffers(); I != E; ++I) {
-    auto It = ExpectedDiagsPerBuffer.find(Mgr.getMemoryBuffer(I + 1));
-    if (It == ExpectedDiagsPerBuffer.end())
-      continue;
-    for (const ExpectedDiag &Diag : It->second)
+    const MemoryBuffer *Buf = Mgr.getMemoryBuffer(I + 1);
+    for (const ExpectedDiag &Diag : computeExpectedDiags(Buf))
       CheckExpectedDiags(Diag);
+    ExpectedDiagsPerBuffer[Buf].clear();
   }
   for (const ExpectedDiag &Diag : ExpectedUnknownLocDiags)
     CheckExpectedDiags(Diag);
-  ExpectedDiagsPerBuffer.clear();
+  ExpectedUnknownLocDiags.clear();
   return OK;
 }
diff --git a/llvm/test/tools/llvm-mc/verify/basic.s b/llvm/test/tools/llvm-mc/verify/basic.s
index 82fd0a1f70bcf..cb9fe5e70490d 100644
--- a/llvm/test/tools/llvm-mc/verify/basic.s
+++ b/llvm/test/tools/llvm-mc/verify/basic.s
@@ -39,6 +39,10 @@
 # RUN: not llvm-mc --triple=riscv32 --verify missing.s 2>&1 \
 # RUN:   | FileCheck missing.s
 
+## Directives in included files without diagnostics are checked too.
+# RUN: not llvm-mc --triple=riscv32 --verify include.s 2>&1 \
+# RUN:   | FileCheck included.s
+
 ## A file without diagnostics or directives passes.
 # RUN: llvm-mc --triple=riscv32 --verify no-diagnostics.s
 
@@ -146,6 +150,14 @@ addi a0, a0, 1
 # expected-error at -1 {{this will never happen}}
 # CHECK: error: expected error "this will never happen" was not produced
 
+#--- include.s
+.include "included.s"
+
+#--- included.s
+addi a0, a0, 1
+# expected-error at -1 {{this will never happen}}
+# CHECK: error: expected error "this will never happen" was not produced
+
 #--- no-diagnostics.s
 addi a0, a0, 1
 
diff --git a/llvm/unittests/Support/SourceMgrDiagnosticVerifierTest.cpp b/llvm/unittests/Support/SourceMgrDiagnosticVerifierTest.cpp
index 713fb17bbc6d9..c70bedc69048f 100644
--- a/llvm/unittests/Support/SourceMgrDiagnosticVerifierTest.cpp
+++ b/llvm/unittests/Support/SourceMgrDiagnosticVerifierTest.cpp
@@ -212,3 +212,20 @@ TEST_F(SourceMgrDiagnosticVerifierTest, BuffersWithSameName) {
             MatchResult::Matched);
   EXPECT_FALSE(Verifier.verify());
 }
+
+TEST_F(SourceMgrDiagnosticVerifierTest, BufferAddedLater) {
+  addBuffer("op\n");
+  SourceMgrDiagnosticVerifier Verifier(SM, OS);
+  // E.g. an included file without any diagnostics.
+  addBuffer("op\n# expected-error at -1 {{missing}}\n", "inc.s");
+
+  EXPECT_FALSE(Verifier.verify());
+  EXPECT_NE(StringRef(Output).find(
+                "error: expected error \"missing\" was not produced"),
+            StringRef::npos);
+
+  // Calling verify() again does not report the same directives again.
+  std::string FirstOutput = Output;
+  EXPECT_FALSE(Verifier.verify());
+  EXPECT_EQ(Output, FirstOutput);
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/233307