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

[Andy Gibbs]

On Tuesday, July 03, 2012 2:33 AM, Jordan Rose wrote:
>> Index: tools/clang/test/Modules/Inputs/Module.framework/Headers/Module.h
>> ==================================================================
>> --- tools/clang/test/Modules/Inputs/Module.framework/Headers/Module.h
>> +++ tools/clang/test/Modules/Inputs/Module.framework/Headers/Module.h
>> @@ -1,6 +1,6 @@
>> -// expected-warning{{umbrella header}}
>> +// expected-warning 0-1 {{umbrella header}}
>>
>> #ifndef MODULE_H
>> #define MODULE_H
>> const char *getModuleVersion(void);
>
> This does not mean the same thing. Why is it being changed?

You are right, it means the expected warning may or may not appear once.
What you observe here is a test-case bug which was exposed by handling
included files properly once my patch is applied.  I imagine that if proper
handling of include files was always the case, this test-case would have
been written differently...

Let me give an example: (assuming my patch isn't applied)

///// test-case-include.h
// expected-warning{{some warning}}

///// test-case.c
#include "test-case-include.h"

What would you expect if you ran the test-case with -verify?  I should
expect that it fails since there is an unseen warning, however, it will
pass.  This is one of the bugs in -verify which is now fixed.

In the case above, Module.h is shared across a number of tests.  In some
tests the include file was parsed correctly and in others it was not.  (I
made some comments about a net with holes in another post and this
is one example of where it applied!)  Unfortunately, this incorrect parsing
coincided with the cases where the diagnostic also not generated (if you
look at the original implementation you will understand why), so the
test-case bug was missed.  Since the diagnostic sometimes is and
sometimes is not generated, hence the "0-1".

> In general I think the "referenced here" / "declared here" notes don't 
> need to be
> moved. Sure, going forward we should be writing them using @line, but I 
> think
> it's okay to leave existing ones as is. Although I would actually expect 
> there to be
>many more such notes, so were you already only doing a few of them? How
> were they being chosen?

You mean the PCH test-cases?  Well in this case, as you know, the PCH 
doesn't
include comments within it once it is generated, hence these comments must
be moved out of the PCH declaration into the actual test-case -- i.e. out of 
the
#if/#endif that is used for the PCH generation.

Can I refer you to my submission post which, hopefully, explains a lot of 
the
rationale behind the patch, the necessary test-case changes, the bugs fixed, 
etc.:

http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20120611/058902.html

Thanks
Andy