On 4 May 2017 at 12:52, Alex L <arpha...@gmail.com> wrote: > > > On 4 May 2017 at 12:43, Alex L <arpha...@gmail.com> wrote: > >> >> >> On 3 May 2017 at 22:23, Richard Smith <rich...@metafoo.co.uk> wrote: >> >>> On 3 May 2017 at 07:29, Alex L <arpha...@gmail.com> wrote: >>> >>>> Hi Richard, >>>> >>>> This commit has caused an infinite loop in one of our internal libclang >>>> based tooling tests. It keeps repeating the following frames: >>>> >>>> frame #33528: 0x0000000109db2edf libclang.dylib`clang::Diagnost >>>> icsEngine::ReportDelayed(this=0x000000011c002c00) at Diagnostic.cpp:149 >>>> frame #33529: 0x0000000109db5a36 libclang.dylib`clang::Diagnost >>>> icsEngine::EmitCurrentDiagnostic(this=0x000000011c002c00, Force=false) >>>> at Diagnostic.cpp:428 >>>> frame #33530: 0x000000010a33f93c libclang.dylib`clang::Diagnost >>>> icBuilder::Emit(this=0x0000700008d4bfd8) at Diagnostic.h:1013 >>>> frame #33531: 0x000000010a33f8e5 libclang.dylib`clang::Diagnost >>>> icBuilder::~DiagnosticBuilder(this=0x0000700008d4bfd8) at >>>> Diagnostic.h:1036 >>>> frame #33532: 0x000000010a335015 libclang.dylib`clang::Diagnost >>>> icBuilder::~DiagnosticBuilder(this=0x0000700008d4bfd8) at >>>> Diagnostic.h:1035 >>>> >>>> It doesn't really look like a regression though, it seems that this has >>>> just uncovered a bug in Clang: DiagnosticsEngine::ReportDelayed is >>>> clearing DelayedDiagID *after* reporting the issue which causes the >>>> infinite recursion, when it should clear it before. Is that right? >>>> >>> >>> EmitCurrentDiagnostic checks "DelayedDiagID != DiagID" before making the >>> recursive call, which should be preventing the infinite recursion. >>> >>> It looks like the immediate bug is that EmitCurrentDiagnostic grabs >>> CurDiagID *after* calling EmitDiag / ProcessDiag, which clear CurDiagID >>> when they succeed in emitting the diagnostic. But I agree, the right thing >>> to do is clear DelayedDiagID before emitting the delayed diagnostic, not >>> after. You should also be able to delete the incorrect recursion check in >>> EmitCurrentDiagnostic too. >>> >> >> Thanks, that makes sense. >> >> Unfortunately I had to revert my fix as it uncovered two issues in >> Misc/error-limit-multiple-notes.cpp and Misc/error-limit.c . It looks >> like your commit caused the infinite recursion in both of them, but the >> bots didn't catch that because they invoked not clang, and emitted the >> diagnostics that were checked (using FileCheck) before crashing, so they >> "passed". However, my fix causes the tests to fail, seemingly because now >> Clang doesn't suppress multiple notes after reaching the error limit (it >> suppresses just the first note). It looks like this is a regression >> introduced in this commit. >> >> Sorry, I was wrong in my analysis. It's not a regression introduced by > your commit, it's just a problem with my fix. >
I figured out what was wrong, turns out there was another subtle bug - Clang was actually reporting `fatal_too_many_errors` twice (recursively), and the second invocation set `FatalErrorOccurred`, and thus prevented the notes from the diagnostic that caused `fatal_too_many_errors` to get emitted. But my fix fixed that subtle bug as well, so I had to explicitly set `FatalErrorOccurred` when reporting `fatal_too_many_errors`. It should be fixed now in r302151. > > >> >> >>> >>> >>>> I will commit a fix for this now. >>>> >>> >>> Thank you! >>> >>> >>>> Alex >>>> >>>> >>>> On 3 May 2017 at 01:28, Richard Smith via cfe-commits < >>>> cfe-commits@lists.llvm.org> wrote: >>>> >>>>> Author: rsmith >>>>> Date: Tue May 2 19:28:49 2017 >>>>> New Revision: 301992 >>>>> >>>>> URL: http://llvm.org/viewvc/llvm-project?rev=301992&view=rev >>>>> Log: >>>>> [modules] Round-trip -Werror flag through explicit module build. >>>>> >>>>> The intent for an explicit module build is that the diagnostics >>>>> produced within >>>>> the module are those that were configured when the module was built, >>>>> not those >>>>> that are enabled within a user of the module. This includes >>>>> diagnostics that >>>>> don't actually show up until the module is used (for instance, >>>>> diagnostics >>>>> produced during template instantiation and weird cases like -Wpadded). >>>>> >>>>> We serialized and restored the diagnostic state for individual warning >>>>> groups, >>>>> but previously did not track the state for flags like -Werror and >>>>> -Weverything, >>>>> which are implemented as separate bits rather than as part of the >>>>> diagnostics >>>>> mapping information. >>>>> >>>>> Modified: >>>>> cfe/trunk/include/clang/Basic/Diagnostic.h >>>>> cfe/trunk/include/clang/Basic/DiagnosticIDs.h >>>>> cfe/trunk/lib/Basic/Diagnostic.cpp >>>>> cfe/trunk/lib/Basic/DiagnosticIDs.cpp >>>>> cfe/trunk/lib/Serialization/ASTReader.cpp >>>>> cfe/trunk/lib/Serialization/ASTWriter.cpp >>>>> cfe/trunk/test/Index/keep-going.cpp >>>>> cfe/trunk/test/Modules/diag-flags.cpp >>>>> cfe/trunk/tools/libclang/CIndex.cpp >>>>> cfe/trunk/unittests/Basic/DiagnosticTest.cpp >>>>> >>>>> Modified: cfe/trunk/include/clang/Basic/Diagnostic.h >>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ >>>>> Basic/Diagnostic.h?rev=301992&r1=301991&r2=301992&view=diff >>>>> ============================================================ >>>>> ================== >>>>> --- cfe/trunk/include/clang/Basic/Diagnostic.h (original) >>>>> +++ cfe/trunk/include/clang/Basic/Diagnostic.h Tue May 2 19:28:49 >>>>> 2017 >>>>> @@ -178,12 +178,7 @@ public: >>>>> >>>>> private: >>>>> unsigned char AllExtensionsSilenced; // Used by __extension__ >>>>> - bool IgnoreAllWarnings; // Ignore all warnings: -w >>>>> - bool WarningsAsErrors; // Treat warnings like errors. >>>>> - bool EnableAllWarnings; // Enable all warnings. >>>>> - bool ErrorsAsFatal; // Treat errors like fatal errors. >>>>> - bool FatalsAsError; // Treat fatal errors like errors. >>>>> - bool SuppressSystemWarnings; // Suppress warnings in system >>>>> headers. >>>>> + bool SuppressAfterFatalError; // Suppress diagnostics after a >>>>> fatal error? >>>>> bool SuppressAllDiagnostics; // Suppress all diagnostics. >>>>> bool ElideType; // Elide common types of templates. >>>>> bool PrintTemplateTree; // Print a tree when comparing >>>>> templates. >>>>> @@ -194,7 +189,6 @@ private: >>>>> // 0 -> no limit. >>>>> unsigned ConstexprBacktraceLimit; // Cap on depth of constexpr >>>>> evaluation >>>>> // backtrace stack, 0 -> no limit. >>>>> - diag::Severity ExtBehavior; // Map extensions to warnings or >>>>> errors? >>>>> IntrusiveRefCntPtr<DiagnosticIDs> Diags; >>>>> IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts; >>>>> DiagnosticConsumer *Client; >>>>> @@ -216,6 +210,19 @@ private: >>>>> llvm::DenseMap<unsigned, DiagnosticMapping> DiagMap; >>>>> >>>>> public: >>>>> + // "Global" configuration state that can actually vary between >>>>> modules. >>>>> + unsigned IgnoreAllWarnings : 1; // Ignore all warnings: -w >>>>> + unsigned EnableAllWarnings : 1; // Enable all warnings. >>>>> + unsigned WarningsAsErrors : 1; // Treat warnings like >>>>> errors. >>>>> + unsigned ErrorsAsFatal : 1; // Treat errors like fatal >>>>> errors. >>>>> + unsigned SuppressSystemWarnings : 1; // Suppress warnings in >>>>> system headers. >>>>> + diag::Severity ExtBehavior : 4; // Map extensions to warnings >>>>> or errors? >>>>> + >>>>> + DiagState() >>>>> + : IgnoreAllWarnings(false), EnableAllWarnings(false), >>>>> + WarningsAsErrors(false), ErrorsAsFatal(false), >>>>> + SuppressSystemWarnings(false), >>>>> ExtBehavior(diag::Severity::Ignored) {} >>>>> + >>>>> typedef llvm::DenseMap<unsigned, DiagnosticMapping>::iterator >>>>> iterator; >>>>> typedef llvm::DenseMap<unsigned, DiagnosticMapping>::const_iter >>>>> ator >>>>> const_iterator; >>>>> @@ -493,33 +500,47 @@ public: >>>>> /// \brief When set to true, any unmapped warnings are ignored. >>>>> /// >>>>> /// If this and WarningsAsErrors are both set, then this one wins. >>>>> - void setIgnoreAllWarnings(bool Val) { IgnoreAllWarnings = Val; } >>>>> - bool getIgnoreAllWarnings() const { return IgnoreAllWarnings; } >>>>> + void setIgnoreAllWarnings(bool Val) { >>>>> + GetCurDiagState()->IgnoreAllWarnings = Val; >>>>> + } >>>>> + bool getIgnoreAllWarnings() const { >>>>> + return GetCurDiagState()->IgnoreAllWarnings; >>>>> + } >>>>> >>>>> /// \brief When set to true, any unmapped ignored warnings are no >>>>> longer >>>>> /// ignored. >>>>> /// >>>>> /// If this and IgnoreAllWarnings are both set, then that one wins. >>>>> - void setEnableAllWarnings(bool Val) { EnableAllWarnings = Val; } >>>>> - bool getEnableAllWarnings() const { return EnableAllWarnings; } >>>>> + void setEnableAllWarnings(bool Val) { >>>>> + GetCurDiagState()->EnableAllWarnings = Val; >>>>> + } >>>>> + bool getEnableAllWarnings() const { >>>>> + return GetCurDiagState()->EnableAllWarnings; >>>>> + } >>>>> >>>>> /// \brief When set to true, any warnings reported are issued as >>>>> errors. >>>>> - void setWarningsAsErrors(bool Val) { WarningsAsErrors = Val; } >>>>> - bool getWarningsAsErrors() const { return WarningsAsErrors; } >>>>> + void setWarningsAsErrors(bool Val) { >>>>> + GetCurDiagState()->WarningsAsErrors = Val; >>>>> + } >>>>> + bool getWarningsAsErrors() const { >>>>> + return GetCurDiagState()->WarningsAsErrors; >>>>> + } >>>>> >>>>> /// \brief When set to true, any error reported is made a fatal >>>>> error. >>>>> - void setErrorsAsFatal(bool Val) { ErrorsAsFatal = Val; } >>>>> - bool getErrorsAsFatal() const { return ErrorsAsFatal; } >>>>> + void setErrorsAsFatal(bool Val) { GetCurDiagState()->ErrorsAsFatal >>>>> = Val; } >>>>> + bool getErrorsAsFatal() const { return >>>>> GetCurDiagState()->ErrorsAsFatal; } >>>>> >>>>> - /// \brief When set to true, any fatal error reported is made an >>>>> error. >>>>> - /// >>>>> - /// This setting takes precedence over the setErrorsAsFatal setting >>>>> above. >>>>> - void setFatalsAsError(bool Val) { FatalsAsError = Val; } >>>>> - bool getFatalsAsError() const { return FatalsAsError; } >>>>> + /// \brief When set to true (the default), suppress further >>>>> diagnostics after >>>>> + /// a fatal error. >>>>> + void setSuppressAfterFatalError(bool Val) { >>>>> SuppressAfterFatalError = Val; } >>>>> >>>>> /// \brief When set to true mask warnings that come from system >>>>> headers. >>>>> - void setSuppressSystemWarnings(bool Val) { SuppressSystemWarnings = >>>>> Val; } >>>>> - bool getSuppressSystemWarnings() const { return >>>>> SuppressSystemWarnings; } >>>>> + void setSuppressSystemWarnings(bool Val) { >>>>> + GetCurDiagState()->SuppressSystemWarnings = Val; >>>>> + } >>>>> + bool getSuppressSystemWarnings() const { >>>>> + return GetCurDiagState()->SuppressSystemWarnings; >>>>> + } >>>>> >>>>> /// \brief Suppress all diagnostics, to silence the front end when >>>>> we >>>>> /// know that we don't want any more diagnostics to be passed along >>>>> to the >>>>> @@ -571,11 +592,15 @@ public: >>>>> } >>>>> >>>>> /// \brief Controls whether otherwise-unmapped extension >>>>> diagnostics are >>>>> - /// mapped onto ignore/warning/error. >>>>> + /// mapped onto ignore/warning/error. >>>>> /// >>>>> /// This corresponds to the GCC -pedantic and -pedantic-errors >>>>> option. >>>>> - void setExtensionHandlingBehavior(diag::Severity H) { ExtBehavior >>>>> = H; } >>>>> - diag::Severity getExtensionHandlingBehavior() const { return >>>>> ExtBehavior; } >>>>> + void setExtensionHandlingBehavior(diag::Severity H) { >>>>> + GetCurDiagState()->ExtBehavior = H; >>>>> + } >>>>> + diag::Severity getExtensionHandlingBehavior() const { >>>>> + return GetCurDiagState()->ExtBehavior; >>>>> + } >>>>> >>>>> /// \brief Counter bumped when an __extension__ block is/ >>>>> encountered. >>>>> /// >>>>> >>>>> Modified: cfe/trunk/include/clang/Basic/DiagnosticIDs.h >>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ >>>>> Basic/DiagnosticIDs.h?rev=301992&r1=301991&r2=301992&view=diff >>>>> ============================================================ >>>>> ================== >>>>> --- cfe/trunk/include/clang/Basic/DiagnosticIDs.h (original) >>>>> +++ cfe/trunk/include/clang/Basic/DiagnosticIDs.h Tue May 2 19:28:49 >>>>> 2017 >>>>> @@ -122,15 +122,21 @@ public: >>>>> bool wasUpgradedFromWarning() const { return >>>>> WasUpgradedFromWarning; } >>>>> void setUpgradedFromWarning(bool Value) { WasUpgradedFromWarning = >>>>> Value; } >>>>> >>>>> - /// Serialize the bits that aren't based on context. >>>>> - unsigned serializeBits() const { >>>>> - return (WasUpgradedFromWarning << 3) | Severity; >>>>> + /// Serialize this mapping as a raw integer. >>>>> + unsigned serialize() const { >>>>> + return (IsUser << 7) | (IsPragma << 6) | (HasNoWarningAsError << >>>>> 5) | >>>>> + (HasNoErrorAsFatal << 4) | (WasUpgradedFromWarning << 3) | >>>>> Severity; >>>>> } >>>>> - static diag::Severity deserializeSeverity(unsigned Bits) { >>>>> - return (diag::Severity)(Bits & 0x7); >>>>> - } >>>>> - static bool deserializeUpgradedFromWarning(unsigned Bits) { >>>>> - return Bits >> 3; >>>>> + /// Deserialize a mapping. >>>>> + static DiagnosticMapping deserialize(unsigned Bits) { >>>>> + DiagnosticMapping Result; >>>>> + Result.IsUser = (Bits >> 7) & 1; >>>>> + Result.IsPragma = (Bits >> 6) & 1; >>>>> + Result.HasNoWarningAsError = (Bits >> 5) & 1; >>>>> + Result.HasNoErrorAsFatal = (Bits >> 4) & 1; >>>>> + Result.WasUpgradedFromWarning = (Bits >> 3) & 1; >>>>> + Result.Severity = Bits & 0x7; >>>>> + return Result; >>>>> } >>>>> }; >>>>> >>>>> >>>>> Modified: cfe/trunk/lib/Basic/Diagnostic.cpp >>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Diag >>>>> nostic.cpp?rev=301992&r1=301991&r2=301992&view=diff >>>>> ============================================================ >>>>> ================== >>>>> --- cfe/trunk/lib/Basic/Diagnostic.cpp (original) >>>>> +++ cfe/trunk/lib/Basic/Diagnostic.cpp Tue May 2 19:28:49 2017 >>>>> @@ -67,18 +67,12 @@ DiagnosticsEngine::DiagnosticsEngine(Int >>>>> ArgToStringCookie = nullptr; >>>>> >>>>> AllExtensionsSilenced = 0; >>>>> - IgnoreAllWarnings = false; >>>>> - WarningsAsErrors = false; >>>>> - EnableAllWarnings = false; >>>>> - ErrorsAsFatal = false; >>>>> - FatalsAsError = false; >>>>> - SuppressSystemWarnings = false; >>>>> + SuppressAfterFatalError = true; >>>>> SuppressAllDiagnostics = false; >>>>> ElideType = true; >>>>> PrintTemplateTree = false; >>>>> ShowColors = false; >>>>> ShowOverloads = Ovl_All; >>>>> - ExtBehavior = diag::Severity::Ignored; >>>>> >>>>> ErrorLimit = 0; >>>>> TemplateBacktraceLimit = 0; >>>>> @@ -343,8 +337,8 @@ bool DiagnosticsEngine::setDiagnosticGro >>>>> return setSeverityForGroup(diag::Flavor::WarningOrError, Group, >>>>> diag::Severity::Fatal); >>>>> >>>>> - // Otherwise, we want to set the diagnostic mapping's "no Werror" >>>>> bit, and >>>>> - // potentially downgrade anything already mapped to be an error. >>>>> + // Otherwise, we want to set the diagnostic mapping's "no >>>>> Wfatal-errors" bit, >>>>> + // and potentially downgrade anything already mapped to be a fatal >>>>> error. >>>>> >>>>> // Get the diagnostics in this group. >>>>> SmallVector<diag::kind, 8> GroupDiags; >>>>> >>>>> Modified: cfe/trunk/lib/Basic/DiagnosticIDs.cpp >>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Diag >>>>> nosticIDs.cpp?rev=301992&r1=301991&r2=301992&view=diff >>>>> ============================================================ >>>>> ================== >>>>> --- cfe/trunk/lib/Basic/DiagnosticIDs.cpp (original) >>>>> +++ cfe/trunk/lib/Basic/DiagnosticIDs.cpp Tue May 2 19:28:49 2017 >>>>> @@ -420,7 +420,7 @@ DiagnosticIDs::getDiagnosticSeverity(uns >>>>> Result = Mapping.getSeverity(); >>>>> >>>>> // Upgrade ignored diagnostics if -Weverything is enabled. >>>>> - if (Diag.EnableAllWarnings && Result == diag::Severity::Ignored && >>>>> + if (State->EnableAllWarnings && Result == diag::Severity::Ignored && >>>>> !Mapping.isUser() && getBuiltinDiagClass(DiagID) != >>>>> CLASS_REMARK) >>>>> Result = diag::Severity::Warning; >>>>> >>>>> @@ -435,7 +435,7 @@ DiagnosticIDs::getDiagnosticSeverity(uns >>>>> // For extension diagnostics that haven't been explicitly mapped, >>>>> check if we >>>>> // should upgrade the diagnostic. >>>>> if (IsExtensionDiag && !Mapping.isUser()) >>>>> - Result = std::max(Result, Diag.ExtBehavior); >>>>> + Result = std::max(Result, State->ExtBehavior); >>>>> >>>>> // At this point, ignored errors can no longer be upgraded. >>>>> if (Result == diag::Severity::Ignored) >>>>> @@ -443,28 +443,24 @@ DiagnosticIDs::getDiagnosticSeverity(uns >>>>> >>>>> // Honor -w, which is lower in priority than pedantic-errors, but >>>>> higher than >>>>> // -Werror. >>>>> - if (Result == diag::Severity::Warning && Diag.IgnoreAllWarnings) >>>>> + // FIXME: Under GCC, this also suppresses warnings that have been >>>>> mapped to >>>>> + // errors by -W flags and #pragma diagnostic. >>>>> + if (Result == diag::Severity::Warning && State->IgnoreAllWarnings) >>>>> return diag::Severity::Ignored; >>>>> >>>>> // If -Werror is enabled, map warnings to errors unless explicitly >>>>> disabled. >>>>> if (Result == diag::Severity::Warning) { >>>>> - if (Diag.WarningsAsErrors && !Mapping.hasNoWarningAsError()) >>>>> + if (State->WarningsAsErrors && !Mapping.hasNoWarningAsError()) >>>>> Result = diag::Severity::Error; >>>>> } >>>>> >>>>> // If -Wfatal-errors is enabled, map errors to fatal unless >>>>> explicity >>>>> // disabled. >>>>> if (Result == diag::Severity::Error) { >>>>> - if (Diag.ErrorsAsFatal && !Mapping.hasNoErrorAsFatal()) >>>>> + if (State->ErrorsAsFatal && !Mapping.hasNoErrorAsFatal()) >>>>> Result = diag::Severity::Fatal; >>>>> } >>>>> >>>>> - // If explicitly requested, map fatal errors to errors. >>>>> - if (Result == diag::Severity::Fatal) { >>>>> - if (Diag.FatalsAsError) >>>>> - Result = diag::Severity::Error; >>>>> - } >>>>> - >>>>> // Custom diagnostics always are emitted in system headers. >>>>> bool ShowInSystemHeader = >>>>> !GetDiagInfo(DiagID) || GetDiagInfo(DiagID)->WarnShowI >>>>> nSystemHeader; >>>>> @@ -472,7 +468,7 @@ DiagnosticIDs::getDiagnosticSeverity(uns >>>>> // If we are in a system header, we ignore it. We look at the >>>>> diagnostic class >>>>> // because we also want to ignore extensions and warnings in >>>>> -Werror and >>>>> // -pedantic-errors modes, which *map* warnings/extensions to >>>>> errors. >>>>> - if (Diag.SuppressSystemWarnings && !ShowInSystemHeader && >>>>> Loc.isValid() && >>>>> + if (State->SuppressSystemWarnings && !ShowInSystemHeader && >>>>> Loc.isValid() && >>>>> Diag.getSourceManager().isInSystemHeader( >>>>> Diag.getSourceManager().getExpansionLoc(Loc))) >>>>> return diag::Severity::Ignored; >>>>> @@ -632,7 +628,7 @@ bool DiagnosticIDs::ProcessDiag(Diagnost >>>>> >>>>> // If a fatal error has already been emitted, silence all subsequent >>>>> // diagnostics. >>>>> - if (Diag.FatalErrorOccurred) { >>>>> + if (Diag.FatalErrorOccurred && Diag.SuppressAfterFatalError) { >>>>> if (DiagLevel >= DiagnosticIDs::Error && >>>>> Diag.Client->IncludeInDiagnosticCounts()) { >>>>> ++Diag.NumErrors; >>>>> >>>>> Modified: cfe/trunk/lib/Serialization/ASTReader.cpp >>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serializat >>>>> ion/ASTReader.cpp?rev=301992&r1=301991&r2=301992&view=diff >>>>> ============================================================ >>>>> ================== >>>>> --- cfe/trunk/lib/Serialization/ASTReader.cpp (original) >>>>> +++ cfe/trunk/lib/Serialization/ASTReader.cpp Tue May 2 19:28:49 2017 >>>>> @@ -5531,14 +5531,8 @@ void ASTReader::ReadPragmaDiagnosticMapp >>>>> "Invalid data, not enough diag/map pairs"); >>>>> while (Size--) { >>>>> unsigned DiagID = Record[Idx++]; >>>>> - unsigned SeverityAndUpgradedFromWarning = Record[Idx++]; >>>>> - bool WasUpgradedFromWarning = >>>>> - DiagnosticMapping::deserializeUpgradedFromWarning( >>>>> - SeverityAndUpgradedFromWarning); >>>>> DiagnosticMapping NewMapping = >>>>> - Diag.makeUserMapping(Diagnosti >>>>> cMapping::deserializeSeverity( >>>>> - SeverityAndUpgradedFromWarning), >>>>> - Loc); >>>>> + DiagnosticMapping::deserialize(Record[Idx++]); >>>>> if (!NewMapping.isPragma() && !IncludeNonPragmaStates) >>>>> continue; >>>>> >>>>> @@ -5547,14 +5541,12 @@ void ASTReader::ReadPragmaDiagnosticMapp >>>>> // If this mapping was specified as a warning but the >>>>> severity was >>>>> // upgraded due to diagnostic settings, simulate the current >>>>> diagnostic >>>>> // settings (and use a warning). >>>>> - if (WasUpgradedFromWarning && !Mapping.isErrorOrFatal()) { >>>>> - Mapping = Diag.makeUserMapping(diag::Severity::Warning, >>>>> Loc); >>>>> - continue; >>>>> + if (NewMapping.wasUpgradedFromWarning() && >>>>> !Mapping.isErrorOrFatal()) { >>>>> + NewMapping.setSeverity(diag::Severity::Warning); >>>>> + NewMapping.setUpgradedFromWarning(false); >>>>> } >>>>> >>>>> - // Use the deserialized mapping verbatim. >>>>> Mapping = NewMapping; >>>>> - Mapping.setUpgradedFromWarning(WasUpgradedFromWarning); >>>>> } >>>>> return NewState; >>>>> }; >>>>> @@ -5569,22 +5561,36 @@ void ASTReader::ReadPragmaDiagnosticMapp >>>>> DiagStates.push_back(FirstState); >>>>> >>>>> // Skip the initial diagnostic state from the serialized module. >>>>> - assert(Record[0] == 0 && >>>>> + assert(Record[1] == 0 && >>>>> "Invalid data, unexpected backref in initial state"); >>>>> - Idx = 2 + Record[1] * 2; >>>>> + Idx = 3 + Record[2] * 2; >>>>> assert(Idx < Record.size() && >>>>> "Invalid data, not enough state change pairs in initial >>>>> state"); >>>>> + } else if (F.isModule()) { >>>>> + // For an explicit module, preserve the flags from the module >>>>> build >>>>> + // command line (-w, -Weverything, -Werror, ...) along with any >>>>> explicit >>>>> + // -Wblah flags. >>>>> + unsigned Flags = Record[Idx++]; >>>>> + DiagState Initial; >>>>> + Initial.SuppressSystemWarnings = Flags & 1; Flags >>= 1; >>>>> + Initial.ErrorsAsFatal = Flags & 1; Flags >>= 1; >>>>> + Initial.WarningsAsErrors = Flags & 1; Flags >>= 1; >>>>> + Initial.EnableAllWarnings = Flags & 1; Flags >>= 1; >>>>> + Initial.IgnoreAllWarnings = Flags & 1; Flags >>= 1; >>>>> + Initial.ExtBehavior = (diag::Severity)Flags; >>>>> + FirstState = ReadDiagState(Initial, SourceLocation(), true); >>>>> + >>>>> + // Set up the root buffer of the module to start with the >>>>> initial >>>>> + // diagnostic state of the module itself, to cover files that >>>>> contain no >>>>> + // explicit transitions (for which we did not serialize >>>>> anything). >>>>> + Diag.DiagStatesByLoc.Files[F.OriginalSourceFileID] >>>>> + .StateTransitions.push_back({FirstState, 0}); >>>>> } else { >>>>> - FirstState = ReadDiagState( >>>>> - F.isModule() ? DiagState() : *Diag.DiagStatesByLoc.CurDiagS >>>>> tate, >>>>> - SourceLocation(), F.isModule()); >>>>> - >>>>> - // For an explicit module, set up the root buffer of the module >>>>> to start >>>>> - // with the initial diagnostic state of the module itself, to >>>>> cover files >>>>> - // that contain no explicit transitions. >>>>> - if (F.isModule()) >>>>> - Diag.DiagStatesByLoc.Files[F.OriginalSourceFileID] >>>>> - .StateTransitions.push_back({FirstState, 0}); >>>>> + // For prefix ASTs, start with whatever the user configured on >>>>> the >>>>> + // command line. >>>>> + Idx++; // Skip flags. >>>>> + FirstState = ReadDiagState(*Diag.DiagStatesByLoc.CurDiagState, >>>>> + SourceLocation(), false); >>>>> } >>>>> >>>>> // Read the state transitions. >>>>> >>>>> Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp >>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serializat >>>>> ion/ASTWriter.cpp?rev=301992&r1=301991&r2=301992&view=diff >>>>> ============================================================ >>>>> ================== >>>>> --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original) >>>>> +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Tue May 2 19:28:49 2017 >>>>> @@ -2868,8 +2868,26 @@ void ASTWriter::WritePragmaDiagnosticMap >>>>> unsigned CurrID = 0; >>>>> RecordData Record; >>>>> >>>>> + auto EncodeDiagStateFlags = >>>>> + [](const DiagnosticsEngine::DiagState *DS) -> unsigned { >>>>> + unsigned Result = (unsigned)DS->ExtBehavior; >>>>> + for (unsigned Val : >>>>> + {DS->IgnoreAllWarnings, DS->EnableAllWarnings, >>>>> DS->WarningsAsErrors, >>>>> + DS->ErrorsAsFatal, DS->SuppressSystemWarnings}) >>>>> + Result = (Result << 1) | Val; >>>>> + return Result; >>>>> + }; >>>>> + >>>>> + unsigned Flags = EncodeDiagStateFlags(Diag.Diag >>>>> StatesByLoc.FirstDiagState); >>>>> + Record.push_back(Flags); >>>>> + >>>>> auto AddDiagState = [&](const DiagnosticsEngine::DiagState *State, >>>>> bool IncludeNonPragmaStates) { >>>>> + // Ensure that the diagnostic state wasn't modified since it was >>>>> created. >>>>> + // We will not correctly round-trip this information otherwise. >>>>> + assert(Flags == EncodeDiagStateFlags(State) && >>>>> + "diag state flags vary in single AST file"); >>>>> + >>>>> unsigned &DiagStateID = DiagStateIDMap[State]; >>>>> Record.push_back(DiagStateID); >>>>> >>>>> @@ -2882,7 +2900,7 @@ void ASTWriter::WritePragmaDiagnosticMap >>>>> for (const auto &I : *State) { >>>>> if (I.second.isPragma() || IncludeNonPragmaStates) { >>>>> Record.push_back(I.first); >>>>> - Record.push_back(I.second.serializeBits()); >>>>> + Record.push_back(I.second.serialize()); >>>>> } >>>>> } >>>>> // Update the placeholder. >>>>> >>>>> Modified: cfe/trunk/test/Index/keep-going.cpp >>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/kee >>>>> p-going.cpp?rev=301992&r1=301991&r2=301992&view=diff >>>>> ============================================================ >>>>> ================== >>>>> --- cfe/trunk/test/Index/keep-going.cpp (original) >>>>> +++ cfe/trunk/test/Index/keep-going.cpp Tue May 2 19:28:49 2017 >>>>> @@ -25,5 +25,5 @@ class C : public A<float> { }; >>>>> // CHECK: C++ base class specifier=A<float>:4:7 [access=public >>>>> isVirtual=false] [type=A<float>] [typekind=Unexposed] [templateargs/1= >>>>> [type=float] [typekind=Float]] [canonicaltype=A<float>] >>>>> [canonicaltypekind=Record] [canonicaltemplateargs/1= [type=float] >>>>> [typekind=Float]] [isPOD=0] [nbFields=1] >>>>> // CHECK: TemplateRef=A:4:7 [type=] [typekind=Invalid] [isPOD=0] >>>>> >>>>> -// CHECK-DIAG: keep-going.cpp:1:10: error: 'missing1.h' file not found >>>>> -// CHECK-DIAG: keep-going.cpp:8:10: error: 'missing2.h' file not found >>>>> +// CHECK-DIAG: keep-going.cpp:1:10: fatal error: 'missing1.h' file >>>>> not found >>>>> +// CHECK-DIAG: keep-going.cpp:8:10: fatal error: 'missing2.h' file >>>>> not found >>>>> >>>>> Modified: cfe/trunk/test/Modules/diag-flags.cpp >>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/d >>>>> iag-flags.cpp?rev=301992&r1=301991&r2=301992&view=diff >>>>> ============================================================ >>>>> ================== >>>>> --- cfe/trunk/test/Modules/diag-flags.cpp (original) >>>>> +++ cfe/trunk/test/Modules/diag-flags.cpp Tue May 2 19:28:49 2017 >>>>> @@ -1,20 +1,42 @@ >>>>> // RUN: rm -rf %t >>>>> // >>>>> -// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fmodules >>>>> -fimplicit-module-maps -emit-module -fmodules-cache-path=%t >>>>> -fmodule-name=diag_flags -x c++ %S/Inputs/module.map -fmodules-ts >>>>> -// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fmodules >>>>> -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s >>>>> -fmodules-ts >>>>> -// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fmodules >>>>> -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s >>>>> -fmodules-ts -DIMPLICIT_FLAG -Werror=padded >>>>> -// >>>>> -// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fmodules >>>>> -fimplicit-module-maps -emit-module -fmodule-name=diag_flags -x c++ >>>>> %S/Inputs/module.map -fmodules-ts -o %t/explicit.pcm >>>>> -Werror=string-plus-int -Wpadded >>>>> -// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fmodules >>>>> -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s >>>>> -fmodules-ts -DEXPLICIT_FLAG -fmodule-file=%t/explicit.pcm >>>>> -// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fmodules >>>>> -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s >>>>> -fmodules-ts -DEXPLICIT_FLAG -fmodule-file=%t/explicit.pcm -Werror=padded >>>>> +// For an implicit module, all that matters are the warning flags in >>>>> the user. >>>>> +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules >>>>> -fimplicit-module-maps -emit-module -fmodules-cache-path=%t >>>>> -fmodule-name=diag_flags -x c++ %S/Inputs/module.map -fmodules-ts >>>>> +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules >>>>> -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s >>>>> -fmodules-ts >>>>> +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules >>>>> -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s >>>>> -fmodules-ts -DWARNING -Wpadded >>>>> +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules >>>>> -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s >>>>> -fmodules-ts -DERROR -Wpadded -Werror >>>>> +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules >>>>> -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s >>>>> -fmodules-ts -DERROR -Werror=padded >>>>> +// >>>>> +// For an explicit module, all that matters are the warning flags in >>>>> the module build. >>>>> +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules >>>>> -fimplicit-module-maps -emit-module -fmodule-name=diag_flags -x c++ >>>>> %S/Inputs/module.map -fmodules-ts -o %t/nodiag.pcm >>>>> +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules >>>>> -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s >>>>> -fmodules-ts -fmodule-file=%t/nodiag.pcm -Wpadded >>>>> +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules >>>>> -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s >>>>> -fmodules-ts -fmodule-file=%t/nodiag.pcm -Werror -Wpadded >>>>> +// >>>>> +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules >>>>> -fimplicit-module-maps -emit-module -fmodule-name=diag_flags -x c++ >>>>> %S/Inputs/module.map -fmodules-ts -o %t/warning.pcm -Wpadded >>>>> +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules >>>>> -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s >>>>> -fmodules-ts -DWARNING -fmodule-file=%t/warning.pcm >>>>> +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules >>>>> -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s >>>>> -fmodules-ts -DWARNING -fmodule-file=%t/warning.pcm -Werror=padded >>>>> +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules >>>>> -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s >>>>> -fmodules-ts -DWARNING -fmodule-file=%t/warning.pcm -Werror >>>>> +// >>>>> +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules >>>>> -fimplicit-module-maps -emit-module -fmodule-name=diag_flags -x c++ >>>>> %S/Inputs/module.map -fmodules-ts -o %t/werror-no-error.pcm -Werror >>>>> -Wpadded -Wno-error=padded >>>>> +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules >>>>> -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s >>>>> -fmodules-ts -DWARNING -fmodule-file=%t/werror-no-error.pcm >>>>> +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules >>>>> -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s >>>>> -fmodules-ts -DWARNING -fmodule-file=%t/werror-no-error.pcm >>>>> -Wno-padded >>>>> +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules >>>>> -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s >>>>> -fmodules-ts -DWARNING -fmodule-file=%t/werror-no-error.pcm >>>>> -Werror=padded >>>>> +// >>>>> +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules >>>>> -fimplicit-module-maps -emit-module -fmodule-name=diag_flags -x c++ >>>>> %S/Inputs/module.map -fmodules-ts -o %t/error.pcm -Werror=padded >>>>> +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules >>>>> -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s >>>>> -fmodules-ts -DERROR -fmodule-file=%t/error.pcm -Wno-padded >>>>> +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules >>>>> -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s >>>>> -fmodules-ts -DERROR -fmodule-file=%t/error.pcm -Wno-error=padded >>>>> +// >>>>> +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules >>>>> -fimplicit-module-maps -emit-module -fmodule-name=diag_flags -x c++ >>>>> %S/Inputs/module.map -fmodules-ts -o %t/werror.pcm -Werror -Wpadded >>>>> +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules >>>>> -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s >>>>> -fmodules-ts -DERROR -fmodule-file=%t/werror.pcm -Wno-error >>>>> +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules >>>>> -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s >>>>> -fmodules-ts -DERROR -fmodule-file=%t/werror.pcm -Wno-padded >>>>> >>>>> import diag_flags; >>>>> >>>>> // Diagnostic flags from the module user make no difference to >>>>> diagnostics >>>>> // emitted within the module when using an explicitly-loaded module. >>>>> -#ifdef IMPLICIT_FLAG >>>>> +#if ERROR >>>>> // expected-error@diag_flags.h:14 {{padding struct}} >>>>> -#elif defined(EXPLICIT_FLAG) >>>>> +#elif WARNING >>>>> // expected-warning@diag_flags.h:14 {{padding struct}} >>>>> #else >>>>> // expected-no-diagnostics >>>>> >>>>> Modified: cfe/trunk/tools/libclang/CIndex.cpp >>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang >>>>> /CIndex.cpp?rev=301992&r1=301991&r2=301992&view=diff >>>>> ============================================================ >>>>> ================== >>>>> --- cfe/trunk/tools/libclang/CIndex.cpp (original) >>>>> +++ cfe/trunk/tools/libclang/CIndex.cpp Tue May 2 19:28:49 2017 >>>>> @@ -3313,7 +3313,7 @@ clang_parseTranslationUnit_Impl(CXIndex >>>>> Diags(CompilerInstance::createDiagnostics(new >>>>> DiagnosticOptions)); >>>>> >>>>> if (options & CXTranslationUnit_KeepGoing) >>>>> - Diags->setFatalsAsError(true); >>>>> + Diags->setSuppressAfterFatalError(false); >>>>> >>>>> // Recover resources if we crash before exiting this function. >>>>> llvm::CrashRecoveryContextCleanupRegistrar<DiagnosticsEngine, >>>>> >>>>> Modified: cfe/trunk/unittests/Basic/DiagnosticTest.cpp >>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basi >>>>> c/DiagnosticTest.cpp?rev=301992&r1=301991&r2=301992&view=diff >>>>> ============================================================ >>>>> ================== >>>>> --- cfe/trunk/unittests/Basic/DiagnosticTest.cpp (original) >>>>> +++ cfe/trunk/unittests/Basic/DiagnosticTest.cpp Tue May 2 19:28:49 >>>>> 2017 >>>>> @@ -46,27 +46,30 @@ TEST(DiagnosticTest, suppressAndTrap) { >>>>> EXPECT_FALSE(Diags.hasUnrecoverableErrorOccurred()); >>>>> } >>>>> >>>>> -// Check that FatalsAsErrors works as intended >>>>> -TEST(DiagnosticTest, fatalsAsErrors) { >>>>> - DiagnosticsEngine Diags(new DiagnosticIDs(), >>>>> - new DiagnosticOptions, >>>>> - new IgnoringDiagConsumer()); >>>>> - Diags.setFatalsAsError(true); >>>>> - >>>>> - // Diag that would set UncompilableErrorOccurred and ErrorOccurred. >>>>> - Diags.Report(diag::err_target_unknown_triple) << "unknown"; >>>>> - >>>>> - // Diag that would set UnrecoverableErrorOccurred and ErrorOccurred. >>>>> - Diags.Report(diag::err_cannot_open_file) << "file" << "error"; >>>>> - >>>>> - // Diag that would set FatalErrorOccurred >>>>> - // (via non-note following a fatal error). >>>>> - Diags.Report(diag::warn_mt_message) << "warning"; >>>>> - >>>>> - EXPECT_TRUE(Diags.hasErrorOccurred()); >>>>> - EXPECT_FALSE(Diags.hasFatalErrorOccurred()); >>>>> - EXPECT_TRUE(Diags.hasUncompilableErrorOccurred()); >>>>> - EXPECT_TRUE(Diags.hasUnrecoverableErrorOccurred()); >>>>> +// Check that SuppressAfterFatalError works as intended >>>>> +TEST(DiagnosticTest, suppressAfterFatalError) { >>>>> + for (unsigned Suppress = 0; Suppress != 2; ++Suppress) { >>>>> + DiagnosticsEngine Diags(new DiagnosticIDs(), >>>>> + new DiagnosticOptions, >>>>> + new IgnoringDiagConsumer()); >>>>> + Diags.setSuppressAfterFatalError(Suppress); >>>>> + >>>>> + // Diag that would set UnrecoverableErrorOccurred and >>>>> ErrorOccurred. >>>>> + Diags.Report(diag::err_cannot_open_file) << "file" << "error"; >>>>> + >>>>> + // Diag that would set FatalErrorOccurred >>>>> + // (via non-note following a fatal error). >>>>> + Diags.Report(diag::warn_mt_message) << "warning"; >>>>> + >>>>> + EXPECT_TRUE(Diags.hasErrorOccurred()); >>>>> + EXPECT_TRUE(Diags.hasFatalErrorOccurred()); >>>>> + EXPECT_TRUE(Diags.hasUncompilableErrorOccurred()); >>>>> + EXPECT_TRUE(Diags.hasUnrecoverableErrorOccurred()); >>>>> + >>>>> + // The warning should be emitted and counted only if we're not >>>>> suppressing >>>>> + // after fatal errors. >>>>> + EXPECT_EQ(Diags.getNumWarnings(), Suppress ? 0u : 1u); >>>>> + } >>>>> } >>>>> >>>>> } >>>>> >>>>> >>>>> _______________________________________________ >>>>> cfe-commits mailing list >>>>> cfe-commits@lists.llvm.org >>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>>>> >>>> >>>> >>> >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits