2014/1/8 Richard Smith <[email protected]>: > On Tue, Jan 7, 2014 at 8:33 PM, Alp Toker <[email protected]> wrote: >> >> >> On 08/01/2014 04:21, Argyrios Kyrtzidis wrote: >>> >>> On Jan 7, 2014, at 7:56 PM, Alp Toker <[email protected]> wrote: >>> >>>> On 08/01/2014 01:48, Argyrios Kyrtzidis wrote: >>>>> >>>>> On Jan 6, 2014, at 1:47 PM, Richard Smith <[email protected] >>>>> <mailto:[email protected]>> 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. > > > These diagnostics are set up as 'errors that we suppress in system headers' > (they're suppressed because they actually happen in some system headers). > > If the viewpoint is that '-Wsystem-headers' means 'don't suppress > diagnostics in system headers', then issuing errors in these cases seems > correct to me. If the viewpoint is that '-Wsystem-headers' means 'warn on > problems in system headers', then issuing warnings, not errors, in these > cases seems correct. It depends on what the user of the flag intends it to > mean. If an error has been downgraded for system headers I don't think it makes sense if -Wsystem-headers (which in the user's manual is described as "Enables warnings from system headers.") causes the build to fail. How is a system library developer supposed to check for warnings if it isn't possible to enable warnings without breaking the build?
> > I think the former option makes a little more sense for system header > developers -- if their system headers contain ill-formed code, they probably > want to know about that with more urgency than if their system headers > merely contain dubious code. But we still don't know what the person who > filed the original PR was trying to do, and maybe there's a use case where > the latter view makes more sense. The use case for me was that I was looking at a bug report and when reducing a test case I ended up finding a problem in lib\Headers. When enabling -Wsystem-headers the build failed early instead of completing and producing a warning to point me in the right direction. > >> 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. > > > I agree. If we want this flag to mean 'there are no system headers', it does > not go far enough. > >> >> >> 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 >> > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
