Re: r301992 - [modules] Round-trip -Werror flag through explicit module build.

2017-05-04 Thread Alex L via cfe-commits
On 4 May 2017 at 12:52, Alex L  wrote:

>
>
> On 4 May 2017 at 12:43, Alex L  wrote:
>
>>
>>
>> On 3 May 2017 at 22:23, Richard Smith  wrote:
>>
>>> On 3 May 2017 at 07:29, Alex L  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: 0x000109db2edf libclang.dylib`clang::Diagnost
 icsEngine::ReportDelayed(this=0x00011c002c00) at Diagnostic.cpp:149
 frame #33529: 0x000109db5a36 libclang.dylib`clang::Diagnost
 icsEngine::EmitCurrentDiagnostic(this=0x00011c002c00, Force=false)
 at Diagnostic.cpp:428
 frame #33530: 0x00010a33f93c libclang.dylib`clang::Diagnost
 icBuilder::Emit(this=0x78d4bfd8) at Diagnostic.h:1013
 frame #33531: 0x00010a33f8e5 libclang.dylib`clang::Diagnost
 icBuilder::~DiagnosticBuilder(this=0x78d4bfd8) at
 Diagnostic.h:1036
 frame #33532: 0x00010a335015 libclang.dylib`clang::Diagnost
 icBuilder::~DiagnosticBuilder(this=0x78d4bfd8) 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

Re: r301992 - [modules] Round-trip -Werror flag through explicit module build.

2017-05-04 Thread Alex L via cfe-commits
On 4 May 2017 at 12:43, Alex L  wrote:

>
>
> On 3 May 2017 at 22:23, Richard Smith  wrote:
>
>> On 3 May 2017 at 07:29, Alex L  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: 0x000109db2edf libclang.dylib`clang::Diagnost
>>> icsEngine::ReportDelayed(this=0x00011c002c00) at Diagnostic.cpp:149
>>> frame #33529: 0x000109db5a36 libclang.dylib`clang::Diagnost
>>> icsEngine::EmitCurrentDiagnostic(this=0x00011c002c00, Force=false)
>>> at Diagnostic.cpp:428
>>> frame #33530: 0x00010a33f93c libclang.dylib`clang::Diagnost
>>> icBuilder::Emit(this=0x78d4bfd8) at Diagnostic.h:1013
>>> frame #33531: 0x00010a33f8e5 libclang.dylib`clang::Diagnost
>>> icBuilder::~DiagnosticBuilder(this=0x78d4bfd8) at
>>> Diagnostic.h:1036
>>> frame #33532: 0x00010a335015 libclang.dylib`clang::Diagnost
>>> icBuilder::~DiagnosticBuilder(this=0x78d4bfd8) 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@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
 - 

Re: r301992 - [modules] Round-trip -Werror flag through explicit module build.

2017-05-04 Thread Alex L via cfe-commits
On 3 May 2017 at 22:23, Richard Smith  wrote:

> On 3 May 2017 at 07:29, Alex L  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: 0x000109db2edf libclang.dylib`clang::Diagnost
>> icsEngine::ReportDelayed(this=0x00011c002c00) at Diagnostic.cpp:149
>> frame #33529: 0x000109db5a36 libclang.dylib`clang::Diagnost
>> icsEngine::EmitCurrentDiagnostic(this=0x00011c002c00, Force=false)
>> at Diagnostic.cpp:428
>> frame #33530: 0x00010a33f93c libclang.dylib`clang::Diagnost
>> icBuilder::Emit(this=0x78d4bfd8) at Diagnostic.h:1013
>> frame #33531: 0x00010a33f8e5 libclang.dylib`clang::Diagnost
>> icBuilder::~DiagnosticBuilder(this=0x78d4bfd8) at
>> Diagnostic.h:1036
>> frame #33532: 0x00010a335015 libclang.dylib`clang::Diagnost
>> icBuilder::~DiagnosticBuilder(this=0x78d4bfd8) 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.



>
>
>> 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 SuppressSys

Re: r301992 - [modules] Round-trip -Werror flag through explicit module build.

2017-05-03 Thread Richard Smith via cfe-commits
On 3 May 2017 at 07:29, Alex L  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: 0x000109db2edf libclang.dylib`clang::
> DiagnosticsEngine::ReportDelayed(this=0x00011c002c00) at
> Diagnostic.cpp:149
> frame #33529: 0x000109db5a36 libclang.dylib`clang::
> DiagnosticsEngine::EmitCurrentDiagnostic(this=0x00011c002c00,
> Force=false) at Diagnostic.cpp:428
> frame #33530: 0x00010a33f93c libclang.dylib`clang::
> DiagnosticBuilder::Emit(this=0x78d4bfd8) at Diagnostic.h:1013
> frame #33531: 0x00010a33f8e5 libclang.dylib`clang::
> DiagnosticBuilder::~DiagnosticBuilder(this=0x78d4bfd8) at
> Diagnostic.h:1036
> frame #33532: 0x00010a335015 libclang.dylib`clang::
> DiagnosticBuilder::~DiagnosticBuilder(this=0x78d4bfd8) 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.


> 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 Diags;
>>IntrusiveRefCntPtr DiagOpts;
>>DiagnosticConsumer *Client

Re: r301992 - [modules] Round-trip -Werror flag through explicit module build.

2017-05-03 Thread Alex L via cfe-commits
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: 0x000109db2edf
libclang.dylib`clang::DiagnosticsEngine::ReportDelayed(this=0x00011c002c00)
at Diagnostic.cpp:149
frame #33529: 0x000109db5a36
libclang.dylib`clang::DiagnosticsEngine::EmitCurrentDiagnostic(this=0x00011c002c00,
Force=false) at Diagnostic.cpp:428
frame #33530: 0x00010a33f93c
libclang.dylib`clang::DiagnosticBuilder::Emit(this=0x78d4bfd8) at
Diagnostic.h:1013
frame #33531: 0x00010a33f8e5
libclang.dylib`clang::DiagnosticBuilder::~DiagnosticBuilder(this=0x78d4bfd8)
at Diagnostic.h:1036
frame #33532: 0x00010a335015
libclang.dylib`clang::DiagnosticBuilder::~DiagnosticBuilder(this=0x78d4bfd8)
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? I will
commit a fix for this now.

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 Diags;
>IntrusiveRefCntPtr DiagOpts;
>DiagnosticConsumer *Client;
> @@ -216,6 +210,19 @@ private:
>  llvm::DenseMap 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),
> +

r301992 - [modules] Round-trip -Werror flag through explicit module build.

2017-05-02 Thread Richard Smith via cfe-commits
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 Diags;
   IntrusiveRefCntPtr DiagOpts;
   DiagnosticConsumer *Client;
@@ -216,6 +210,19 @@ private:
 llvm::DenseMap 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::iterator iterator;
 typedef llvm::DenseMap::const_iterator
 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 War