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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
