[clang] [clang] Make SARIF better handle diagnostic about unknown argument (PR #233241)

[Vlad Serebrennikov via cfe-commits]

https://github.com/Endilll created https://github.com/llvm/llvm-project/pull/233241

`SARIFDiagnosticPrinter::HandleDiagnostic` optimistically assumes that its diagnostic renderer (`SARIFDiag`) is always available, but it doesn't hold when driver is parsing arguments and emits unknown argument diagnostic. There is a FIXME to handle diagnostics in this limited mode, but the short-circuit `return` is placed too late, which makes assertion builds crash.

See `TextDiagnosticPrinter::handleDiagnostic` for additional context: https://github.com/llvm/llvm-project/blob/42ac09cebaad223f14f4c3052bcd5d6d25be0f3f/clang/lib/Frontend/TextDiagnosticPrinter.cpp#L131-L150

Fixes #233228

>From c217e801d958f0bf15fbfddd4680373ab4c6587e Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov <serebrennikov.vladislav at gmail.com>
Date: Sun, 18 Oct 2026 17:17:43 +0300
Subject: [PATCH] [clang] Make SARIF better handle diagnostic about unknown
 argument

---
 clang/lib/Frontend/SARIFDiagnosticPrinter.cpp | 3 ++-
 clang/test/Driver/unknown-arg.c               | 7 +++++++
 clang/test/Frontend/warning-options.cpp       | 7 +++++++
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Frontend/SARIFDiagnosticPrinter.cpp b/clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
index 72b796f8db7983..edc021f5569656 100644
--- a/clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
+++ b/clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
@@ -47,7 +47,6 @@ void SARIFDiagnosticPrinter::EndSourceFile() {
 
 void SARIFDiagnosticPrinter::HandleDiagnostic(DiagnosticsEngine::Level Level,
                                               const Diagnostic &Info) {
-  assert(SARIFDiag && "SARIFDiagnostic has not been set.");
   // Default implementation (Warnings/errors count). Keeps track of the
   // number of errors.
   DiagnosticConsumer::HandleDiagnostic(Level, Info);
@@ -68,6 +67,8 @@ void SARIFDiagnosticPrinter::HandleDiagnostic(DiagnosticsEngine::Level Level,
     return;
   }
 
+  assert(SARIFDiag && "SARIFDiagnostic has not been set.");
+
   // Assert that the rest of our infrastructure is setup properly.
   assert(Info.hasSourceManager() &&
          "Unexpected diagnostic with no source manager");
diff --git a/clang/test/Driver/unknown-arg.c b/clang/test/Driver/unknown-arg.c
index 26032a407f4131..030952089b4acf 100644
--- a/clang/test/Driver/unknown-arg.c
+++ b/clang/test/Driver/unknown-arg.c
@@ -65,6 +65,13 @@
 
 // IGNORED: warning: unknown warning option '-Wunknown-to-clang-option'
 
+// RUN %clang -fdiagnostics-format=sarif -Wunknown-to-clang-option
+
+// FIXME: unlike TextDiagnosticPrinter, SARIFDiagnosticPrinter can't yet emit
+// diagnostics in this limited mode without SourceManager, etc.
+// At the moment all we can do is to ensure we don't crash with and without
+// assertions.
+
 // RUN: %clang -### -offload-arch=sm_70 --offload-arch=sm_70 -offload-device-only -o ffload-device-only \
 // RUN:   -output -o foo %s 2>&1 | FileCheck --check-prefix=O-WARN %s
 //      O-WARN: warning: joined argument treated as '-o ffload-arch=sm_70'; did you mean '--offload-arch=sm_70'?
diff --git a/clang/test/Frontend/warning-options.cpp b/clang/test/Frontend/warning-options.cpp
index 444733c8b7f365..e6ddf2e0aad5ea 100644
--- a/clang/test/Frontend/warning-options.cpp
+++ b/clang/test/Frontend/warning-options.cpp
@@ -6,3 +6,10 @@
 // CHECK: unknown warning option '-Wmodule-build'; did you mean '-Wmodule-conflict'?
 // CHECK-NEXT: unknown -Werror warning specifier: '-Werror-vla'
 // CHECK: unknown remark option '-Rmodule-built'; did you mean '-Rmodule-build'?
+
+// RUN: %clang_cc1 -Wmonkey -Wno-monkey -Wno-unused-command-line-arguments \
+// RUN:        -Wno-unused-command-line-argument -Wmodule-build -Werror-vla -Rmodule-built -fdiagnostics-format sarif %s 2>&1
+// FIXME: unlike TextDiagnosticPrinter, SARIFDiagnosticPrinter can't yet emit
+// diagnostics in this limited mode without SourceManager, etc.
+// At the moment all we can do is to ensure we don't crash with and without
+// assertions.