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

[Douglas Gregor]

On Jul 10, 2012, at 10:25 AM, Andy Gibbs <andyg1001 at hotmail.co.uk> wrote:

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

Yes, it's completely reasonable for me to fix up the test cases for my unfinished, undocumented feature ;) It shouldn't block your (much appreciated!) work on -verify.

	- Doug