[clang] [llvm] DiagnosticHandler: refactor error checking (PR #75889)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-codegen

Author: Fangrui Song (MaskRay)

<details>
<summary>Changes</summary>

In LLVMContext::diagnose, set `HasErrors` for `DS_Error` so that all
derived `DiagnosticHandler` have correct `HasErrors` information.

An alternative is to set `HasErrors` in
`DiagnosticHandler::handleDiagnostics`, but all derived
`handleDiagnostics` would have to call the base function.


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


3 Files Affected:

- (modified) clang/lib/CodeGen/CodeGenAction.cpp (-2) 
- (modified) llvm/lib/IR/LLVMContext.cpp (+7-4) 
- (modified) llvm/tools/llc/llc.cpp (+3-14) 


``````````diff
diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp
index 4121a3709bc3af..753a8fd74fa696 100644
--- a/clang/lib/CodeGen/CodeGenAction.cpp
+++ b/clang/lib/CodeGen/CodeGenAction.cpp
@@ -418,8 +418,6 @@ void BackendConsumer::anchor() { }
 } // namespace clang
 
 bool ClangDiagnosticHandler::handleDiagnostics(const DiagnosticInfo &DI) {
-  if (DI.getSeverity() == DS_Error)
-    HasErrors = true;
   BackendCon->DiagnosticHandlerImpl(DI);
   return true;
 }
diff --git a/llvm/lib/IR/LLVMContext.cpp b/llvm/lib/IR/LLVMContext.cpp
index 8ddf51537ec1af..57077e786efc26 100644
--- a/llvm/lib/IR/LLVMContext.cpp
+++ b/llvm/lib/IR/LLVMContext.cpp
@@ -256,10 +256,13 @@ void LLVMContext::diagnose(const DiagnosticInfo &DI) {
       RS->emit(*OptDiagBase);
 
   // If there is a report handler, use it.
-  if (pImpl->DiagHandler &&
-      (!pImpl->RespectDiagnosticFilters || isDiagnosticEnabled(DI)) &&
-      pImpl->DiagHandler->handleDiagnostics(DI))
-    return;
+  if (pImpl->DiagHandler) {
+    if (DI.getSeverity() == DS_Error)
+      pImpl->DiagHandler->HasErrors = true;
+    if ((!pImpl->RespectDiagnosticFilters || isDiagnosticEnabled(DI)) &&
+        pImpl->DiagHandler->handleDiagnostics(DI))
+      return;
+  }
 
   if (!isDiagnosticEnabled(DI))
     return;
diff --git a/llvm/tools/llc/llc.cpp b/llvm/tools/llc/llc.cpp
index 8d906cf372878f..4a1957588a2243 100644
--- a/llvm/tools/llc/llc.cpp
+++ b/llvm/tools/llc/llc.cpp
@@ -307,16 +307,12 @@ static std::unique_ptr<ToolOutputFile> GetOutputStream(const char *TargetName,
 }
 
 struct LLCDiagnosticHandler : public DiagnosticHandler {
-  bool *HasError;
-  LLCDiagnosticHandler(bool *HasErrorPtr) : HasError(HasErrorPtr) {}
   bool handleDiagnostics(const DiagnosticInfo &DI) override {
+    DiagnosticHandler::handleDiagnostics(DI);
     if (DI.getKind() == llvm::DK_SrcMgr) {
       const auto &DISM = cast<DiagnosticInfoSrcMgr>(DI);
       const SMDiagnostic &SMD = DISM.getSMDiag();
 
-      if (SMD.getKind() == SourceMgr::DK_Error)
-        *HasError = true;
-
       SMD.print(nullptr, errs());
 
       // For testing purposes, we print the LocCookie here.
@@ -326,9 +322,6 @@ struct LLCDiagnosticHandler : public DiagnosticHandler {
       return true;
     }
 
-    if (DI.getSeverity() == DS_Error)
-      *HasError = true;
-
     if (auto *Remark = dyn_cast<DiagnosticInfoOptimizationBase>(&DI))
       if (!Remark->isEnabled())
         return true;
@@ -413,9 +406,7 @@ int main(int argc, char **argv) {
   Context.setDiscardValueNames(DiscardValueNames);
 
   // Set a diagnostic handler that doesn't exit on the first error
-  bool HasError = false;
-  Context.setDiagnosticHandler(
-      std::make_unique<LLCDiagnosticHandler>(&HasError));
+  Context.setDiagnosticHandler(std::make_unique<LLCDiagnosticHandler>());
 
   Expected<std::unique_ptr<ToolOutputFile>> RemarksFileOrErr =
       setupLLVMOptimizationRemarks(Context, RemarksFilename, RemarksPasses,
@@ -757,9 +748,7 @@ static int compileModule(char **argv, LLVMContext &Context) {
 
     PM.run(*M);
 
-    auto HasError =
-        ((const LLCDiagnosticHandler *)(Context.getDiagHandlerPtr()))->HasError;
-    if (*HasError)
+    if (Context.getDiagHandlerPtr()->HasErrors)
       return 1;
 
     // Compare the two outputs and make sure they're the same

``````````

</details>


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