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

[Andy Gibbs]

On Friday, July 06, 2012 5:00 PM, Douglas Gregor wrote:
> On Jul 3, 2012, at 5:48 PM, Jordan Rose wrote:
>> On Jul 3, 2012, at 3:23 PM, Andy Gibbs wrote:
>>> On Tuesday, July 03, 2012 5:18 PM, Jordan Rose wrote:
>>>> On Jul 2, 2012, at 10:39 PM, Andy Gibbs wrote:
>>>> [...snip...]
>>>>> 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".
>>>> 
>>>> I see. It doesn't look like the "umbrella header" warning (-Wincomplete-umbrella)
>>>> is exercised anywhere else, though. Perhaps it should be put into a test of
>>>> its own? (I think it's reasonable to make a separate "Umbrella.framework"
>>>> because of the existing expectation in Module.framework.)
>>> 
>>> Hmm, I'm not sure I understand enough about how the modules part of the compiler
>>> works to change a test-case so drastically.  Would it not be better to get this
>>> changed by someone better qualified?
>> 
>> Let's rope Doug in on this part. Doug, what exactly is -Wincomplete-umbrella for?
> 
> It checks, at module build time, whether an umbrella header <Foo/Foo.h> actually
> includes all of the headers <Foo/*.h>.
> 
>> Is it okay to just test it in one place, and make the "default" Module.framework
>> a completely clean framework?
> 
> Yes, that would be cleaner.

Attached is the updated test-case set for the -verify patches.  I've stripped out
the test-case relating to the part 6 patch since this is being left out for now.
I also have not adjusted the test-case relating to the umbrella framework since
I really don't feel confident to change a test-case for a feature I do not fully
understand!  I would suggest that following the commital of these patches, that
someone else makes the necessary amendment to this particular test-case.  The
patch attached here as it stands fixes the test-case to ensure the test suite
passes, but that is all.

Cheers
Andy

-------------- next part --------------
A non-text attachment was scrubbed...
Name: verify-part7.diff
Type: application/octet-stream
Size: 19992 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120710/130be718/attachment.obj>