[flang-commits] [flang] eda72fa - Fix OOM in FormatDiagnostic (2nd attempt) (#108866)
via flang-commits
flang-commits at lists.llvm.org
Wed Sep 18 08:46:28 PDT 2024
Author: Vakhurin Sergei
Date: 2024-09-18T11:46:25-04:00
New Revision: eda72fac548f317cec997967494763e9a7bafa27
URL: https://github.com/llvm/llvm-project/commit/eda72fac548f317cec997967494763e9a7bafa27
DIFF: https://github.com/llvm/llvm-project/commit/eda72fac548f317cec997967494763e9a7bafa27.diff
LOG: Fix OOM in FormatDiagnostic (2nd attempt) (#108866)
Resolves: #70930 (and probably latest comments from clangd/clangd#251)
by fixing racing for the shared DiagStorage value which caused messing with args inside the storage and then formatting the following message with getArgSInt(1) == 2:
def err_module_odr_violation_function : Error<
"%q0 has different definitions in different modules; "
"%select{definition in module '%2'|defined here}1 "
"first difference is "
which causes HandleSelectModifier to go beyond the ArgumentLen so the recursive call to FormatDiagnostic was made with DiagStr > DiagEnd that leads to infinite while (DiagStr != DiagEnd).
The Main Idea:
Reuse the existing DiagStorageAllocator logic to make all DiagnosticBuilders having independent states.
Also, encapsulating the rest of state (e.g. ID and Loc) into DiagnosticBuilder.
The last attempt failed -
https://github.com/llvm/llvm-project/pull/108187#issuecomment-2353122096
so was reverted - #108838
Added:
clang/test/PCH/race-condition.cpp
Modified:
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
clang/include/clang/Basic/Diagnostic.h
clang/include/clang/Basic/DiagnosticIDs.h
clang/include/clang/Basic/PartialDiagnostic.h
clang/include/clang/Sema/Sema.h
clang/lib/Basic/Diagnostic.cpp
clang/lib/Basic/DiagnosticIDs.cpp
clang/lib/Basic/SourceManager.cpp
clang/lib/Frontend/Rewrite/FixItRewriter.cpp
clang/lib/Frontend/TextDiagnosticPrinter.cpp
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaBase.cpp
clang/lib/Serialization/ASTReader.cpp
clang/unittests/Basic/DiagnosticTest.cpp
clang/unittests/Driver/DXCModeTest.cpp
flang/lib/Frontend/TextDiagnosticPrinter.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 200bb87a5ac3cb..4c75b422701148 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -380,7 +380,6 @@ 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;
@@ -457,7 +456,6 @@ 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-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
index 021d731f8f1768..cf9b42828568da 100644
--- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -305,33 +305,33 @@ TEST_F(ConfigCompileTests, DiagnosticSuppression) {
{
auto D = DiagEngine.Report(diag::warn_unreachable);
EXPECT_TRUE(isDiagnosticSuppressed(
- Diag{&DiagEngine}, Conf.Diagnostics.Suppress, LangOptions()));
+ Diag{&DiagEngine, D}, Conf.Diagnostics.Suppress, LangOptions()));
}
// Subcategory not respected/suppressed.
{
auto D = DiagEngine.Report(diag::warn_unreachable_break);
EXPECT_FALSE(isDiagnosticSuppressed(
- Diag{&DiagEngine}, Conf.Diagnostics.Suppress, LangOptions()));
+ Diag{&DiagEngine, D}, Conf.Diagnostics.Suppress, LangOptions()));
}
{
auto D = DiagEngine.Report(diag::warn_unused_variable);
EXPECT_TRUE(isDiagnosticSuppressed(
- Diag{&DiagEngine}, Conf.Diagnostics.Suppress, LangOptions()));
+ Diag{&DiagEngine, D}, Conf.Diagnostics.Suppress, LangOptions()));
}
{
auto D = DiagEngine.Report(diag::err_typecheck_bool_condition);
EXPECT_TRUE(isDiagnosticSuppressed(
- Diag{&DiagEngine}, Conf.Diagnostics.Suppress, LangOptions()));
+ Diag{&DiagEngine, D}, Conf.Diagnostics.Suppress, LangOptions()));
}
{
auto D = DiagEngine.Report(diag::err_unexpected_friend);
EXPECT_TRUE(isDiagnosticSuppressed(
- Diag{&DiagEngine}, Conf.Diagnostics.Suppress, LangOptions()));
+ Diag{&DiagEngine, D}, Conf.Diagnostics.Suppress, LangOptions()));
}
{
auto D = DiagEngine.Report(diag::warn_alloca);
EXPECT_TRUE(isDiagnosticSuppressed(
- Diag{&DiagEngine}, Conf.Diagnostics.Suppress, LangOptions()));
+ Diag{&DiagEngine, D}, Conf.Diagnostics.Suppress, LangOptions()));
}
Frag.Diagnostics.Suppress.emplace_back("*");
diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h
index 54b69e98540239..e17ed8f98afa9a 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -183,6 +183,41 @@ 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
@@ -522,27 +557,6 @@ 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,
@@ -949,70 +963,18 @@ 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 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.
+ // 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.
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.
///
@@ -1022,7 +984,7 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
MaxArguments = DiagnosticStorage::MaxArguments,
};
- DiagnosticStorage DiagStorage;
+ DiagStorageAllocator DiagAllocator;
DiagnosticMapping makeUserMapping(diag::Severity Map, SourceLocation L) {
bool isPragma = L.isValid();
@@ -1042,8 +1004,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() {
- return Diags->ProcessDiag(*this);
+ bool ProcessDiag(const DiagnosticBuilder &DiagBuilder) {
+ return Diags->ProcessDiag(*this, DiagBuilder);
}
/// @name Diagnostic Emission
@@ -1058,14 +1020,10 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
// Sema::Diag() patterns.
friend class Sema;
- /// Emit the current diagnostic and clear the diagnostic state.
+ /// Emit the diagnostic
///
/// \param Force Emit the diagnostic regardless of suppression settings.
- bool EmitCurrentDiagnostic(bool Force = false);
-
- unsigned getCurrentDiagID() const { return CurDiagID; }
-
- SourceLocation getCurrentDiagLoc() const { return CurDiagLoc; }
+ bool EmitDiagnostic(const DiagnosticBuilder &DB, bool Force = false);
/// @}
};
@@ -1118,40 +1076,7 @@ class DiagnosticErrorTrap {
///
class StreamingDiagnostic {
public:
- /// 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;
- }
- };
+ using DiagStorageAllocator = clang::DiagStorageAllocator;
protected:
mutable DiagnosticStorage *DiagStorage = nullptr;
@@ -1240,11 +1165,6 @@ 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)
@@ -1275,9 +1195,20 @@ 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)),
@@ -1291,16 +1222,8 @@ class DiagnosticBuilder : public StreamingDiagnostic {
DiagnosticBuilder() = default;
- 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();
- }
+ DiagnosticBuilder(DiagnosticsEngine *DiagObj, SourceLocation DiagLoc,
+ unsigned DiagID);
protected:
/// Clear out the current diagnostic.
@@ -1326,7 +1249,7 @@ class DiagnosticBuilder : public StreamingDiagnostic {
if (!isActive()) return false;
// Process the diagnostic.
- bool Result = DiagObj->EmitCurrentDiagnostic(IsForceEmit);
+ bool Result = DiagObj->EmitDiagnostic(*this, IsForceEmit);
// This diagnostic is dead.
Clear();
@@ -1337,13 +1260,7 @@ class DiagnosticBuilder : public StreamingDiagnostic {
public:
/// Copy constructor. When copied, this "takes" the diagnostic info from the
/// input and neuters it.
- DiagnosticBuilder(const DiagnosticBuilder &D) : StreamingDiagnostic() {
- DiagObj = D.DiagObj;
- DiagStorage = D.DiagStorage;
- IsActive = D.IsActive;
- IsForceEmit = D.IsForceEmit;
- D.Clear();
- }
+ DiagnosticBuilder(const DiagnosticBuilder &D);
template <typename T> const DiagnosticBuilder &operator<<(const T &V) const {
assert(isActive() && "Clients must not add to cleared diagnostic!");
@@ -1375,7 +1292,7 @@ class DiagnosticBuilder : public StreamingDiagnostic {
return *this;
}
- void addFlagValue(StringRef V) const { DiagObj->FlagValue = std::string(V); }
+ void addFlagValue(StringRef V) const { FlagValue = std::string(V); }
};
struct AddFlagValue {
@@ -1550,12 +1467,7 @@ const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
inline DiagnosticBuilder DiagnosticsEngine::Report(SourceLocation Loc,
unsigned DiagID) {
- assert(CurDiagID == std::numeric_limits<unsigned>::max() &&
- "Multiple diagnostics in flight at once!");
- CurDiagLoc = Loc;
- CurDiagID = DiagID;
- FlagValue.clear();
- return DiagnosticBuilder(this);
+ return DiagnosticBuilder(this, Loc, DiagID);
}
const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
@@ -1570,24 +1482,29 @@ inline DiagnosticBuilder DiagnosticsEngine::Report(unsigned DiagID) {
//===----------------------------------------------------------------------===//
/// A little helper class (which is basically a smart pointer that forwards
-/// info from DiagnosticsEngine) that allows clients to enquire about the
-/// currently in-flight diagnostic.
+/// info from DiagnosticsEngine and DiagnosticStorage) that allows clients to
+/// enquire about the diagnostic.
class Diagnostic {
const DiagnosticsEngine *DiagObj;
+ SourceLocation DiagLoc;
+ unsigned DiagID;
+ std::string FlagValue;
+ const DiagnosticStorage &DiagStorage;
std::optional<StringRef> StoredDiagMessage;
public:
- explicit Diagnostic(const DiagnosticsEngine *DO) : DiagObj(DO) {}
- Diagnostic(const DiagnosticsEngine *DO, StringRef storedDiagMessage)
- : DiagObj(DO), StoredDiagMessage(storedDiagMessage) {}
+ Diagnostic(const DiagnosticsEngine *DO, const DiagnosticBuilder &DiagBuilder);
+ Diagnostic(const DiagnosticsEngine *DO, SourceLocation DiagLoc,
+ unsigned DiagID, const DiagnosticStorage &DiagStorage,
+ StringRef StoredDiagMessage);
const DiagnosticsEngine *getDiags() const { return DiagObj; }
- unsigned getID() const { return DiagObj->CurDiagID; }
- const SourceLocation &getLocation() const { return DiagObj->CurDiagLoc; }
+ unsigned getID() const { return DiagID; }
+ const SourceLocation &getLocation() const { return DiagLoc; }
bool hasSourceManager() const { return DiagObj->hasSourceManager(); }
SourceManager &getSourceManager() const { return DiagObj->getSourceManager();}
- unsigned getNumArgs() const { return DiagObj->DiagStorage.NumDiagArgs; }
+ unsigned getNumArgs() const { return DiagStorage.NumDiagArgs; }
/// Return the kind of the specified index.
///
@@ -1597,8 +1514,7 @@ class Diagnostic {
/// \pre Idx < getNumArgs()
DiagnosticsEngine::ArgumentKind getArgKind(unsigned Idx) const {
assert(Idx < getNumArgs() && "Argument index out of range!");
- return (DiagnosticsEngine::ArgumentKind)
- DiagObj->DiagStorage.DiagArgumentsKind[Idx];
+ return (DiagnosticsEngine::ArgumentKind)DiagStorage.DiagArgumentsKind[Idx];
}
/// Return the provided argument string specified by \p Idx.
@@ -1606,7 +1522,7 @@ class Diagnostic {
const std::string &getArgStdStr(unsigned Idx) const {
assert(getArgKind(Idx) == DiagnosticsEngine::ak_std_string &&
"invalid argument accessor!");
- return DiagObj->DiagStorage.DiagArgumentsStr[Idx];
+ return DiagStorage.DiagArgumentsStr[Idx];
}
/// Return the specified C string argument.
@@ -1614,8 +1530,7 @@ class Diagnostic {
const char *getArgCStr(unsigned Idx) const {
assert(getArgKind(Idx) == DiagnosticsEngine::ak_c_string &&
"invalid argument accessor!");
- return reinterpret_cast<const char *>(
- DiagObj->DiagStorage.DiagArgumentsVal[Idx]);
+ return reinterpret_cast<const char *>(DiagStorage.DiagArgumentsVal[Idx]);
}
/// Return the specified signed integer argument.
@@ -1623,7 +1538,7 @@ class Diagnostic {
int64_t getArgSInt(unsigned Idx) const {
assert(getArgKind(Idx) == DiagnosticsEngine::ak_sint &&
"invalid argument accessor!");
- return (int64_t)DiagObj->DiagStorage.DiagArgumentsVal[Idx];
+ return (int64_t)DiagStorage.DiagArgumentsVal[Idx];
}
/// Return the specified unsigned integer argument.
@@ -1631,7 +1546,7 @@ class Diagnostic {
uint64_t getArgUInt(unsigned Idx) const {
assert(getArgKind(Idx) == DiagnosticsEngine::ak_uint &&
"invalid argument accessor!");
- return DiagObj->DiagStorage.DiagArgumentsVal[Idx];
+ return DiagStorage.DiagArgumentsVal[Idx];
}
/// Return the specified IdentifierInfo argument.
@@ -1640,7 +1555,7 @@ class Diagnostic {
assert(getArgKind(Idx) == DiagnosticsEngine::ak_identifierinfo &&
"invalid argument accessor!");
return reinterpret_cast<IdentifierInfo *>(
- DiagObj->DiagStorage.DiagArgumentsVal[Idx]);
+ DiagStorage.DiagArgumentsVal[Idx]);
}
/// Return the specified non-string argument in an opaque form.
@@ -1648,37 +1563,32 @@ class Diagnostic {
uint64_t getRawArg(unsigned Idx) const {
assert(getArgKind(Idx) != DiagnosticsEngine::ak_std_string &&
"invalid argument accessor!");
- return DiagObj->DiagStorage.DiagArgumentsVal[Idx];
+ return DiagStorage.DiagArgumentsVal[Idx];
}
/// Return the number of source ranges associated with this diagnostic.
- unsigned getNumRanges() const {
- return DiagObj->DiagStorage.DiagRanges.size();
- }
+ unsigned getNumRanges() const { return DiagStorage.DiagRanges.size(); }
/// \pre Idx < getNumRanges()
const CharSourceRange &getRange(unsigned Idx) const {
assert(Idx < getNumRanges() && "Invalid diagnostic range index!");
- return DiagObj->DiagStorage.DiagRanges[Idx];
+ return DiagStorage.DiagRanges[Idx];
}
/// Return an array reference for this diagnostic's ranges.
- ArrayRef<CharSourceRange> getRanges() const {
- return DiagObj->DiagStorage.DiagRanges;
- }
+ ArrayRef<CharSourceRange> getRanges() const { return DiagStorage.DiagRanges; }
- unsigned getNumFixItHints() const {
- return DiagObj->DiagStorage.FixItHints.size();
- }
+ unsigned getNumFixItHints() const { return DiagStorage.FixItHints.size(); }
const FixItHint &getFixItHint(unsigned Idx) const {
assert(Idx < getNumFixItHints() && "Invalid index!");
- return DiagObj->DiagStorage.FixItHints[Idx];
+ return DiagStorage.FixItHints[Idx];
}
- ArrayRef<FixItHint> getFixItHints() const {
- return DiagObj->DiagStorage.FixItHints;
- }
+ ArrayRef<FixItHint> getFixItHints() const { return DiagStorage.FixItHints; }
+
+ /// Return the value associated with this diagnostic flag.
+ StringRef getFlagValue() const { return FlagValue; }
/// 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 daad66f499538f..1fa38ed6066e26 100644
--- a/clang/include/clang/Basic/DiagnosticIDs.h
+++ b/clang/include/clang/Basic/DiagnosticIDs.h
@@ -24,6 +24,7 @@
namespace clang {
class DiagnosticsEngine;
+ class DiagnosticBuilder;
class SourceLocation;
// Import the diagnostic enums themselves.
@@ -486,11 +487,13 @@ class DiagnosticIDs : public RefCountedBase<DiagnosticIDs> {
///
/// \returns \c true if the diagnostic was emitted, \c false if it was
/// suppressed.
- bool ProcessDiag(DiagnosticsEngine &Diag) const;
+ bool ProcessDiag(DiagnosticsEngine &Diag,
+ const DiagnosticBuilder &DiagBuilder) const;
/// Used to emit a diagnostic that is finally fully formed,
/// ignoring suppression.
- void EmitDiag(DiagnosticsEngine &Diag, Level DiagLevel) const;
+ 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.
diff --git a/clang/include/clang/Basic/PartialDiagnostic.h b/clang/include/clang/Basic/PartialDiagnostic.h
index 507d789c54ff9b..4bf6049d08fdb4 100644
--- a/clang/include/clang/Basic/PartialDiagnostic.h
+++ b/clang/include/clang/Basic/PartialDiagnostic.h
@@ -166,13 +166,10 @@ 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).FormatDiagnostic(Buf);
+ Diagnostic(&Diags, DB).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 0809ac1b144ef6..e1c3a99cfa167e 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 active diagnostic on the DiagosticsEngine to be
- /// emitted. This is closely coupled to the SemaDiagnosticBuilder class and
+ /// Cause the built diagnostic to be emitted on the DiagosticsEngine.
+ /// This is closely coupled to the SemaDiagnosticBuilder class and
/// should not be used elsewhere.
- void EmitCurrentDiagnostic(unsigned DiagID);
+ void EmitDiagnostic(unsigned DiagID, const DiagnosticBuilder &DB);
void addImplicitTypedef(StringRef Name, QualType T);
diff --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp
index ecff80a5090630..0bd6845085b735 100644
--- a/clang/lib/Basic/Diagnostic.cpp
+++ b/clang/lib/Basic/Diagnostic.cpp
@@ -126,9 +126,7 @@ 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.
@@ -143,23 +141,6 @@ 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 =
@@ -503,39 +484,31 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor Flavor,
}
void DiagnosticsEngine::Report(const StoredDiagnostic &storedDiag) {
- 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();
+ DiagnosticStorage DiagStorage;
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.getMessage());
+ Diagnostic Info(this, storedDiag.getLocation(), storedDiag.getID(),
+ DiagStorage, storedDiag.getMessage());
Client->HandleDiagnostic(DiagLevel, Info);
if (Client->IncludeInDiagnosticCounts()) {
if (DiagLevel == DiagnosticsEngine::Warning)
++NumWarnings;
}
-
- CurDiagID = std::numeric_limits<unsigned>::max();
}
-bool DiagnosticsEngine::EmitCurrentDiagnostic(bool Force) {
+bool DiagnosticsEngine::EmitDiagnostic(const DiagnosticBuilder &DB,
+ bool Force) {
assert(getClient() && "DiagnosticClient not set!");
bool Emitted;
if (Force) {
- Diagnostic Info(this);
+ Diagnostic Info(this, DB);
// Figure out the diagnostic level of this message.
DiagnosticIDs::Level DiagLevel
@@ -544,24 +517,50 @@ bool DiagnosticsEngine::EmitCurrentDiagnostic(bool Force) {
Emitted = (DiagLevel != DiagnosticIDs::Ignored);
if (Emitted) {
// Emit the diagnostic regardless of suppression level.
- Diags->EmitDiag(*this, DiagLevel);
+ Diags->EmitDiag(*this, DB, DiagLevel);
}
} else {
// Process the diagnostic, sending the accumulated information to the
// DiagnosticConsumer.
- Emitted = ProcessDiag();
+ Emitted = ProcessDiag(DB);
}
- // Clear out the current diagnostic object.
- Clear();
+ return Emitted;
+}
+
+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,
@@ -1216,13 +1215,13 @@ bool ForwardingDiagnosticConsumer::IncludeInDiagnosticCounts() const {
return Target.IncludeInDiagnosticCounts();
}
-PartialDiagnostic::DiagStorageAllocator::DiagStorageAllocator() {
+DiagStorageAllocator::DiagStorageAllocator() {
for (unsigned I = 0; I != NumCached; ++I)
FreeList[I] = Cached + I;
NumFreeListEntries = NumCached;
}
-PartialDiagnostic::DiagStorageAllocator::~DiagStorageAllocator() {
+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 cae6642bd4bd3e..031d9d7817d1f6 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 &&
- Diag.CurDiagID != diag::fatal_too_many_errors && Diag.FatalsAsError)
+ DiagID != diag::fatal_too_many_errors && Diag.FatalsAsError)
Result = diag::Severity::Error;
bool ShowInSystemHeader =
@@ -800,8 +800,9 @@ 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 {
- Diagnostic Info(&Diag);
+bool DiagnosticIDs::ProcessDiag(DiagnosticsEngine &Diag,
+ const DiagnosticBuilder &DiagBuilder) const {
+ Diagnostic Info(&Diag, DiagBuilder);
assert(Diag.getClient() && "DiagnosticClient not set!");
@@ -867,22 +868,24 @@ bool DiagnosticIDs::ProcessDiag(DiagnosticsEngine &Diag) const {
// stop a flood of bogus errors.
if (Diag.ErrorLimit && Diag.NumErrors > Diag.ErrorLimit &&
DiagLevel == DiagnosticIDs::Error) {
- Diag.SetDelayedDiagnostic(diag::fatal_too_many_errors);
+ 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 (Diag.CurDiagID == diag::fatal_too_many_errors)
+ if (Info.getID() == diag::fatal_too_many_errors)
Diag.FatalErrorOccurred = true;
// Finally, report it.
- EmitDiag(Diag, DiagLevel);
+ EmitDiag(Diag, DiagBuilder, DiagLevel);
return true;
}
-void DiagnosticIDs::EmitDiag(DiagnosticsEngine &Diag, Level DiagLevel) const {
- Diagnostic Info(&Diag);
+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);
@@ -890,8 +893,6 @@ void DiagnosticIDs::EmitDiag(DiagnosticsEngine &Diag, Level DiagLevel) const {
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 d6ec26af80aadd..65a8a7253e054f 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -130,13 +130,8 @@ 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) {
- 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();
+ Diag.Report(Loc, diag::err_cannot_open_file)
+ << ContentsEntry->getName() << BufferOrError.getError().message();
return std::nullopt;
}
@@ -153,12 +148,7 @@ 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()) {
- if (Diag.isDiagnosticInFlight())
- Diag.SetDelayedDiagnostic(diag::err_file_too_large,
- ContentsEntry->getName());
- else
- Diag.Report(Loc, diag::err_file_too_large)
- << ContentsEntry->getName();
+ Diag.Report(Loc, diag::err_file_too_large) << ContentsEntry->getName();
return std::nullopt;
}
@@ -168,12 +158,7 @@ ContentCache::getBufferOrNone(DiagnosticsEngine &Diag, FileManager &FM,
// have come from a stat cache).
if (!ContentsEntry->isNamedPipe() &&
Buffer->getBufferSize() != (size_t)ContentsEntry->getSize()) {
- if (Diag.isDiagnosticInFlight())
- Diag.SetDelayedDiagnostic(diag::err_file_modified,
- ContentsEntry->getName());
- else
- Diag.Report(Loc, diag::err_file_modified)
- << ContentsEntry->getName();
+ 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 44dfaf20eae73f..7309553e3bc0b4 100644
--- a/clang/lib/Frontend/Rewrite/FixItRewriter.cpp
+++ b/clang/lib/Frontend/Rewrite/FixItRewriter.cpp
@@ -200,10 +200,8 @@ 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,
- // clear out any current diagnostic, and let the downstream client
- // format the 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 c2fea3d03f0c0f..28f7218dc23f54 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.getDiags()->getFlagValue();
+ StringRef OptValue = Info.getFlagValue();
if (!OptValue.empty())
OS << "=" << OptValue;
Started = true;
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 85cbbe7750c2b3..69d72412471809 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -1590,7 +1590,7 @@ LangAS Sema::getDefaultCXXMethodAddrSpace() const {
return LangAS::Default;
}
-void Sema::EmitCurrentDiagnostic(unsigned DiagID) {
+void Sema::EmitDiagnostic(unsigned DiagID, const DiagnosticBuilder &DB) {
// 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
@@ -1598,9 +1598,9 @@ void Sema::EmitCurrentDiagnostic(unsigned DiagID) {
// 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(
- Diags.getCurrentDiagID())) {
+ switch (DiagnosticIDs::getDiagnosticSFINAEResponse(DiagInfo.getID())) {
case DiagnosticIDs::SFINAE_Report:
// We'll report the diagnostic below.
break;
@@ -1613,13 +1613,11 @@ void Sema::EmitCurrentDiagnostic(unsigned DiagID) {
// 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: {
@@ -1630,7 +1628,7 @@ void Sema::EmitCurrentDiagnostic(unsigned DiagID) {
if (!AccessCheckingSFINAE && !getLangOpts().CPlusPlus11)
break;
- SourceLocation Loc = Diags.getCurrentDiagLoc();
+ SourceLocation Loc = DiagInfo.getLocation();
// Suppress this diagnostic.
++NumSFINAEErrors;
@@ -1638,16 +1636,13 @@ void Sema::EmitCurrentDiagnostic(unsigned DiagID) {
// 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 the diagnostic state is clear, produce a C++98 compatibility
- // warning.
+ // Now 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
@@ -1660,14 +1655,12 @@ void Sema::EmitCurrentDiagnostic(unsigned DiagID) {
// 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;
}
}
@@ -1677,7 +1670,7 @@ void Sema::EmitCurrentDiagnostic(unsigned DiagID) {
Context.setPrintingPolicy(getPrintingPolicy());
// Emit the diagnostic.
- if (!Diags.EmitCurrentDiagnostic())
+ if (!Diags.EmitDiagnostic(DB))
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 a2f12d622e8ccc..5c24f21b469b01 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.EmitCurrentDiagnostic(DiagID);
+ SemaRef.EmitDiagnostic(DiagID, *this);
}
PartialDiagnostic SemaBase::PDiag(unsigned DiagID) {
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 4fae6ff02ea9a3..7efcc81e194d95 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 && !Diags.isDiagnosticInFlight() &&
+ if (PP.getLangOpts().Modules &&
!PP.getHeaderSearchInfo().getModuleCachePath().empty()) {
Diag(diag::note_module_cache_path)
<< PP.getHeaderSearchInfo().getModuleCachePath();
@@ -1391,10 +1391,7 @@ void ASTReader::Error(StringRef Msg) const {
void ASTReader::Error(unsigned DiagID, StringRef Arg1, StringRef Arg2,
StringRef Arg3) const {
- if (Diags.isDiagnosticInFlight())
- Diags.SetDelayedDiagnostic(DiagID, Arg1, Arg2, Arg3);
- else
- Diag(DiagID) << Arg1 << Arg2 << Arg3;
+ Diag(DiagID) << Arg1 << Arg2 << Arg3;
}
void ASTReader::Error(llvm::Error &&Err) const {
@@ -2713,7 +2710,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 && !Diags.isDiagnosticInFlight()) {
+ if (Complain) {
// Build a list of the PCH imports that got us here (in reverse).
SmallVector<ModuleFile *, 4> ImportStack(1, &F);
while (!ImportStack.back()->ImportedBy.empty())
@@ -3689,10 +3686,8 @@ llvm::Error ASTReader::ReadASTBlock(ModuleFile &F,
SourceMgr.AllocateLoadedSLocEntries(F.LocalNumSLocEntries,
SLocSpaceSize);
if (!F.SLocEntryBaseID) {
- if (!Diags.isDiagnosticInFlight()) {
- Diags.Report(SourceLocation(), diag::remark_sloc_usage);
- SourceMgr.noteSLocAddressSpaceUsage(Diags);
- }
+ 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
new file mode 100644
index 00000000000000..752b0cc3ff6286
--- /dev/null
+++ b/clang/test/PCH/race-condition.cpp
@@ -0,0 +1,41 @@
+// 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
diff erent definitions in
diff erent modules; defined here first
diff erence is 1st parameter with type 'F'}}
+// expected-error at 24{{'N::midpoint' has
diff erent definitions in
diff erent modules; defined here first
diff erence 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 74690193917165..691d74f697f278 100644
--- a/clang/unittests/Basic/DiagnosticTest.cpp
+++ b/clang/unittests/Basic/DiagnosticTest.cpp
@@ -17,9 +17,6 @@ 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());
@@ -83,6 +80,21 @@ 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,
@@ -104,7 +116,6 @@ TEST(DiagnosticTest, softReset) {
// Check for private variables of DiagnosticsEngine
diff erentiating 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 41ab30bc81d5f9..2a079a62f1bc13 100644
--- a/clang/unittests/Driver/DXCModeTest.cpp
+++ b/clang/unittests/Driver/DXCModeTest.cpp
@@ -51,7 +51,6 @@ static void validateTargetProfile(
EXPECT_TRUE(C);
EXPECT_EQ(Diags.getNumErrors(), NumOfErrors);
EXPECT_STREQ(DiagConsumer->Errors.back().c_str(), ExpectError.data());
- Diags.Clear();
DiagConsumer->clear();
}
@@ -160,7 +159,6 @@ 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,
@@ -176,7 +174,6 @@ 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,
@@ -193,7 +190,6 @@ 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,
@@ -210,7 +206,6 @@ 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();
}
diff --git a/flang/lib/Frontend/TextDiagnosticPrinter.cpp b/flang/lib/Frontend/TextDiagnosticPrinter.cpp
index 8b00fb69b3cefb..dc182d68a1a979 100644
--- a/flang/lib/Frontend/TextDiagnosticPrinter.cpp
+++ b/flang/lib/Frontend/TextDiagnosticPrinter.cpp
@@ -45,7 +45,7 @@ static void printRemarkOption(llvm::raw_ostream &os,
// warning could be printed i.e. [-Wunknown-warning-option]
os << " [" << (level == clang::DiagnosticsEngine::Remark ? "-R" : "-W")
<< opt;
- llvm::StringRef optValue = info.getDiags()->getFlagValue();
+ llvm::StringRef optValue = info.getFlagValue();
if (!optValue.empty())
os << "=" << optValue;
os << ']';
More information about the flang-commits
mailing list