[clang] [clang-tools-extra] Revert "Fix OOM in FormatDiagnostic" (PR #108838)

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 16 07:48:38 PDT 2024


https://github.com/AaronBallman created https://github.com/llvm/llvm-project/pull/108838

Reverts llvm/llvm-project#108187

>From dee00441ed4f5d2b872dca9cd0c925e2e5e04107 Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aaron at aaronballman.com>
Date: Mon, 16 Sep 2024 10:48:26 -0400
Subject: [PATCH] Revert "Fix OOM in FormatDiagnostic (#108187)"

This reverts commit e5d255607d200f59c5f7474b8dde6fe72d53e348.
---
 .../ClangTidyDiagnosticConsumer.cpp           |   2 +
 clang/include/clang/Basic/Diagnostic.h        | 270 ++++++++++++------
 clang/include/clang/Basic/DiagnosticIDs.h     |   7 +-
 clang/include/clang/Basic/PartialDiagnostic.h |   5 +-
 clang/include/clang/Sema/Sema.h               |   6 +-
 clang/lib/Basic/Diagnostic.cpp                |  85 +++---
 clang/lib/Basic/DiagnosticIDs.cpp             |  21 +-
 clang/lib/Basic/SourceManager.cpp             |  23 +-
 clang/lib/Frontend/Rewrite/FixItRewriter.cpp  |   4 +-
 clang/lib/Frontend/TextDiagnosticPrinter.cpp  |   2 +-
 clang/lib/Sema/Sema.cpp                       |  19 +-
 clang/lib/Sema/SemaBase.cpp                   |   2 +-
 clang/lib/Serialization/ASTReader.cpp         |  15 +-
 clang/test/PCH/race-condition.cpp             |  41 ---
 clang/unittests/Basic/DiagnosticTest.cpp      |  19 +-
 clang/unittests/Driver/DXCModeTest.cpp        |   5 +
 16 files changed, 300 insertions(+), 226 deletions(-)
 delete mode 100644 clang/test/PCH/race-condition.cpp

diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 4c75b422701148..200bb87a5ac3cb 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -380,6 +380,7 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
     ++Context.Stats.ErrorsIgnoredNOLINT;
     // Ignored a warning, should ignore related notes as well
     LastErrorWasIgnored = true;
+    Context.DiagEngine->Clear();
     for (const auto &Error : SuppressionErrors)
       Context.diag(Error);
     return;
@@ -456,6 +457,7 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
   if (Info.hasSourceManager())
     checkFilters(Info.getLocation(), Info.getSourceManager());
 
+  Context.DiagEngine->Clear();
   for (const auto &Error : SuppressionErrors)
     Context.diag(Error);
 }
diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h
index e17ed8f98afa9a..54b69e98540239 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -183,41 +183,6 @@ struct DiagnosticStorage {
   DiagnosticStorage() = default;
 };
 
-/// An allocator for DiagnosticStorage objects, which uses a small cache to
-/// objects, used to reduce malloc()/free() traffic for partial diagnostics.
-class DiagStorageAllocator {
-  static const unsigned NumCached = 16;
-  DiagnosticStorage Cached[NumCached];
-  DiagnosticStorage *FreeList[NumCached];
-  unsigned NumFreeListEntries;
-
-public:
-  DiagStorageAllocator();
-  ~DiagStorageAllocator();
-
-  /// Allocate new storage.
-  DiagnosticStorage *Allocate() {
-    if (NumFreeListEntries == 0)
-      return new DiagnosticStorage;
-
-    DiagnosticStorage *Result = FreeList[--NumFreeListEntries];
-    Result->NumDiagArgs = 0;
-    Result->DiagRanges.clear();
-    Result->FixItHints.clear();
-    return Result;
-  }
-
-  /// Free the given storage object.
-  void Deallocate(DiagnosticStorage *S) {
-    if (S >= Cached && S <= Cached + NumCached) {
-      FreeList[NumFreeListEntries++] = S;
-      return;
-    }
-
-    delete S;
-  }
-};
-
 /// Concrete class used by the front-end to report problems and issues.
 ///
 /// This massages the diagnostics (e.g. handling things like "report warnings
@@ -557,6 +522,27 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
   void *ArgToStringCookie = nullptr;
   ArgToStringFnTy ArgToStringFn;
 
+  /// ID of the "delayed" diagnostic, which is a (typically
+  /// fatal) diagnostic that had to be delayed because it was found
+  /// while emitting another diagnostic.
+  unsigned DelayedDiagID;
+
+  /// First string argument for the delayed diagnostic.
+  std::string DelayedDiagArg1;
+
+  /// Second string argument for the delayed diagnostic.
+  std::string DelayedDiagArg2;
+
+  /// Third string argument for the delayed diagnostic.
+  std::string DelayedDiagArg3;
+
+  /// Optional flag value.
+  ///
+  /// Some flags accept values, for instance: -Wframe-larger-than=<value> and
+  /// -Rpass=<value>. The content of this string is emitted after the flag name
+  /// and '='.
+  std::string FlagValue;
+
 public:
   explicit DiagnosticsEngine(IntrusiveRefCntPtr<DiagnosticIDs> Diags,
                              IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts,
@@ -963,18 +949,70 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
 
   void Report(const StoredDiagnostic &storedDiag);
 
+  /// Determine whethere there is already a diagnostic in flight.
+  bool isDiagnosticInFlight() const {
+    return CurDiagID != std::numeric_limits<unsigned>::max();
+  }
+
+  /// Set the "delayed" diagnostic that will be emitted once
+  /// the current diagnostic completes.
+  ///
+  ///  If a diagnostic is already in-flight but the front end must
+  ///  report a problem (e.g., with an inconsistent file system
+  ///  state), this routine sets a "delayed" diagnostic that will be
+  ///  emitted after the current diagnostic completes. This should
+  ///  only be used for fatal errors detected at inconvenient
+  ///  times. If emitting a delayed diagnostic causes a second delayed
+  ///  diagnostic to be introduced, that second delayed diagnostic
+  ///  will be ignored.
+  ///
+  /// \param DiagID The ID of the diagnostic being delayed.
+  ///
+  /// \param Arg1 A string argument that will be provided to the
+  /// diagnostic. A copy of this string will be stored in the
+  /// DiagnosticsEngine object itself.
+  ///
+  /// \param Arg2 A string argument that will be provided to the
+  /// diagnostic. A copy of this string will be stored in the
+  /// DiagnosticsEngine object itself.
+  ///
+  /// \param Arg3 A string argument that will be provided to the
+  /// diagnostic. A copy of this string will be stored in the
+  /// DiagnosticsEngine object itself.
+  void SetDelayedDiagnostic(unsigned DiagID, StringRef Arg1 = "",
+                            StringRef Arg2 = "", StringRef Arg3 = "");
+
+  /// Clear out the current diagnostic.
+  void Clear() { CurDiagID = std::numeric_limits<unsigned>::max(); }
+
+  /// Return the value associated with this diagnostic flag.
+  StringRef getFlagValue() const { return FlagValue; }
+
 private:
   // This is private state used by DiagnosticBuilder.  We put it here instead of
   // in DiagnosticBuilder in order to keep DiagnosticBuilder a small lightweight
-  // object.  This implementation choice means that we can only have a few
-  // diagnostics "in flight" at a time, but this seems to be a reasonable
-  // tradeoff to keep these objects small.
+  // object.  This implementation choice means that we can only have one
+  // diagnostic "in flight" at a time, but this seems to be a reasonable
+  // tradeoff to keep these objects small.  Assertions verify that only one
+  // diagnostic is in flight at a time.
   friend class Diagnostic;
   friend class DiagnosticBuilder;
   friend class DiagnosticErrorTrap;
   friend class DiagnosticIDs;
   friend class PartialDiagnostic;
 
+  /// Report the delayed diagnostic.
+  void ReportDelayed();
+
+  /// The location of the current diagnostic that is in flight.
+  SourceLocation CurDiagLoc;
+
+  /// The ID of the current diagnostic that is in flight.
+  ///
+  /// This is set to std::numeric_limits<unsigned>::max() when there is no
+  /// diagnostic in flight.
+  unsigned CurDiagID;
+
   enum {
     /// The maximum number of arguments we can hold.
     ///
@@ -984,7 +1022,7 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
     MaxArguments = DiagnosticStorage::MaxArguments,
   };
 
-  DiagStorageAllocator DiagAllocator;
+  DiagnosticStorage DiagStorage;
 
   DiagnosticMapping makeUserMapping(diag::Severity Map, SourceLocation L) {
     bool isPragma = L.isValid();
@@ -1004,8 +1042,8 @@ 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() {
+    return Diags->ProcessDiag(*this);
   }
 
   /// @name Diagnostic Emission
@@ -1020,10 +1058,14 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
   // Sema::Diag() patterns.
   friend class Sema;
 
-  /// Emit the diagnostic
+  /// Emit the current diagnostic and clear the diagnostic state.
   ///
   /// \param Force Emit the diagnostic regardless of suppression settings.
-  bool EmitDiagnostic(const DiagnosticBuilder &DB, bool Force = false);
+  bool EmitCurrentDiagnostic(bool Force = false);
+
+  unsigned getCurrentDiagID() const { return CurDiagID; }
+
+  SourceLocation getCurrentDiagLoc() const { return CurDiagLoc; }
 
   /// @}
 };
@@ -1076,7 +1118,40 @@ class DiagnosticErrorTrap {
 ///
 class StreamingDiagnostic {
 public:
-  using DiagStorageAllocator = clang::DiagStorageAllocator;
+  /// An allocator for DiagnosticStorage objects, which uses a small cache to
+  /// objects, used to reduce malloc()/free() traffic for partial diagnostics.
+  class DiagStorageAllocator {
+    static const unsigned NumCached = 16;
+    DiagnosticStorage Cached[NumCached];
+    DiagnosticStorage *FreeList[NumCached];
+    unsigned NumFreeListEntries;
+
+  public:
+    DiagStorageAllocator();
+    ~DiagStorageAllocator();
+
+    /// Allocate new storage.
+    DiagnosticStorage *Allocate() {
+      if (NumFreeListEntries == 0)
+        return new DiagnosticStorage;
+
+      DiagnosticStorage *Result = FreeList[--NumFreeListEntries];
+      Result->NumDiagArgs = 0;
+      Result->DiagRanges.clear();
+      Result->FixItHints.clear();
+      return Result;
+    }
+
+    /// Free the given storage object.
+    void Deallocate(DiagnosticStorage *S) {
+      if (S >= Cached && S <= Cached + NumCached) {
+        FreeList[NumFreeListEntries++] = S;
+        return;
+      }
+
+      delete S;
+    }
+  };
 
 protected:
   mutable DiagnosticStorage *DiagStorage = nullptr;
@@ -1165,6 +1240,11 @@ class StreamingDiagnostic {
 protected:
   StreamingDiagnostic() = default;
 
+  /// Construct with an external storage not owned by itself. The allocator
+  /// is a null pointer in this case.
+  explicit StreamingDiagnostic(DiagnosticStorage *Storage)
+      : DiagStorage(Storage) {}
+
   /// Construct with a storage allocator which will manage the storage. The
   /// allocator is not a null pointer in this case.
   explicit StreamingDiagnostic(DiagStorageAllocator &Alloc)
@@ -1195,20 +1275,9 @@ class StreamingDiagnostic {
 class DiagnosticBuilder : public StreamingDiagnostic {
   friend class DiagnosticsEngine;
   friend class PartialDiagnostic;
-  friend class Diagnostic;
 
   mutable DiagnosticsEngine *DiagObj = nullptr;
 
-  SourceLocation DiagLoc;
-  unsigned DiagID;
-
-  /// Optional flag value.
-  ///
-  /// Some flags accept values, for instance: -Wframe-larger-than=<value> and
-  /// -Rpass=<value>. The content of this string is emitted after the flag name
-  /// and '='.
-  mutable std::string FlagValue;
-
   /// Status variable indicating if this diagnostic is still active.
   ///
   // NOTE: This field is redundant with DiagObj (IsActive iff (DiagObj == 0)),
@@ -1222,8 +1291,16 @@ class DiagnosticBuilder : public StreamingDiagnostic {
 
   DiagnosticBuilder() = default;
 
-  DiagnosticBuilder(DiagnosticsEngine *DiagObj, SourceLocation DiagLoc,
-                    unsigned DiagID);
+  explicit DiagnosticBuilder(DiagnosticsEngine *diagObj)
+      : StreamingDiagnostic(&diagObj->DiagStorage), DiagObj(diagObj),
+        IsActive(true) {
+    assert(diagObj && "DiagnosticBuilder requires a valid DiagnosticsEngine!");
+    assert(DiagStorage &&
+           "DiagnosticBuilder requires a valid DiagnosticStorage!");
+    DiagStorage->NumDiagArgs = 0;
+    DiagStorage->DiagRanges.clear();
+    DiagStorage->FixItHints.clear();
+  }
 
 protected:
   /// Clear out the current diagnostic.
@@ -1249,7 +1326,7 @@ class DiagnosticBuilder : public StreamingDiagnostic {
     if (!isActive()) return false;
 
     // Process the diagnostic.
-    bool Result = DiagObj->EmitDiagnostic(*this, IsForceEmit);
+    bool Result = DiagObj->EmitCurrentDiagnostic(IsForceEmit);
 
     // This diagnostic is dead.
     Clear();
@@ -1260,7 +1337,13 @@ class DiagnosticBuilder : public StreamingDiagnostic {
 public:
   /// Copy constructor.  When copied, this "takes" the diagnostic info from the
   /// input and neuters it.
-  DiagnosticBuilder(const DiagnosticBuilder &D);
+  DiagnosticBuilder(const DiagnosticBuilder &D) : StreamingDiagnostic() {
+    DiagObj = D.DiagObj;
+    DiagStorage = D.DiagStorage;
+    IsActive = D.IsActive;
+    IsForceEmit = D.IsForceEmit;
+    D.Clear();
+  }
 
   template <typename T> const DiagnosticBuilder &operator<<(const T &V) const {
     assert(isActive() && "Clients must not add to cleared diagnostic!");
@@ -1292,7 +1375,7 @@ class DiagnosticBuilder : public StreamingDiagnostic {
     return *this;
   }
 
-  void addFlagValue(StringRef V) const { FlagValue = std::string(V); }
+  void addFlagValue(StringRef V) const { DiagObj->FlagValue = std::string(V); }
 };
 
 struct AddFlagValue {
@@ -1467,7 +1550,12 @@ const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
 
 inline DiagnosticBuilder DiagnosticsEngine::Report(SourceLocation Loc,
                                                    unsigned DiagID) {
-  return DiagnosticBuilder(this, Loc, DiagID);
+  assert(CurDiagID == std::numeric_limits<unsigned>::max() &&
+         "Multiple diagnostics in flight at once!");
+  CurDiagLoc = Loc;
+  CurDiagID = DiagID;
+  FlagValue.clear();
+  return DiagnosticBuilder(this);
 }
 
 const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
@@ -1482,29 +1570,24 @@ inline DiagnosticBuilder DiagnosticsEngine::Report(unsigned DiagID) {
 //===----------------------------------------------------------------------===//
 
 /// A little helper class (which is basically a smart pointer that forwards
-/// info from DiagnosticsEngine and DiagnosticStorage) that allows clients to
-/// enquire about the diagnostic.
+/// info from DiagnosticsEngine) that allows clients to enquire about the
+/// currently in-flight diagnostic.
 class Diagnostic {
   const DiagnosticsEngine *DiagObj;
-  SourceLocation DiagLoc;
-  unsigned DiagID;
-  std::string FlagValue;
-  const DiagnosticStorage &DiagStorage;
   std::optional<StringRef> StoredDiagMessage;
 
 public:
-  Diagnostic(const DiagnosticsEngine *DO, const DiagnosticBuilder &DiagBuilder);
-  Diagnostic(const DiagnosticsEngine *DO, SourceLocation DiagLoc,
-             unsigned DiagID, const DiagnosticStorage &DiagStorage,
-             StringRef StoredDiagMessage);
+  explicit Diagnostic(const DiagnosticsEngine *DO) : DiagObj(DO) {}
+  Diagnostic(const DiagnosticsEngine *DO, StringRef storedDiagMessage)
+      : DiagObj(DO), StoredDiagMessage(storedDiagMessage) {}
 
   const DiagnosticsEngine *getDiags() const { return DiagObj; }
-  unsigned getID() const { return DiagID; }
-  const SourceLocation &getLocation() const { return DiagLoc; }
+  unsigned getID() const { return DiagObj->CurDiagID; }
+  const SourceLocation &getLocation() const { return DiagObj->CurDiagLoc; }
   bool hasSourceManager() const { return DiagObj->hasSourceManager(); }
   SourceManager &getSourceManager() const { return DiagObj->getSourceManager();}
 
-  unsigned getNumArgs() const { return DiagStorage.NumDiagArgs; }
+  unsigned getNumArgs() const { return DiagObj->DiagStorage.NumDiagArgs; }
 
   /// Return the kind of the specified index.
   ///
@@ -1514,7 +1597,8 @@ class Diagnostic {
   /// \pre Idx < getNumArgs()
   DiagnosticsEngine::ArgumentKind getArgKind(unsigned Idx) const {
     assert(Idx < getNumArgs() && "Argument index out of range!");
-    return (DiagnosticsEngine::ArgumentKind)DiagStorage.DiagArgumentsKind[Idx];
+    return (DiagnosticsEngine::ArgumentKind)
+        DiagObj->DiagStorage.DiagArgumentsKind[Idx];
   }
 
   /// Return the provided argument string specified by \p Idx.
@@ -1522,7 +1606,7 @@ class Diagnostic {
   const std::string &getArgStdStr(unsigned Idx) const {
     assert(getArgKind(Idx) == DiagnosticsEngine::ak_std_string &&
            "invalid argument accessor!");
-    return DiagStorage.DiagArgumentsStr[Idx];
+    return DiagObj->DiagStorage.DiagArgumentsStr[Idx];
   }
 
   /// Return the specified C string argument.
@@ -1530,7 +1614,8 @@ class Diagnostic {
   const char *getArgCStr(unsigned Idx) const {
     assert(getArgKind(Idx) == DiagnosticsEngine::ak_c_string &&
            "invalid argument accessor!");
-    return reinterpret_cast<const char *>(DiagStorage.DiagArgumentsVal[Idx]);
+    return reinterpret_cast<const char *>(
+        DiagObj->DiagStorage.DiagArgumentsVal[Idx]);
   }
 
   /// Return the specified signed integer argument.
@@ -1538,7 +1623,7 @@ class Diagnostic {
   int64_t getArgSInt(unsigned Idx) const {
     assert(getArgKind(Idx) == DiagnosticsEngine::ak_sint &&
            "invalid argument accessor!");
-    return (int64_t)DiagStorage.DiagArgumentsVal[Idx];
+    return (int64_t)DiagObj->DiagStorage.DiagArgumentsVal[Idx];
   }
 
   /// Return the specified unsigned integer argument.
@@ -1546,7 +1631,7 @@ class Diagnostic {
   uint64_t getArgUInt(unsigned Idx) const {
     assert(getArgKind(Idx) == DiagnosticsEngine::ak_uint &&
            "invalid argument accessor!");
-    return DiagStorage.DiagArgumentsVal[Idx];
+    return DiagObj->DiagStorage.DiagArgumentsVal[Idx];
   }
 
   /// Return the specified IdentifierInfo argument.
@@ -1555,7 +1640,7 @@ class Diagnostic {
     assert(getArgKind(Idx) == DiagnosticsEngine::ak_identifierinfo &&
            "invalid argument accessor!");
     return reinterpret_cast<IdentifierInfo *>(
-        DiagStorage.DiagArgumentsVal[Idx]);
+        DiagObj->DiagStorage.DiagArgumentsVal[Idx]);
   }
 
   /// Return the specified non-string argument in an opaque form.
@@ -1563,32 +1648,37 @@ class Diagnostic {
   uint64_t getRawArg(unsigned Idx) const {
     assert(getArgKind(Idx) != DiagnosticsEngine::ak_std_string &&
            "invalid argument accessor!");
-    return DiagStorage.DiagArgumentsVal[Idx];
+    return DiagObj->DiagStorage.DiagArgumentsVal[Idx];
   }
 
   /// Return the number of source ranges associated with this diagnostic.
-  unsigned getNumRanges() const { return DiagStorage.DiagRanges.size(); }
+  unsigned getNumRanges() const {
+    return DiagObj->DiagStorage.DiagRanges.size();
+  }
 
   /// \pre Idx < getNumRanges()
   const CharSourceRange &getRange(unsigned Idx) const {
     assert(Idx < getNumRanges() && "Invalid diagnostic range index!");
-    return DiagStorage.DiagRanges[Idx];
+    return DiagObj->DiagStorage.DiagRanges[Idx];
   }
 
   /// Return an array reference for this diagnostic's ranges.
-  ArrayRef<CharSourceRange> getRanges() const { return DiagStorage.DiagRanges; }
+  ArrayRef<CharSourceRange> getRanges() const {
+    return DiagObj->DiagStorage.DiagRanges;
+  }
 
-  unsigned getNumFixItHints() const { return DiagStorage.FixItHints.size(); }
+  unsigned getNumFixItHints() const {
+    return DiagObj->DiagStorage.FixItHints.size();
+  }
 
   const FixItHint &getFixItHint(unsigned Idx) const {
     assert(Idx < getNumFixItHints() && "Invalid index!");
-    return DiagStorage.FixItHints[Idx];
+    return DiagObj->DiagStorage.FixItHints[Idx];
   }
 
-  ArrayRef<FixItHint> getFixItHints() const { return DiagStorage.FixItHints; }
-
-  /// Return the value associated with this diagnostic flag.
-  StringRef getFlagValue() const { return FlagValue; }
+  ArrayRef<FixItHint> getFixItHints() const {
+    return DiagObj->DiagStorage.FixItHints;
+  }
 
   /// Format this diagnostic into a string, substituting the
   /// formal arguments into the %0 slots.
diff --git a/clang/include/clang/Basic/DiagnosticIDs.h b/clang/include/clang/Basic/DiagnosticIDs.h
index 1fa38ed6066e26..daad66f499538f 100644
--- a/clang/include/clang/Basic/DiagnosticIDs.h
+++ b/clang/include/clang/Basic/DiagnosticIDs.h
@@ -24,7 +24,6 @@
 
 namespace clang {
   class DiagnosticsEngine;
-  class DiagnosticBuilder;
   class SourceLocation;
 
   // Import the diagnostic enums themselves.
@@ -487,13 +486,11 @@ class DiagnosticIDs : public RefCountedBase<DiagnosticIDs> {
   ///
   /// \returns \c true if the diagnostic was emitted, \c false if it was
   /// suppressed.
-  bool ProcessDiag(DiagnosticsEngine &Diag,
-                   const DiagnosticBuilder &DiagBuilder) const;
+  bool ProcessDiag(DiagnosticsEngine &Diag) const;
 
   /// Used to emit a diagnostic that is finally fully formed,
   /// ignoring suppression.
-  void EmitDiag(DiagnosticsEngine &Diag, const DiagnosticBuilder &DiagBuilder,
-                Level DiagLevel) const;
+  void EmitDiag(DiagnosticsEngine &Diag, Level DiagLevel) const;
 
   /// Whether the diagnostic may leave the AST in a state where some
   /// invariants can break.
diff --git a/clang/include/clang/Basic/PartialDiagnostic.h b/clang/include/clang/Basic/PartialDiagnostic.h
index 4bf6049d08fdb4..507d789c54ff9b 100644
--- a/clang/include/clang/Basic/PartialDiagnostic.h
+++ b/clang/include/clang/Basic/PartialDiagnostic.h
@@ -166,10 +166,13 @@ class PartialDiagnostic : public StreamingDiagnostic {
 
   void EmitToString(DiagnosticsEngine &Diags,
                     SmallVectorImpl<char> &Buf) const {
+    // FIXME: It should be possible to render a diagnostic to a string without
+    //        messing with the state of the diagnostics engine.
     DiagnosticBuilder DB(Diags.Report(getDiagID()));
     Emit(DB);
-    Diagnostic(&Diags, DB).FormatDiagnostic(Buf);
+    Diagnostic(&Diags).FormatDiagnostic(Buf);
     DB.Clear();
+    Diags.Clear();
   }
 
   /// Clear out this partial diagnostic, giving it a new diagnostic ID
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 887d4beea7cfb6..99eef472223a00 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -626,10 +626,10 @@ class Sema final : public SemaBase {
   const llvm::MapVector<FieldDecl *, DeleteLocs> &
   getMismatchingDeleteExpressions() const;
 
-  /// Cause the built diagnostic to be emitted on the DiagosticsEngine.
-  /// This is closely coupled to the SemaDiagnosticBuilder class and
+  /// Cause the active diagnostic on the DiagosticsEngine to be
+  /// emitted. This is closely coupled to the SemaDiagnosticBuilder class and
   /// should not be used elsewhere.
-  void EmitDiagnostic(unsigned DiagID, const DiagnosticBuilder &DB);
+  void EmitCurrentDiagnostic(unsigned DiagID);
 
   void addImplicitTypedef(StringRef Name, QualType T);
 
diff --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp
index ffb3cd42707700..ecff80a5090630 100644
--- a/clang/lib/Basic/Diagnostic.cpp
+++ b/clang/lib/Basic/Diagnostic.cpp
@@ -126,7 +126,9 @@ void DiagnosticsEngine::Reset(bool soft /*=false*/) {
   TrapNumErrorsOccurred = 0;
   TrapNumUnrecoverableErrorsOccurred = 0;
 
+  CurDiagID = std::numeric_limits<unsigned>::max();
   LastDiagLevel = DiagnosticIDs::Ignored;
+  DelayedDiagID = 0;
 
   if (!soft) {
     // Clear state related to #pragma diagnostic.
@@ -141,6 +143,23 @@ void DiagnosticsEngine::Reset(bool soft /*=false*/) {
   }
 }
 
+void DiagnosticsEngine::SetDelayedDiagnostic(unsigned DiagID, StringRef Arg1,
+                                             StringRef Arg2, StringRef Arg3) {
+  if (DelayedDiagID)
+    return;
+
+  DelayedDiagID = DiagID;
+  DelayedDiagArg1 = Arg1.str();
+  DelayedDiagArg2 = Arg2.str();
+  DelayedDiagArg3 = Arg3.str();
+}
+
+void DiagnosticsEngine::ReportDelayed() {
+  unsigned ID = DelayedDiagID;
+  DelayedDiagID = 0;
+  Report(ID) << DelayedDiagArg1 << DelayedDiagArg2 << DelayedDiagArg3;
+}
+
 DiagnosticMapping &
 DiagnosticsEngine::DiagState::getOrAddMapping(diag::kind Diag) {
   std::pair<iterator, bool> Result =
@@ -484,31 +503,39 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor Flavor,
 }
 
 void DiagnosticsEngine::Report(const StoredDiagnostic &storedDiag) {
-  DiagnosticStorage DiagStorage;
+  assert(CurDiagID == std::numeric_limits<unsigned>::max() &&
+         "Multiple diagnostics in flight at once!");
+
+  CurDiagLoc = storedDiag.getLocation();
+  CurDiagID = storedDiag.getID();
+  DiagStorage.NumDiagArgs = 0;
+
+  DiagStorage.DiagRanges.clear();
   DiagStorage.DiagRanges.append(storedDiag.range_begin(),
                                 storedDiag.range_end());
 
+  DiagStorage.FixItHints.clear();
   DiagStorage.FixItHints.append(storedDiag.fixit_begin(),
                                 storedDiag.fixit_end());
 
   assert(Client && "DiagnosticConsumer not set!");
   Level DiagLevel = storedDiag.getLevel();
-  Diagnostic Info(this, storedDiag.getLocation(), storedDiag.getID(),
-                  DiagStorage, storedDiag.getMessage());
+  Diagnostic Info(this, storedDiag.getMessage());
   Client->HandleDiagnostic(DiagLevel, Info);
   if (Client->IncludeInDiagnosticCounts()) {
     if (DiagLevel == DiagnosticsEngine::Warning)
       ++NumWarnings;
   }
+
+  CurDiagID = std::numeric_limits<unsigned>::max();
 }
 
-bool DiagnosticsEngine::EmitDiagnostic(const DiagnosticBuilder &DB,
-                                       bool Force) {
+bool DiagnosticsEngine::EmitCurrentDiagnostic(bool Force) {
   assert(getClient() && "DiagnosticClient not set!");
 
   bool Emitted;
   if (Force) {
-    Diagnostic Info(this, DB);
+    Diagnostic Info(this);
 
     // Figure out the diagnostic level of this message.
     DiagnosticIDs::Level DiagLevel
@@ -517,50 +544,24 @@ bool DiagnosticsEngine::EmitDiagnostic(const DiagnosticBuilder &DB,
     Emitted = (DiagLevel != DiagnosticIDs::Ignored);
     if (Emitted) {
       // Emit the diagnostic regardless of suppression level.
-      Diags->EmitDiag(*this, DB, DiagLevel);
+      Diags->EmitDiag(*this, DiagLevel);
     }
   } else {
     // Process the diagnostic, sending the accumulated information to the
     // DiagnosticConsumer.
-    Emitted = ProcessDiag(DB);
+    Emitted = ProcessDiag();
   }
 
-  return Emitted;
-}
+  // Clear out the current diagnostic object.
+  Clear();
 
-DiagnosticBuilder::DiagnosticBuilder(DiagnosticsEngine *DiagObj,
-                                     SourceLocation DiagLoc, unsigned DiagID)
-    : StreamingDiagnostic(DiagObj->DiagAllocator), DiagObj(DiagObj),
-      DiagLoc(DiagLoc), DiagID(DiagID), IsActive(true) {
-  assert(DiagObj && "DiagnosticBuilder requires a valid DiagnosticsEngine!");
-}
+  // If there was a delayed diagnostic, emit it now.
+  if (!Force && DelayedDiagID)
+    ReportDelayed();
 
-DiagnosticBuilder::DiagnosticBuilder(const DiagnosticBuilder &D)
-    : StreamingDiagnostic() {
-  DiagLoc = D.DiagLoc;
-  DiagID = D.DiagID;
-  FlagValue = D.FlagValue;
-  DiagObj = D.DiagObj;
-  DiagStorage = D.DiagStorage;
-  D.DiagStorage = nullptr;
-  Allocator = D.Allocator;
-  IsActive = D.IsActive;
-  IsForceEmit = D.IsForceEmit;
-  D.Clear();
+  return Emitted;
 }
 
-Diagnostic::Diagnostic(const DiagnosticsEngine *DO,
-                       const DiagnosticBuilder &DiagBuilder)
-    : DiagObj(DO), DiagLoc(DiagBuilder.DiagLoc),
-      DiagID(DiagBuilder.DiagID), FlagValue(DiagBuilder.FlagValue),
-      DiagStorage(*DiagBuilder.getStorage()) {}
-
-Diagnostic::Diagnostic(const DiagnosticsEngine *DO, SourceLocation DiagLoc,
-                       unsigned DiagID, const DiagnosticStorage &DiagStorage,
-                       StringRef StoredDiagMessage)
-    : DiagObj(DO), DiagLoc(DiagLoc), DiagID(DiagID),
-      DiagStorage(DiagStorage), StoredDiagMessage(StoredDiagMessage) {}
-
 DiagnosticConsumer::~DiagnosticConsumer() = default;
 
 void DiagnosticConsumer::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
@@ -1215,13 +1216,13 @@ bool ForwardingDiagnosticConsumer::IncludeInDiagnosticCounts() const {
   return Target.IncludeInDiagnosticCounts();
 }
 
-DiagStorageAllocator::DiagStorageAllocator() {
+PartialDiagnostic::DiagStorageAllocator::DiagStorageAllocator() {
   for (unsigned I = 0; I != NumCached; ++I)
     FreeList[I] = Cached + I;
   NumFreeListEntries = NumCached;
 }
 
-DiagStorageAllocator::~DiagStorageAllocator() {
+PartialDiagnostic::DiagStorageAllocator::~DiagStorageAllocator() {
   // Don't assert if we are in a CrashRecovery context, as this invariant may
   // be invalidated during a crash.
   assert((NumFreeListEntries == NumCached ||
diff --git a/clang/lib/Basic/DiagnosticIDs.cpp b/clang/lib/Basic/DiagnosticIDs.cpp
index 031d9d7817d1f6..cae6642bd4bd3e 100644
--- a/clang/lib/Basic/DiagnosticIDs.cpp
+++ b/clang/lib/Basic/DiagnosticIDs.cpp
@@ -566,7 +566,7 @@ DiagnosticIDs::getDiagnosticSeverity(unsigned DiagID, SourceLocation Loc,
 
   // If explicitly requested, map fatal errors to errors.
   if (Result == diag::Severity::Fatal &&
-      DiagID != diag::fatal_too_many_errors && Diag.FatalsAsError)
+      Diag.CurDiagID != diag::fatal_too_many_errors && Diag.FatalsAsError)
     Result = diag::Severity::Error;
 
   bool ShowInSystemHeader =
@@ -800,9 +800,8 @@ StringRef DiagnosticIDs::getNearestOption(diag::Flavor Flavor,
 
 /// 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);
+bool DiagnosticIDs::ProcessDiag(DiagnosticsEngine &Diag) const {
+  Diagnostic Info(&Diag);
 
   assert(Diag.getClient() && "DiagnosticClient not set!");
 
@@ -868,24 +867,22 @@ bool DiagnosticIDs::ProcessDiag(DiagnosticsEngine &Diag,
     // stop a flood of bogus errors.
     if (Diag.ErrorLimit && Diag.NumErrors > Diag.ErrorLimit &&
         DiagLevel == DiagnosticIDs::Error) {
-      Diag.Report(diag::fatal_too_many_errors);
+      Diag.SetDelayedDiagnostic(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)
+  if (Diag.CurDiagID == diag::fatal_too_many_errors)
     Diag.FatalErrorOccurred = true;
   // Finally, report it.
-  EmitDiag(Diag, DiagBuilder, DiagLevel);
+  EmitDiag(Diag, DiagLevel);
   return true;
 }
 
-void DiagnosticIDs::EmitDiag(DiagnosticsEngine &Diag,
-                             const DiagnosticBuilder &DiagBuilder,
-                             Level DiagLevel) const {
-  Diagnostic Info(&Diag, DiagBuilder);
+void DiagnosticIDs::EmitDiag(DiagnosticsEngine &Diag, Level DiagLevel) const {
+  Diagnostic Info(&Diag);
   assert(DiagLevel != DiagnosticIDs::Ignored && "Cannot emit ignored diagnostics!");
 
   Diag.Client->HandleDiagnostic((DiagnosticsEngine::Level)DiagLevel, Info);
@@ -893,6 +890,8 @@ void DiagnosticIDs::EmitDiag(DiagnosticsEngine &Diag,
     if (DiagLevel == DiagnosticIDs::Warning)
       ++Diag.NumWarnings;
   }
+
+  Diag.CurDiagID = ~0U;
 }
 
 bool DiagnosticIDs::isUnrecoverable(unsigned DiagID) const {
diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp
index 65a8a7253e054f..d6ec26af80aadd 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -130,8 +130,13 @@ ContentCache::getBufferOrNone(DiagnosticsEngine &Diag, FileManager &FM,
   // the file could also have been removed during processing. Since we can't
   // really deal with this situation, just create an empty buffer.
   if (!BufferOrError) {
-    Diag.Report(Loc, diag::err_cannot_open_file)
-        << ContentsEntry->getName() << BufferOrError.getError().message();
+    if (Diag.isDiagnosticInFlight())
+      Diag.SetDelayedDiagnostic(diag::err_cannot_open_file,
+                                ContentsEntry->getName(),
+                                BufferOrError.getError().message());
+    else
+      Diag.Report(Loc, diag::err_cannot_open_file)
+          << ContentsEntry->getName() << BufferOrError.getError().message();
 
     return std::nullopt;
   }
@@ -148,7 +153,12 @@ ContentCache::getBufferOrNone(DiagnosticsEngine &Diag, FileManager &FM,
   // ContentsEntry::getSize() could have the wrong size. Use
   // MemoryBuffer::getBufferSize() instead.
   if (Buffer->getBufferSize() >= std::numeric_limits<unsigned>::max()) {
-    Diag.Report(Loc, diag::err_file_too_large) << ContentsEntry->getName();
+    if (Diag.isDiagnosticInFlight())
+      Diag.SetDelayedDiagnostic(diag::err_file_too_large,
+                                ContentsEntry->getName());
+    else
+      Diag.Report(Loc, diag::err_file_too_large)
+        << ContentsEntry->getName();
 
     return std::nullopt;
   }
@@ -158,7 +168,12 @@ ContentCache::getBufferOrNone(DiagnosticsEngine &Diag, FileManager &FM,
   // have come from a stat cache).
   if (!ContentsEntry->isNamedPipe() &&
       Buffer->getBufferSize() != (size_t)ContentsEntry->getSize()) {
-    Diag.Report(Loc, diag::err_file_modified) << ContentsEntry->getName();
+    if (Diag.isDiagnosticInFlight())
+      Diag.SetDelayedDiagnostic(diag::err_file_modified,
+                                ContentsEntry->getName());
+    else
+      Diag.Report(Loc, diag::err_file_modified)
+        << ContentsEntry->getName();
 
     return std::nullopt;
   }
diff --git a/clang/lib/Frontend/Rewrite/FixItRewriter.cpp b/clang/lib/Frontend/Rewrite/FixItRewriter.cpp
index 7309553e3bc0b4..44dfaf20eae73f 100644
--- a/clang/lib/Frontend/Rewrite/FixItRewriter.cpp
+++ b/clang/lib/Frontend/Rewrite/FixItRewriter.cpp
@@ -200,8 +200,10 @@ void FixItRewriter::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
 /// Emit a diagnostic via the adapted diagnostic client.
 void FixItRewriter::Diag(SourceLocation Loc, unsigned DiagID) {
   // When producing this diagnostic, we temporarily bypass ourselves,
-  // and let the downstream client format the diagnostic.
+  // clear out any current diagnostic, and let the downstream client
+  // format the diagnostic.
   Diags.setClient(Client, false);
+  Diags.Clear();
   Diags.Report(Loc, DiagID);
   Diags.setClient(this, false);
 }
diff --git a/clang/lib/Frontend/TextDiagnosticPrinter.cpp b/clang/lib/Frontend/TextDiagnosticPrinter.cpp
index 28f7218dc23f54..c2fea3d03f0c0f 100644
--- a/clang/lib/Frontend/TextDiagnosticPrinter.cpp
+++ b/clang/lib/Frontend/TextDiagnosticPrinter.cpp
@@ -84,7 +84,7 @@ static void printDiagnosticOptions(raw_ostream &OS,
     if (!Opt.empty()) {
       OS << (Started ? "," : " [")
          << (Level == DiagnosticsEngine::Remark ? "-R" : "-W") << Opt;
-      StringRef OptValue = Info.getFlagValue();
+      StringRef OptValue = Info.getDiags()->getFlagValue();
       if (!OptValue.empty())
         OS << "=" << OptValue;
       Started = true;
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index e950ad675072ab..d567de703903c9 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -1589,7 +1589,7 @@ LangAS Sema::getDefaultCXXMethodAddrSpace() const {
   return LangAS::Default;
 }
 
-void Sema::EmitDiagnostic(unsigned DiagID, const DiagnosticBuilder &DB) {
+void Sema::EmitCurrentDiagnostic(unsigned DiagID) {
   // FIXME: It doesn't make sense to me that DiagID is an incoming argument here
   // and yet we also use the current diag ID on the DiagnosticsEngine. This has
   // been made more painfully obvious by the refactor that introduced this
@@ -1597,9 +1597,9 @@ void Sema::EmitDiagnostic(unsigned DiagID, const DiagnosticBuilder &DB) {
   // eliminated. If it truly cannot be (for example, there is some reentrancy
   // issue I am not seeing yet), then there should at least be a clarifying
   // comment somewhere.
-  Diagnostic DiagInfo(&Diags, DB);
   if (std::optional<TemplateDeductionInfo *> Info = isSFINAEContext()) {
-    switch (DiagnosticIDs::getDiagnosticSFINAEResponse(DiagInfo.getID())) {
+    switch (DiagnosticIDs::getDiagnosticSFINAEResponse(
+              Diags.getCurrentDiagID())) {
     case DiagnosticIDs::SFINAE_Report:
       // We'll report the diagnostic below.
       break;
@@ -1612,11 +1612,13 @@ void Sema::EmitDiagnostic(unsigned DiagID, const DiagnosticBuilder &DB) {
       // Make a copy of this suppressed diagnostic and store it with the
       // template-deduction information.
       if (*Info && !(*Info)->hasSFINAEDiagnostic()) {
+        Diagnostic DiagInfo(&Diags);
         (*Info)->addSFINAEDiagnostic(DiagInfo.getLocation(),
                        PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
       }
 
       Diags.setLastDiagnosticIgnored(true);
+      Diags.Clear();
       return;
 
     case DiagnosticIDs::SFINAE_AccessControl: {
@@ -1627,7 +1629,7 @@ void Sema::EmitDiagnostic(unsigned DiagID, const DiagnosticBuilder &DB) {
       if (!AccessCheckingSFINAE && !getLangOpts().CPlusPlus11)
         break;
 
-      SourceLocation Loc = DiagInfo.getLocation();
+      SourceLocation Loc = Diags.getCurrentDiagLoc();
 
       // Suppress this diagnostic.
       ++NumSFINAEErrors;
@@ -1635,13 +1637,16 @@ void Sema::EmitDiagnostic(unsigned DiagID, const DiagnosticBuilder &DB) {
       // Make a copy of this suppressed diagnostic and store it with the
       // template-deduction information.
       if (*Info && !(*Info)->hasSFINAEDiagnostic()) {
+        Diagnostic DiagInfo(&Diags);
         (*Info)->addSFINAEDiagnostic(DiagInfo.getLocation(),
                        PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
       }
 
       Diags.setLastDiagnosticIgnored(true);
+      Diags.Clear();
 
-      // Now produce a C++98 compatibility warning.
+      // Now the diagnostic state is clear, produce a C++98 compatibility
+      // warning.
       Diag(Loc, diag::warn_cxx98_compat_sfinae_access_control);
 
       // The last diagnostic which Sema produced was ignored. Suppress any
@@ -1654,12 +1659,14 @@ void Sema::EmitDiagnostic(unsigned DiagID, const DiagnosticBuilder &DB) {
       // Make a copy of this suppressed diagnostic and store it with the
       // template-deduction information;
       if (*Info) {
+        Diagnostic DiagInfo(&Diags);
         (*Info)->addSuppressedDiagnostic(DiagInfo.getLocation(),
                        PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
       }
 
       // Suppress this diagnostic.
       Diags.setLastDiagnosticIgnored(true);
+      Diags.Clear();
       return;
     }
   }
@@ -1669,7 +1676,7 @@ void Sema::EmitDiagnostic(unsigned DiagID, const DiagnosticBuilder &DB) {
   Context.setPrintingPolicy(getPrintingPolicy());
 
   // Emit the diagnostic.
-  if (!Diags.EmitDiagnostic(DB))
+  if (!Diags.EmitCurrentDiagnostic())
     return;
 
   // If this is not a note, and we're in a template instantiation
diff --git a/clang/lib/Sema/SemaBase.cpp b/clang/lib/Sema/SemaBase.cpp
index 5c24f21b469b01..a2f12d622e8ccc 100644
--- a/clang/lib/Sema/SemaBase.cpp
+++ b/clang/lib/Sema/SemaBase.cpp
@@ -26,7 +26,7 @@ SemaBase::ImmediateDiagBuilder::~ImmediateDiagBuilder() {
   Clear();
 
   // Dispatch to Sema to emit the diagnostic.
-  SemaRef.EmitDiagnostic(DiagID, *this);
+  SemaRef.EmitCurrentDiagnostic(DiagID);
 }
 
 PartialDiagnostic SemaBase::PDiag(unsigned DiagID) {
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 7efcc81e194d95..4fae6ff02ea9a3 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -1382,7 +1382,7 @@ bool ASTReader::ReadVisibleDeclContextStorage(ModuleFile &M,
 
 void ASTReader::Error(StringRef Msg) const {
   Error(diag::err_fe_pch_malformed, Msg);
-  if (PP.getLangOpts().Modules &&
+  if (PP.getLangOpts().Modules && !Diags.isDiagnosticInFlight() &&
       !PP.getHeaderSearchInfo().getModuleCachePath().empty()) {
     Diag(diag::note_module_cache_path)
       << PP.getHeaderSearchInfo().getModuleCachePath();
@@ -1391,7 +1391,10 @@ void ASTReader::Error(StringRef Msg) const {
 
 void ASTReader::Error(unsigned DiagID, StringRef Arg1, StringRef Arg2,
                       StringRef Arg3) const {
-  Diag(DiagID) << Arg1 << Arg2 << Arg3;
+  if (Diags.isDiagnosticInFlight())
+    Diags.SetDelayedDiagnostic(DiagID, Arg1, Arg2, Arg3);
+  else
+    Diag(DiagID) << Arg1 << Arg2 << Arg3;
 }
 
 void ASTReader::Error(llvm::Error &&Err) const {
@@ -2710,7 +2713,7 @@ InputFile ASTReader::getInputFile(ModuleFile &F, unsigned ID, bool Complain) {
 
   // For an overridden file, there is nothing to validate.
   if (!Overridden && FileChange.Kind != Change::None) {
-    if (Complain) {
+    if (Complain && !Diags.isDiagnosticInFlight()) {
       // Build a list of the PCH imports that got us here (in reverse).
       SmallVector<ModuleFile *, 4> ImportStack(1, &F);
       while (!ImportStack.back()->ImportedBy.empty())
@@ -3686,8 +3689,10 @@ llvm::Error ASTReader::ReadASTBlock(ModuleFile &F,
           SourceMgr.AllocateLoadedSLocEntries(F.LocalNumSLocEntries,
                                               SLocSpaceSize);
       if (!F.SLocEntryBaseID) {
-        Diags.Report(SourceLocation(), diag::remark_sloc_usage);
-        SourceMgr.noteSLocAddressSpaceUsage(Diags);
+        if (!Diags.isDiagnosticInFlight()) {
+          Diags.Report(SourceLocation(), diag::remark_sloc_usage);
+          SourceMgr.noteSLocAddressSpaceUsage(Diags);
+        }
         return llvm::createStringError(std::errc::invalid_argument,
                                        "ran out of source locations");
       }
diff --git a/clang/test/PCH/race-condition.cpp b/clang/test/PCH/race-condition.cpp
deleted file mode 100644
index 752b0cc3ff6286..00000000000000
--- a/clang/test/PCH/race-condition.cpp
+++ /dev/null
@@ -1,41 +0,0 @@
-// RUN: %clang_cc1 -fallow-pch-with-compiler-errors -std=c++20 -x c++-header -emit-pch %s -o %t -verify
-// RUN: %clang_cc1 -fallow-pch-with-compiler-errors -std=c++20 -include-pch %t %s -verify
-#ifndef HEADER_H
-#define HEADER_H
-
-#include "bad_include.h"
-// expected-error at 6{{'bad_include.h' file not found}}
-
-template <bool, class = void> struct enable_if {};
-template <class T> struct enable_if<true, T> { typedef T type; };
-template <bool B, class T = void> using enable_if_t = typename enable_if<B, T>::type;
-
-template <typename> struct meta { static constexpr int value = 0; };
-template <> struct meta<int> { static constexpr int value = 1; };
-template <> struct meta<float> { static constexpr int value = 2; };
-
-namespace N {
-inline namespace inner {
-
-template <class T>
-constexpr enable_if_t<meta<T>::value == 0, void> midpoint(T) {}
-
-template <class U>
-constexpr enable_if_t<meta<U>::value == 1, void> midpoint(U) {}
-
-template <class F>
-constexpr enable_if_t<meta<F>::value == 2, void> midpoint(F) {}
-
-} // namespace inner
-} // namespace N
-
-#else
-
-// expected-error at 27{{'N::midpoint' has different definitions in different modules; defined here first difference is 1st parameter with type 'F'}}
-// expected-error at 24{{'N::midpoint' has different definitions in different modules; defined here first difference is 1st parameter with type 'U'}}
-// expected-note at 21{{but in '' found 1st parameter with type 'T'}}
-int x = N::something;
-// expected-error at 37{{no member named 'something' in namespace 'N'}}
-// expected-note at 21{{but in '' found 1st parameter with type 'T'}}
-
-#endif
diff --git a/clang/unittests/Basic/DiagnosticTest.cpp b/clang/unittests/Basic/DiagnosticTest.cpp
index 691d74f697f278..74690193917165 100644
--- a/clang/unittests/Basic/DiagnosticTest.cpp
+++ b/clang/unittests/Basic/DiagnosticTest.cpp
@@ -17,6 +17,9 @@ using namespace llvm;
 using namespace clang;
 
 void clang::DiagnosticsTestHelper(DiagnosticsEngine &diag) {
+  unsigned delayedDiagID = 0U;
+
+  EXPECT_EQ(diag.DelayedDiagID, delayedDiagID);
   EXPECT_FALSE(diag.DiagStates.empty());
   EXPECT_TRUE(diag.DiagStatesByLoc.empty());
   EXPECT_TRUE(diag.DiagStateOnPushStack.empty());
@@ -80,21 +83,6 @@ TEST(DiagnosticTest, fatalsAsError) {
   }
 }
 
-TEST(DiagnosticTest, tooManyErrorsIsAlwaysFatal) {
-  DiagnosticsEngine Diags(new DiagnosticIDs(), new DiagnosticOptions,
-                          new IgnoringDiagConsumer());
-  Diags.setFatalsAsError(true);
-
-  // Report a fatal_too_many_errors diagnostic to ensure that still
-  // acts as a fatal error despite downgrading fatal errors to errors.
-  Diags.Report(diag::fatal_too_many_errors);
-  EXPECT_TRUE(Diags.hasFatalErrorOccurred());
-
-  // Ensure that the severity of that diagnostic is really "fatal".
-  EXPECT_EQ(Diags.getDiagnosticLevel(diag::fatal_too_many_errors, {}),
-            DiagnosticsEngine::Level::Fatal);
-}
-
 // Check that soft RESET works as intended
 TEST(DiagnosticTest, softReset) {
   DiagnosticsEngine Diags(new DiagnosticIDs(), new DiagnosticOptions,
@@ -116,6 +104,7 @@ TEST(DiagnosticTest, softReset) {
   // Check for private variables of DiagnosticsEngine differentiating soft reset
   DiagnosticsTestHelper(Diags);
 
+  EXPECT_FALSE(Diags.isDiagnosticInFlight());
   EXPECT_TRUE(Diags.isLastDiagnosticIgnored());
 }
 
diff --git a/clang/unittests/Driver/DXCModeTest.cpp b/clang/unittests/Driver/DXCModeTest.cpp
index 2a079a62f1bc13..41ab30bc81d5f9 100644
--- a/clang/unittests/Driver/DXCModeTest.cpp
+++ b/clang/unittests/Driver/DXCModeTest.cpp
@@ -51,6 +51,7 @@ static void validateTargetProfile(
   EXPECT_TRUE(C);
   EXPECT_EQ(Diags.getNumErrors(), NumOfErrors);
   EXPECT_STREQ(DiagConsumer->Errors.back().c_str(), ExpectError.data());
+  Diags.Clear();
   DiagConsumer->clear();
 }
 
@@ -159,6 +160,7 @@ TEST(DxcModeTest, ValidatorVersionValidation) {
       DiagConsumer->Errors.back().c_str(),
       "invalid validator version : 0.1; if validator major version is 0, "
       "minor version must also be 0");
+  Diags.Clear();
   DiagConsumer->clear();
 
   Args = TheDriver.ParseArgStrings({"-validator-version", "1"}, false,
@@ -174,6 +176,7 @@ TEST(DxcModeTest, ValidatorVersionValidation) {
   EXPECT_STREQ(DiagConsumer->Errors.back().c_str(),
                "invalid validator version : 1; format of validator version is "
                "\"<major>.<minor>\" (ex:\"1.4\")");
+  Diags.Clear();
   DiagConsumer->clear();
 
   Args = TheDriver.ParseArgStrings({"-validator-version", "-Tlib_6_7"}, false,
@@ -190,6 +193,7 @@ TEST(DxcModeTest, ValidatorVersionValidation) {
       DiagConsumer->Errors.back().c_str(),
       "invalid validator version : -Tlib_6_7; format of validator version is "
       "\"<major>.<minor>\" (ex:\"1.4\")");
+  Diags.Clear();
   DiagConsumer->clear();
 
   Args = TheDriver.ParseArgStrings({"-validator-version", "foo"}, false,
@@ -206,6 +210,7 @@ TEST(DxcModeTest, ValidatorVersionValidation) {
       DiagConsumer->Errors.back().c_str(),
       "invalid validator version : foo; format of validator version is "
       "\"<major>.<minor>\" (ex:\"1.4\")");
+  Diags.Clear();
   DiagConsumer->clear();
 }
 



More information about the cfe-commits mailing list