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@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@27{{'N::midpoint' has diff erent definitions in diff erent modules; defined here first diff erence is 1st parameter with type 'F'}} +// expected-error@24{{'N::midpoint' has diff erent definitions in diff erent modules; defined here first diff erence is 1st parameter with type 'U'}} +// expected-note@21{{but in '' found 1st parameter with type 'T'}} +int x = N::something; +// expected-error@37{{no member named 'something' in namespace 'N'}} +// expected-note@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 << ']'; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
