[cfe-commits] [Patch 4 of 7] -verify fixes and enhancement

[Jordan Rose]

On Jul 2, 2012, at 11:45 , Andy Gibbs <andyg1001 at hotmail.co.uk> wrote:

> On Monday, July 02, 2012 7:48 PM, Jordan Rose wrote:
>> On Jun 30, 2012, at 2:12 PM, Andy Gibbs wrote:
>> 
>>> Part 4: Removed accidentally included override on diagnostic notes.
>>> 
>>> <verify-part4.diff>
>> 
>> This one I don't like. -verify tests shouldn't also be fatal error tests anyway. Even
>> though it doesn't actually cause any problems. I don't think this is necessary.
>> 
>> (Also, it invites future code like ReallySuppressAllDiagnostics, etc., even if none
>> of us would ever do that.)
> 
> I take your point, but  my counter argument would be that it may be very possible
> to have the scenario where a test-case that originally didn't fatal error, might following
> an unintentional change the the compiler end up in a fatal error and which is then
> missed it because the diagnostics following the fatal error get swallowed up.  Right
> now I can't be 100% sure but I think this actually came up during testing. It was certainly
> one of the later iterations of the patch that included this change.

I'd rather just make this a rule: "fatal errors can't be tested with -verify", and have them not be suppressed by VerifyDiagnosticsConsumer. I guess this probably requires further discussion either way.

> 
> It may be worth thinking of another alternative, but I personally feel that it *is* very
> useful to have diagnostics from -verify in the situation (intended or not!) that a
> fatal error occurs.  (Of course, any alternative would still need to leave space for the
> future ReallySuppressAllDiagnostics option!! -- joking, of course!).
> 
> Andy
> 
>