[clang] [Clang] [NFC] Move diagnostics emitting code from `DiagnosticIDs` into `DiagnosticsEngine` (PR #143517)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Jun 10 05:24:03 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: None (Sirraide)
<details>
<summary>Changes</summary>
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<DiagnosticsEngine> {
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<DiagnosticsEngine> {
///
/// 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<DiagnosticsEngine> {
/// 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<DiagnosticsEngine> {
/// 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<DiagnosticsEngine> {
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<DiagnosticIDs> {
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::Reset(bool soft /*=false*/) {
TrapNumErrorsOccurred = 0;
TrapNumUnrecoverableErrorsOccurred = 0;
- LastDiagLevel = DiagnosticIDs::Ignored;
+ LastDiagLevel = Ignored;
if (!soft) {
// Clear state related to #pragma diagnostic.
@@ -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;
+ }
+
+ // If we've emitted a lot of errors, emit a fatal error instead of it to
+ // stop a flood of bogus errors.
+ if (ErrorLimit && NumErrors > ErrorLimit && DiagLevel == Error) {
+ Report(diag::fatal_too_many_errors);
+ return false;
+ }
+ }
+
+ // Make sure we set FatalErrorOccurred to ensure that the notes from the
+ // diagnostic that caused `fatal_too_many_errors` won't be emitted.
+ if (Info.getID() == diag::fatal_too_many_errors)
+ FatalErrorOccurred = true;
+
+ // Finally, report it.
+ Report(DiagLevel, Info);
+ return true;
+}
+
bool DiagnosticsEngine::EmitDiagnostic(const DiagnosticBuilder &DB,
bool Force) {
assert(getClient() && "DiagnosticClient not set!");
@@ -674,14 +758,12 @@ bool DiagnosticsEngine::EmitDiagnostic(const DiagnosticBuilder &DB,
Diagnostic Info(this, DB);
// Figure out the diagnostic level of this message.
- DiagnosticIDs::Level DiagLevel =
- Diags->getDiagnosticLevel(Info.getID(), Info.getLocation(), *this);
+ Level DiagLevel = getDiagnosticLevel(Info.getID(), Info.getLocation());
- Emitted = (DiagLevel != DiagnosticIDs::Ignored);
- if (Emitted) {
- // Emit the diagnostic regardless of suppression level.
- Diags->EmitDiag(*this, DB, DiagLevel);
- }
+ // Emit the diagnostic regardless of suppression level.
+ Emitted = DiagLevel != Ignored;
+ if (Emitted)
+ Report(DiagLevel, Info);
} else {
// Process the diagnostic, sending the accumulated information to the
// DiagnosticConsumer.
diff --git a/clang/lib/Basic/DiagnosticIDs.cpp b/clang/lib/Basic/DiagnosticIDs.cpp
index 3e90b2d804773..dcf0c6cb54282 100644
--- a/clang/lib/Basic/DiagnosticIDs.cpp
+++ b/clang/lib/Basic/DiagnosticIDs.cpp
@@ -823,103 +823,6 @@ unsigned DiagnosticIDs::getCXXCompatDiagId(const LangOptions &LangOpts,
return StdVer >= D.StdVer ? D.DiagId : D.PreDiagId;
}
-/// ProcessDiag - This is the method used to report a diagnostic that is
-/// finally fully formed.
-bool DiagnosticIDs::ProcessDiag(DiagnosticsEngine &Diag,
- const DiagnosticBuilder &DiagBuilder) const {
- Diagnostic Info(&Diag, DiagBuilder);
-
- assert(Diag.getClient() && "DiagnosticClient not set!");
-
- // Figure out the diagnostic level of this message.
- unsigned DiagID = Info.getID();
- DiagnosticIDs::Level DiagLevel
- = getDiagnosticLevel(DiagID, Info.getLocation(), Diag);
-
- // Update counts for DiagnosticErrorTrap even if a fatal error occurred
- // or diagnostics are suppressed.
- if (DiagLevel >= DiagnosticIDs::Error) {
- ++Diag.TrapNumErrorsOccurred;
- if (isUnrecoverable(DiagID))
- ++Diag.TrapNumUnrecoverableErrorsOccurred;
- }
-
- if (Diag.SuppressAllDiagnostics)
- return false;
-
- if (DiagLevel != DiagnosticIDs::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 (Diag.LastDiagLevel == DiagnosticIDs::Fatal)
- Diag.FatalErrorOccurred = true;
-
- Diag.LastDiagLevel = DiagLevel;
- }
-
- // If a fatal error has already been emitted, silence all subsequent
- // diagnostics.
- if (Diag.FatalErrorOccurred) {
- if (DiagLevel >= DiagnosticIDs::Error &&
- Diag.Client->IncludeInDiagnosticCounts()) {
- ++Diag.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 == DiagnosticIDs::Ignored ||
- (DiagLevel == DiagnosticIDs::Note &&
- Diag.LastDiagLevel == DiagnosticIDs::Ignored))
- return false;
-
- if (DiagLevel >= DiagnosticIDs::Error) {
- if (isUnrecoverable(DiagID))
- Diag.UnrecoverableErrorOccurred = true;
-
- // Warnings which have been upgraded to errors do not prevent compilation.
- if (isDefaultMappingAsError(DiagID))
- Diag.UncompilableErrorOccurred = true;
-
- Diag.ErrorOccurred = true;
- if (Diag.Client->IncludeInDiagnosticCounts()) {
- ++Diag.NumErrors;
- }
-
- // If we've emitted a lot of errors, emit a fatal error instead of it to
- // stop a flood of bogus errors.
- if (Diag.ErrorLimit && Diag.NumErrors > Diag.ErrorLimit &&
- DiagLevel == DiagnosticIDs::Error) {
- Diag.Report(diag::fatal_too_many_errors);
- return false;
- }
- }
-
- // Make sure we set FatalErrorOccurred to ensure that the notes from the
- // diagnostic that caused `fatal_too_many_errors` won't be emitted.
- if (Info.getID() == diag::fatal_too_many_errors)
- Diag.FatalErrorOccurred = true;
- // Finally, report it.
- EmitDiag(Diag, DiagBuilder, DiagLevel);
- return true;
-}
-
-void DiagnosticIDs::EmitDiag(DiagnosticsEngine &Diag,
- const DiagnosticBuilder &DiagBuilder,
- Level DiagLevel) const {
- Diagnostic Info(&Diag, DiagBuilder);
- assert(DiagLevel != DiagnosticIDs::Ignored && "Cannot emit ignored diagnostics!");
-
- Diag.Client->HandleDiagnostic((DiagnosticsEngine::Level)DiagLevel, Info);
- if (Diag.Client->IncludeInDiagnosticCounts()) {
- if (DiagLevel == DiagnosticIDs::Warning)
- ++Diag.NumWarnings;
- }
-}
-
bool DiagnosticIDs::isUnrecoverable(unsigned DiagID) const {
// Only errors may be unrecoverable.
if (getDiagClass(DiagID) < CLASS_ERROR)
``````````
</details>
https://github.com/llvm/llvm-project/pull/143517
More information about the cfe-commits
mailing list