[clang] [Clang] [NFC] Move diagnostics emitting code from `DiagnosticIDs` into `DiagnosticsEngine` (PR #143517)
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)
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)
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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
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)
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)
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)
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