[clang] [Clang] [NFC] Move diagnostics emitting code from `DiagnosticIDs` into `DiagnosticsEngine` (PR #143517)

2025-06-11 Thread via cfe-commits

https://github.com/Sirraide closed 
https://github.com/llvm/llvm-project/pull/143517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] [NFC] Move diagnostics emitting code from `DiagnosticIDs` into `DiagnosticsEngine` (PR #143517)

2025-06-10 Thread via cfe-commits

https://github.com/Sirraide updated 
https://github.com/llvm/llvm-project/pull/143517

>From 3c09d0012b173e68c0c760add55d5a968d23bb24 Mon Sep 17 00:00:00 2001
From: Sirraide 
Date: Sat, 7 Jun 2025 17:00:50 +0200
Subject: [PATCH 1/3] [Clang] [NFC] Move diagnostics emitting code from
 DiagnosticIDs into DiagnosticsEngine

---
 clang/include/clang/Basic/Diagnostic.h|  23 ++---
 clang/include/clang/Basic/DiagnosticIDs.h |  12 ---
 clang/lib/Basic/Diagnostic.cpp| 100 --
 clang/lib/Basic/DiagnosticIDs.cpp |  97 -
 4 files changed, 104 insertions(+), 128 deletions(-)

diff --git a/clang/include/clang/Basic/Diagnostic.h 
b/clang/include/clang/Basic/Diagnostic.h
index e9c54c3c487c9..7696acfb45f92 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -18,6 +18,7 @@
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
+#include "clang/Basic/UnsignedOrNone.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/FunctionExtras.h"
@@ -49,6 +50,7 @@ class FileSystem;
 namespace clang {
 
 class DeclContext;
+class Diagnostic;
 class DiagnosticBuilder;
 class DiagnosticConsumer;
 class IdentifierInfo;
@@ -228,6 +230,8 @@ class DiagStorageAllocator {
 class DiagnosticsEngine : public RefCountedBase {
 public:
   /// The level of the diagnostic, after it has been through mapping.
+  // FIXME: Make this an alias for DiagnosticIDs::Level as soon as
+  // we can use 'using enum'.
   enum Level {
 Ignored = DiagnosticIDs::Ignored,
 Note = DiagnosticIDs::Note,
@@ -532,7 +536,7 @@ class DiagnosticsEngine : public 
RefCountedBase {
   ///
   /// This is used to emit continuation diagnostics with the same level as the
   /// diagnostic that they follow.
-  DiagnosticIDs::Level LastDiagLevel;
+  Level LastDiagLevel;
 
   /// Number of warnings reported
   unsigned NumWarnings;
@@ -777,18 +781,16 @@ class DiagnosticsEngine : public 
RefCountedBase {
   /// the middle of another diagnostic.
   ///
   /// This can be used by clients who suppress diagnostics themselves.
-  void setLastDiagnosticIgnored(bool Ignored) {
-if (LastDiagLevel == DiagnosticIDs::Fatal)
+  void setLastDiagnosticIgnored(bool IsIgnored) {
+if (LastDiagLevel == Fatal)
   FatalErrorOccurred = true;
-LastDiagLevel = Ignored ? DiagnosticIDs::Ignored : DiagnosticIDs::Warning;
+LastDiagLevel = IsIgnored ? Ignored : Warning;
   }
 
   /// Determine whether the previous diagnostic was ignored. This can
   /// be used by clients that want to determine whether notes attached to a
   /// diagnostic will be suppressed.
-  bool isLastDiagnosticIgnored() const {
-return LastDiagLevel == DiagnosticIDs::Ignored;
-  }
+  bool isLastDiagnosticIgnored() const { return LastDiagLevel == Ignored; }
 
   /// Controls whether otherwise-unmapped extension diagnostics are
   /// mapped onto ignore/warning/error.
@@ -1024,9 +1026,7 @@ class DiagnosticsEngine : public 
RefCountedBase {
   /// Used to report a diagnostic that is finally fully formed.
   ///
   /// \returns true if the diagnostic was emitted, false if it was suppressed.
-  bool ProcessDiag(const DiagnosticBuilder &DiagBuilder) {
-return Diags->ProcessDiag(*this, DiagBuilder);
-  }
+  bool ProcessDiag(const DiagnosticBuilder &DiagBuilder);
 
   /// @name Diagnostic Emission
   /// @{
@@ -1046,6 +1046,9 @@ class DiagnosticsEngine : public 
RefCountedBase {
   bool EmitDiagnostic(const DiagnosticBuilder &DB, bool Force = false);
 
   /// @}
+
+private:
+  void Report(Level DiagLevel, const Diagnostic &Info);
 };
 
 /// RAII class that determines when any errors have occurred
diff --git a/clang/include/clang/Basic/DiagnosticIDs.h 
b/clang/include/clang/Basic/DiagnosticIDs.h
index 80d52a0d01112..2b095f0fd6741 100644
--- a/clang/include/clang/Basic/DiagnosticIDs.h
+++ b/clang/include/clang/Basic/DiagnosticIDs.h
@@ -483,18 +483,6 @@ class DiagnosticIDs : public RefCountedBase 
{
 
   Class getDiagClass(unsigned DiagID) const;
 
-  /// Used to report a diagnostic that is finally fully formed.
-  ///
-  /// \returns \c true if the diagnostic was emitted, \c false if it was
-  /// suppressed.
-  bool ProcessDiag(DiagnosticsEngine &Diag,
-   const DiagnosticBuilder &DiagBuilder) const;
-
-  /// Used to emit a diagnostic that is finally fully formed,
-  /// ignoring suppression.
-  void EmitDiag(DiagnosticsEngine &Diag, const DiagnosticBuilder &DiagBuilder,
-Level DiagLevel) const;
-
   /// Whether the diagnostic may leave the AST in a state where some
   /// invariants can break.
   bool isUnrecoverable(unsigned DiagID) const;
diff --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp
index 694224071347a..2718874703bfe 100644
--- a/clang/lib/Basic/Diagnostic.cpp
+++ b/clang/lib/Basic/Diagnostic.cpp
@@ -130,7 +130,7 @@ void DiagnosticsEngine::Rese

[clang] [Clang] [NFC] Move diagnostics emitting code from `DiagnosticIDs` into `DiagnosticsEngine` (PR #143517)

2025-06-10 Thread via cfe-commits

https://github.com/Sirraide updated 
https://github.com/llvm/llvm-project/pull/143517

>From 3c09d0012b173e68c0c760add55d5a968d23bb24 Mon Sep 17 00:00:00 2001
From: Sirraide 
Date: Sat, 7 Jun 2025 17:00:50 +0200
Subject: [PATCH 1/2] [Clang] [NFC] Move diagnostics emitting code from
 DiagnosticIDs into DiagnosticsEngine

---
 clang/include/clang/Basic/Diagnostic.h|  23 ++---
 clang/include/clang/Basic/DiagnosticIDs.h |  12 ---
 clang/lib/Basic/Diagnostic.cpp| 100 --
 clang/lib/Basic/DiagnosticIDs.cpp |  97 -
 4 files changed, 104 insertions(+), 128 deletions(-)

diff --git a/clang/include/clang/Basic/Diagnostic.h 
b/clang/include/clang/Basic/Diagnostic.h
index e9c54c3c487c9..7696acfb45f92 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -18,6 +18,7 @@
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
+#include "clang/Basic/UnsignedOrNone.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/FunctionExtras.h"
@@ -49,6 +50,7 @@ class FileSystem;
 namespace clang {
 
 class DeclContext;
+class Diagnostic;
 class DiagnosticBuilder;
 class DiagnosticConsumer;
 class IdentifierInfo;
@@ -228,6 +230,8 @@ class DiagStorageAllocator {
 class DiagnosticsEngine : public RefCountedBase {
 public:
   /// The level of the diagnostic, after it has been through mapping.
+  // FIXME: Make this an alias for DiagnosticIDs::Level as soon as
+  // we can use 'using enum'.
   enum Level {
 Ignored = DiagnosticIDs::Ignored,
 Note = DiagnosticIDs::Note,
@@ -532,7 +536,7 @@ class DiagnosticsEngine : public 
RefCountedBase {
   ///
   /// This is used to emit continuation diagnostics with the same level as the
   /// diagnostic that they follow.
-  DiagnosticIDs::Level LastDiagLevel;
+  Level LastDiagLevel;
 
   /// Number of warnings reported
   unsigned NumWarnings;
@@ -777,18 +781,16 @@ class DiagnosticsEngine : public 
RefCountedBase {
   /// the middle of another diagnostic.
   ///
   /// This can be used by clients who suppress diagnostics themselves.
-  void setLastDiagnosticIgnored(bool Ignored) {
-if (LastDiagLevel == DiagnosticIDs::Fatal)
+  void setLastDiagnosticIgnored(bool IsIgnored) {
+if (LastDiagLevel == Fatal)
   FatalErrorOccurred = true;
-LastDiagLevel = Ignored ? DiagnosticIDs::Ignored : DiagnosticIDs::Warning;
+LastDiagLevel = IsIgnored ? Ignored : Warning;
   }
 
   /// Determine whether the previous diagnostic was ignored. This can
   /// be used by clients that want to determine whether notes attached to a
   /// diagnostic will be suppressed.
-  bool isLastDiagnosticIgnored() const {
-return LastDiagLevel == DiagnosticIDs::Ignored;
-  }
+  bool isLastDiagnosticIgnored() const { return LastDiagLevel == Ignored; }
 
   /// Controls whether otherwise-unmapped extension diagnostics are
   /// mapped onto ignore/warning/error.
@@ -1024,9 +1026,7 @@ class DiagnosticsEngine : public 
RefCountedBase {
   /// Used to report a diagnostic that is finally fully formed.
   ///
   /// \returns true if the diagnostic was emitted, false if it was suppressed.
-  bool ProcessDiag(const DiagnosticBuilder &DiagBuilder) {
-return Diags->ProcessDiag(*this, DiagBuilder);
-  }
+  bool ProcessDiag(const DiagnosticBuilder &DiagBuilder);
 
   /// @name Diagnostic Emission
   /// @{
@@ -1046,6 +1046,9 @@ class DiagnosticsEngine : public 
RefCountedBase {
   bool EmitDiagnostic(const DiagnosticBuilder &DB, bool Force = false);
 
   /// @}
+
+private:
+  void Report(Level DiagLevel, const Diagnostic &Info);
 };
 
 /// RAII class that determines when any errors have occurred
diff --git a/clang/include/clang/Basic/DiagnosticIDs.h 
b/clang/include/clang/Basic/DiagnosticIDs.h
index 80d52a0d01112..2b095f0fd6741 100644
--- a/clang/include/clang/Basic/DiagnosticIDs.h
+++ b/clang/include/clang/Basic/DiagnosticIDs.h
@@ -483,18 +483,6 @@ class DiagnosticIDs : public RefCountedBase 
{
 
   Class getDiagClass(unsigned DiagID) const;
 
-  /// Used to report a diagnostic that is finally fully formed.
-  ///
-  /// \returns \c true if the diagnostic was emitted, \c false if it was
-  /// suppressed.
-  bool ProcessDiag(DiagnosticsEngine &Diag,
-   const DiagnosticBuilder &DiagBuilder) const;
-
-  /// Used to emit a diagnostic that is finally fully formed,
-  /// ignoring suppression.
-  void EmitDiag(DiagnosticsEngine &Diag, const DiagnosticBuilder &DiagBuilder,
-Level DiagLevel) const;
-
   /// Whether the diagnostic may leave the AST in a state where some
   /// invariants can break.
   bool isUnrecoverable(unsigned DiagID) const;
diff --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp
index 694224071347a..2718874703bfe 100644
--- a/clang/lib/Basic/Diagnostic.cpp
+++ b/clang/lib/Basic/Diagnostic.cpp
@@ -130,7 +130,7 @@ void DiagnosticsEngine::Rese

[clang] [Clang] [NFC] Move diagnostics emitting code from `DiagnosticIDs` into `DiagnosticsEngine` (PR #143517)

2025-06-10 Thread via cfe-commits


@@ -1046,6 +1046,9 @@ class DiagnosticsEngine : public 
RefCountedBase {
   bool EmitDiagnostic(const DiagnosticBuilder &DB, bool Force = false);
 
   /// @}
+
+private:
+  void Report(Level DiagLevel, const Diagnostic &Info);

Sirraide wrote:

I thought `ProcessDiag` was public but apparently it isn’t; I’ll move it

https://github.com/llvm/llvm-project/pull/143517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] [NFC] Move diagnostics emitting code from `DiagnosticIDs` into `DiagnosticsEngine` (PR #143517)

2025-06-10 Thread Mariya Podchishchaeva via cfe-commits


@@ -1046,6 +1046,9 @@ class DiagnosticsEngine : public 
RefCountedBase {
   bool EmitDiagnostic(const DiagnosticBuilder &DB, bool Force = false);
 
   /// @}
+
+private:
+  void Report(Level DiagLevel, const Diagnostic &Info);

Fznamznon wrote:

Why not put it right after ProcessDiag on line 1029?

https://github.com/llvm/llvm-project/pull/143517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] [NFC] Move diagnostics emitting code from `DiagnosticIDs` into `DiagnosticsEngine` (PR #143517)

2025-06-10 Thread Mariya Podchishchaeva via cfe-commits


@@ -658,13 +658,97 @@ void DiagnosticsEngine::Report(const StoredDiagnostic 
&storedDiag) {
   Level DiagLevel = storedDiag.getLevel();
   Diagnostic Info(this, storedDiag.getLocation(), storedDiag.getID(),
   DiagStorage, storedDiag.getMessage());
+  Report(DiagLevel, Info);
+}
+
+void DiagnosticsEngine::Report(Level DiagLevel, const Diagnostic &Info) {
+  assert(DiagLevel != Ignored && "Cannot emit ignored diagnostics!");
   Client->HandleDiagnostic(DiagLevel, Info);
   if (Client->IncludeInDiagnosticCounts()) {
-if (DiagLevel == DiagnosticsEngine::Warning)
+if (DiagLevel == Warning)
   ++NumWarnings;
   }
 }
 
+/// ProcessDiag - This is the method used to report a diagnostic that is
+/// finally fully formed.
+bool DiagnosticsEngine::ProcessDiag(const DiagnosticBuilder &DiagBuilder) {
+  Diagnostic Info(this, DiagBuilder);
+
+  assert(getClient() && "DiagnosticClient not set!");
+
+  // Figure out the diagnostic level of this message.
+  unsigned DiagID = Info.getID();
+  Level DiagLevel = getDiagnosticLevel(DiagID, Info.getLocation());
+
+  // Update counts for DiagnosticErrorTrap even if a fatal error occurred
+  // or diagnostics are suppressed.
+  if (DiagLevel >= Error) {
+++TrapNumErrorsOccurred;
+if (Diags->isUnrecoverable(DiagID))
+  ++TrapNumUnrecoverableErrorsOccurred;
+  }
+
+  if (SuppressAllDiagnostics)
+return false;
+
+  if (DiagLevel != Note) {
+// Record that a fatal error occurred only when we see a second
+// non-note diagnostic. This allows notes to be attached to the
+// fatal error, but suppresses any diagnostics that follow those
+// notes.
+if (LastDiagLevel == Fatal)
+  FatalErrorOccurred = true;
+
+LastDiagLevel = DiagLevel;
+  }
+
+  // If a fatal error has already been emitted, silence all subsequent
+  // diagnostics.
+  if (FatalErrorOccurred) {
+if (DiagLevel >= Error && Client->IncludeInDiagnosticCounts()) {
+  ++NumErrors;
+}
+
+return false;
+  }
+
+  // If the client doesn't care about this message, don't issue it.  If this is
+  // a note and the last real diagnostic was ignored, ignore it too.
+  if (DiagLevel == Ignored || (DiagLevel == Note && LastDiagLevel == Ignored))
+return false;
+
+  if (DiagLevel >= Error) {
+if (Diags->isUnrecoverable(DiagID))
+  UnrecoverableErrorOccurred = true;
+
+// Warnings which have been upgraded to errors do not prevent compilation.
+if (Diags->isDefaultMappingAsError(DiagID))
+  UncompilableErrorOccurred = true;
+
+ErrorOccurred = true;
+if (Client->IncludeInDiagnosticCounts()) {
+  ++NumErrors;
+}

Fznamznon wrote:

```suggestion
if (Client->IncludeInDiagnosticCounts())
  ++NumErrors;
```

https://github.com/llvm/llvm-project/pull/143517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] [NFC] Move diagnostics emitting code from `DiagnosticIDs` into `DiagnosticsEngine` (PR #143517)

2025-06-10 Thread Mariya Podchishchaeva via cfe-commits


@@ -658,13 +658,97 @@ void DiagnosticsEngine::Report(const StoredDiagnostic 
&storedDiag) {
   Level DiagLevel = storedDiag.getLevel();
   Diagnostic Info(this, storedDiag.getLocation(), storedDiag.getID(),
   DiagStorage, storedDiag.getMessage());
+  Report(DiagLevel, Info);
+}
+
+void DiagnosticsEngine::Report(Level DiagLevel, const Diagnostic &Info) {
+  assert(DiagLevel != Ignored && "Cannot emit ignored diagnostics!");
   Client->HandleDiagnostic(DiagLevel, Info);
   if (Client->IncludeInDiagnosticCounts()) {
-if (DiagLevel == DiagnosticsEngine::Warning)
+if (DiagLevel == Warning)
   ++NumWarnings;
   }
 }
 
+/// ProcessDiag - This is the method used to report a diagnostic that is
+/// finally fully formed.
+bool DiagnosticsEngine::ProcessDiag(const DiagnosticBuilder &DiagBuilder) {
+  Diagnostic Info(this, DiagBuilder);
+
+  assert(getClient() && "DiagnosticClient not set!");
+
+  // Figure out the diagnostic level of this message.
+  unsigned DiagID = Info.getID();
+  Level DiagLevel = getDiagnosticLevel(DiagID, Info.getLocation());
+
+  // Update counts for DiagnosticErrorTrap even if a fatal error occurred
+  // or diagnostics are suppressed.
+  if (DiagLevel >= Error) {
+++TrapNumErrorsOccurred;
+if (Diags->isUnrecoverable(DiagID))
+  ++TrapNumUnrecoverableErrorsOccurred;
+  }
+
+  if (SuppressAllDiagnostics)
+return false;
+
+  if (DiagLevel != Note) {
+// Record that a fatal error occurred only when we see a second
+// non-note diagnostic. This allows notes to be attached to the
+// fatal error, but suppresses any diagnostics that follow those
+// notes.
+if (LastDiagLevel == Fatal)
+  FatalErrorOccurred = true;
+
+LastDiagLevel = DiagLevel;
+  }
+
+  // If a fatal error has already been emitted, silence all subsequent
+  // diagnostics.
+  if (FatalErrorOccurred) {
+if (DiagLevel >= Error && Client->IncludeInDiagnosticCounts()) {
+  ++NumErrors;
+}

Fznamznon wrote:

```suggestion
if (DiagLevel >= Error && Client->IncludeInDiagnosticCounts())
  ++NumErrors;
```

https://github.com/llvm/llvm-project/pull/143517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] [NFC] Move diagnostics emitting code from `DiagnosticIDs` into `DiagnosticsEngine` (PR #143517)

2025-06-10 Thread Mariya Podchishchaeva via cfe-commits

https://github.com/Fznamznon edited 
https://github.com/llvm/llvm-project/pull/143517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] [NFC] Move diagnostics emitting code from `DiagnosticIDs` into `DiagnosticsEngine` (PR #143517)

2025-06-10 Thread Mariya Podchishchaeva via cfe-commits

https://github.com/Fznamznon approved this pull request.

That makes sense to me, but I've never touched that before, so probably makes 
sense to wait for the feedback from @AaronBallman too.

https://github.com/llvm/llvm-project/pull/143517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] [NFC] Move diagnostics emitting code from `DiagnosticIDs` into `DiagnosticsEngine` (PR #143517)

2025-06-10 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: None (Sirraide)


Changes

It makes more sense for this functionality to be all in one place rather than 
split up across two files—at least it caused me a bit of a headache to try and 
find all places where we were actually forwarding the diagnostic to the 
`DiagnosticConsumer`. Moreover, moving these functions into `DiagnosticsEngine` 
simplifies the code quite a bit since we access members of `DiagnosticsEngine` 
more frequently than those of `DiagnosticIDs`. There was also a duplicated code 
snippet that I’ve moved out into a new function.

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


4 Files Affected:

- (modified) clang/include/clang/Basic/Diagnostic.h (+13-10) 
- (modified) clang/include/clang/Basic/DiagnosticIDs.h (-12) 
- (modified) clang/lib/Basic/Diagnostic.cpp (+91-9) 
- (modified) clang/lib/Basic/DiagnosticIDs.cpp (-97) 


``diff
diff --git a/clang/include/clang/Basic/Diagnostic.h 
b/clang/include/clang/Basic/Diagnostic.h
index e9c54c3c487c9..7696acfb45f92 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -18,6 +18,7 @@
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
+#include "clang/Basic/UnsignedOrNone.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/FunctionExtras.h"
@@ -49,6 +50,7 @@ class FileSystem;
 namespace clang {
 
 class DeclContext;
+class Diagnostic;
 class DiagnosticBuilder;
 class DiagnosticConsumer;
 class IdentifierInfo;
@@ -228,6 +230,8 @@ class DiagStorageAllocator {
 class DiagnosticsEngine : public RefCountedBase {
 public:
   /// The level of the diagnostic, after it has been through mapping.
+  // FIXME: Make this an alias for DiagnosticIDs::Level as soon as
+  // we can use 'using enum'.
   enum Level {
 Ignored = DiagnosticIDs::Ignored,
 Note = DiagnosticIDs::Note,
@@ -532,7 +536,7 @@ class DiagnosticsEngine : public 
RefCountedBase {
   ///
   /// This is used to emit continuation diagnostics with the same level as the
   /// diagnostic that they follow.
-  DiagnosticIDs::Level LastDiagLevel;
+  Level LastDiagLevel;
 
   /// Number of warnings reported
   unsigned NumWarnings;
@@ -777,18 +781,16 @@ class DiagnosticsEngine : public 
RefCountedBase {
   /// the middle of another diagnostic.
   ///
   /// This can be used by clients who suppress diagnostics themselves.
-  void setLastDiagnosticIgnored(bool Ignored) {
-if (LastDiagLevel == DiagnosticIDs::Fatal)
+  void setLastDiagnosticIgnored(bool IsIgnored) {
+if (LastDiagLevel == Fatal)
   FatalErrorOccurred = true;
-LastDiagLevel = Ignored ? DiagnosticIDs::Ignored : DiagnosticIDs::Warning;
+LastDiagLevel = IsIgnored ? Ignored : Warning;
   }
 
   /// Determine whether the previous diagnostic was ignored. This can
   /// be used by clients that want to determine whether notes attached to a
   /// diagnostic will be suppressed.
-  bool isLastDiagnosticIgnored() const {
-return LastDiagLevel == DiagnosticIDs::Ignored;
-  }
+  bool isLastDiagnosticIgnored() const { return LastDiagLevel == Ignored; }
 
   /// Controls whether otherwise-unmapped extension diagnostics are
   /// mapped onto ignore/warning/error.
@@ -1024,9 +1026,7 @@ class DiagnosticsEngine : public 
RefCountedBase {
   /// Used to report a diagnostic that is finally fully formed.
   ///
   /// \returns true if the diagnostic was emitted, false if it was suppressed.
-  bool ProcessDiag(const DiagnosticBuilder &DiagBuilder) {
-return Diags->ProcessDiag(*this, DiagBuilder);
-  }
+  bool ProcessDiag(const DiagnosticBuilder &DiagBuilder);
 
   /// @name Diagnostic Emission
   /// @{
@@ -1046,6 +1046,9 @@ class DiagnosticsEngine : public 
RefCountedBase {
   bool EmitDiagnostic(const DiagnosticBuilder &DB, bool Force = false);
 
   /// @}
+
+private:
+  void Report(Level DiagLevel, const Diagnostic &Info);
 };
 
 /// RAII class that determines when any errors have occurred
diff --git a/clang/include/clang/Basic/DiagnosticIDs.h 
b/clang/include/clang/Basic/DiagnosticIDs.h
index 80d52a0d01112..2b095f0fd6741 100644
--- a/clang/include/clang/Basic/DiagnosticIDs.h
+++ b/clang/include/clang/Basic/DiagnosticIDs.h
@@ -483,18 +483,6 @@ class DiagnosticIDs : public RefCountedBase 
{
 
   Class getDiagClass(unsigned DiagID) const;
 
-  /// Used to report a diagnostic that is finally fully formed.
-  ///
-  /// \returns \c true if the diagnostic was emitted, \c false if it was
-  /// suppressed.
-  bool ProcessDiag(DiagnosticsEngine &Diag,
-   const DiagnosticBuilder &DiagBuilder) const;
-
-  /// Used to emit a diagnostic that is finally fully formed,
-  /// ignoring suppression.
-  void EmitDiag(DiagnosticsEngine &Diag, const DiagnosticBuilder &DiagBuilder,
-Level DiagLevel) const;
-
   /// Whether the diagnostic may leave the AST in a state where some
   ///

[clang] [Clang] [NFC] Move diagnostics emitting code from `DiagnosticIDs` into `DiagnosticsEngine` (PR #143517)

2025-06-10 Thread via cfe-commits

https://github.com/Sirraide created 
https://github.com/llvm/llvm-project/pull/143517

It makes more sense for this functionality to be all in one place rather than 
split up across two files—at least it caused me a bit of a headache to try and 
find all places where we were actually forwarding the diagnostic to the 
`DiagnosticConsumer`. Moreover, moving these functions into `DiagnosticsEngine` 
simplifies the code quite a bit since we access members of `DiagnosticsEngine` 
more frequently than those of `DiagnosticIDs`. There was also a duplicated code 
snippet that I’ve moved out into a new function.

>From 3c09d0012b173e68c0c760add55d5a968d23bb24 Mon Sep 17 00:00:00 2001
From: Sirraide 
Date: Sat, 7 Jun 2025 17:00:50 +0200
Subject: [PATCH] [Clang] [NFC] Move diagnostics emitting code from
 DiagnosticIDs into DiagnosticsEngine

---
 clang/include/clang/Basic/Diagnostic.h|  23 ++---
 clang/include/clang/Basic/DiagnosticIDs.h |  12 ---
 clang/lib/Basic/Diagnostic.cpp| 100 --
 clang/lib/Basic/DiagnosticIDs.cpp |  97 -
 4 files changed, 104 insertions(+), 128 deletions(-)

diff --git a/clang/include/clang/Basic/Diagnostic.h 
b/clang/include/clang/Basic/Diagnostic.h
index e9c54c3c487c9..7696acfb45f92 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -18,6 +18,7 @@
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
+#include "clang/Basic/UnsignedOrNone.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/FunctionExtras.h"
@@ -49,6 +50,7 @@ class FileSystem;
 namespace clang {
 
 class DeclContext;
+class Diagnostic;
 class DiagnosticBuilder;
 class DiagnosticConsumer;
 class IdentifierInfo;
@@ -228,6 +230,8 @@ class DiagStorageAllocator {
 class DiagnosticsEngine : public RefCountedBase {
 public:
   /// The level of the diagnostic, after it has been through mapping.
+  // FIXME: Make this an alias for DiagnosticIDs::Level as soon as
+  // we can use 'using enum'.
   enum Level {
 Ignored = DiagnosticIDs::Ignored,
 Note = DiagnosticIDs::Note,
@@ -532,7 +536,7 @@ class DiagnosticsEngine : public 
RefCountedBase {
   ///
   /// This is used to emit continuation diagnostics with the same level as the
   /// diagnostic that they follow.
-  DiagnosticIDs::Level LastDiagLevel;
+  Level LastDiagLevel;
 
   /// Number of warnings reported
   unsigned NumWarnings;
@@ -777,18 +781,16 @@ class DiagnosticsEngine : public 
RefCountedBase {
   /// the middle of another diagnostic.
   ///
   /// This can be used by clients who suppress diagnostics themselves.
-  void setLastDiagnosticIgnored(bool Ignored) {
-if (LastDiagLevel == DiagnosticIDs::Fatal)
+  void setLastDiagnosticIgnored(bool IsIgnored) {
+if (LastDiagLevel == Fatal)
   FatalErrorOccurred = true;
-LastDiagLevel = Ignored ? DiagnosticIDs::Ignored : DiagnosticIDs::Warning;
+LastDiagLevel = IsIgnored ? Ignored : Warning;
   }
 
   /// Determine whether the previous diagnostic was ignored. This can
   /// be used by clients that want to determine whether notes attached to a
   /// diagnostic will be suppressed.
-  bool isLastDiagnosticIgnored() const {
-return LastDiagLevel == DiagnosticIDs::Ignored;
-  }
+  bool isLastDiagnosticIgnored() const { return LastDiagLevel == Ignored; }
 
   /// Controls whether otherwise-unmapped extension diagnostics are
   /// mapped onto ignore/warning/error.
@@ -1024,9 +1026,7 @@ class DiagnosticsEngine : public 
RefCountedBase {
   /// Used to report a diagnostic that is finally fully formed.
   ///
   /// \returns true if the diagnostic was emitted, false if it was suppressed.
-  bool ProcessDiag(const DiagnosticBuilder &DiagBuilder) {
-return Diags->ProcessDiag(*this, DiagBuilder);
-  }
+  bool ProcessDiag(const DiagnosticBuilder &DiagBuilder);
 
   /// @name Diagnostic Emission
   /// @{
@@ -1046,6 +1046,9 @@ class DiagnosticsEngine : public 
RefCountedBase {
   bool EmitDiagnostic(const DiagnosticBuilder &DB, bool Force = false);
 
   /// @}
+
+private:
+  void Report(Level DiagLevel, const Diagnostic &Info);
 };
 
 /// RAII class that determines when any errors have occurred
diff --git a/clang/include/clang/Basic/DiagnosticIDs.h 
b/clang/include/clang/Basic/DiagnosticIDs.h
index 80d52a0d01112..2b095f0fd6741 100644
--- a/clang/include/clang/Basic/DiagnosticIDs.h
+++ b/clang/include/clang/Basic/DiagnosticIDs.h
@@ -483,18 +483,6 @@ class DiagnosticIDs : public RefCountedBase 
{
 
   Class getDiagClass(unsigned DiagID) const;
 
-  /// Used to report a diagnostic that is finally fully formed.
-  ///
-  /// \returns \c true if the diagnostic was emitted, \c false if it was
-  /// suppressed.
-  bool ProcessDiag(DiagnosticsEngine &Diag,
-   const DiagnosticBuilder &DiagBuilder) const;
-
-  /// Used to emit a diagnostic that is finally fully formed,
-  /// ignoring supp