r301992 - [modules] Round-trip -Werror flag through explicit module build.
Alex L via cfe-commits
cfe-commits at lists.llvm.org
Thu May 4 04:52:24 PDT 2017
On 4 May 2017 at 12:43, Alex L <arphaman at gmail.com> wrote:
>
>
> On 3 May 2017 at 22:23, Richard Smith <richard at metafoo.co.uk> wrote:
>
>> On 3 May 2017 at 07:29, Alex L <arphaman at 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 will commit a fix for this now.
>>>
>>
>> Thank you!
>>
>>
>>> Alex
>>>
>>>
>>> On 3 May 2017 at 01:28, Richard Smith via cfe-commits <
>>> cfe-commits at 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 at diag_flags.h:14 {{padding struct}}
>>>> -#elif defined(EXPLICIT_FLAG)
>>>> +#elif WARNING
>>>> // expected-warning at 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 at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170504/43c30e6a/attachment-0001.html>
More information about the cfe-commits
mailing list