[PATCH] PR18327: -Wsystem-headers introduces build errors

[Alp Toker]

On 08/01/2014 04:33, Alp Toker wrote:
>
> On 08/01/2014 04:21, Argyrios Kyrtzidis wrote:
>> On Jan 7, 2014, at 7:56 PM, Alp Toker <alp at nuanti.com> wrote:
>>
>>> On 08/01/2014 01:48, Argyrios Kyrtzidis wrote:
>>>> On Jan 6, 2014, at 1:47 PM, Richard Smith <richard at metafoo.co.uk 
>>>> <mailto:richard at metafoo.co.uk>> wrote:
>>>>
>>>>> One view on this is simply: -Wsystem-headers means "don't give 
>>>>> system headers special treatment when emitting diagnostics”.
>>>> This is it exactly. “Treat all headers like normal headers"
>>> We don't have a flag to treat all headers as normal headers at the 
>>> moment. It'd be very simple to implement compared to 
>>> -Wsystem-headers which somewhat intricate.
>> Could you elaborate, AFAICT '-Wsystem-headers' is treated specially, 
>> it's outside the diagnostic group machinery, and acts essentially as 
>> a flag.
>> If you'd like to have something like '-fwarn-on-system-headers' or 
>> something instead, that's another discussion, but as far as the PR is 
>> concerned I don't see why we need to change what -Wsystem-headers is 
>> currently doing.
>
> PR18327 reports a valid corner-case bug in the way a very small number 
> (around 8 in total) diagnostics are upgraded from warnings/extensions 
> to errors, and then were forgetting to downgrade them back to warnings 
> as all the other diagnostics are seen. So it's just an implementation 
> detail that was leaking and manifesting as errors in a context where 
> it should have been impossible.

I should add that the upgrade/downgrade terminology is only one way of 
looking at it. One of Richard's original suggestions, to have a 
maxSystemHeader level would also achieve the same thing (and possibly be 
more accurate with user-specified -Werror levels, but I feel that's an 
additional feature at this point).

Alp.


>
> Remember that this flag doesn't control the very many 
> isInSystemHeader() and isInSystemMacro() checks that happen earlier 
> than the diagnostic machinery. A -fno-system-headers flag to just 
> disable the whole system header machinery would be separately useful 
> though, agreed.
>
> Alp.
>
>
>
>
>>
>>> Alp.
>>>
>>>>> That would seem to make perfect sense to people developing system 
>>>>> headers, and is our current behavior. What is the use case that 
>>>>> leads to enabling -Wsystem-headers but not wanting that to lead to 
>>>>> errors? PR18327 doesn't make that obvious.
>>>> Not sure I’m following that report, if one doesn’t like that that 
>>>> diagnostic is by default mapped to an error, maybe map it to a 
>>>> warning on the command-line or discuss whether it should not be 
>>>> mapped to error by default ?
>>>> I don’t see a need to complicate what -Wsystem-headers does.
>>>>
>>> -- 
>>> http://www.nuanti.com
>>> the browser experts
>>>
>

-- 
http://www.nuanti.com
the browser experts