[cfe-commits] Warning flags

[Ted Kremenek]

Applied in r142284!

On Oct 17, 2011, at 1:34 PM, Ahmed Charles wrote:

> This is resync'd as of a few seconds ago. :)
> 
>>>> On Oct 11, 2011, at 12:34 AM, Ahmed Charles wrote:
>>>> 
>>>>> Right, it doesn't work. Here's a patch without those included, since I
>>>>> assume getting those to work will require a bit more effort.
>>>>> 
>>>>> On Mon, Oct 10, 2011 at 7:03 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
>>>>>> On Mon, Oct 10, 2011 at 6:52 PM, Ahmed Charles <ahmedcharles at gmail.com> wrote:
>>>>>>> it probably only does a subset of what it claims. I'm not sure what a
>>>>>>> better name would be and I'm certainly not experienced enough to know
>>>>>>> all of the other command line warnings that are possible.
>>>>>>> 
>>>>>>> Do you have a recommendation other than having a specific flag for each
>>>>>>> instance?
>>>>>> 
>>>>>> That's not really what I was trying to get at.  What I think will
>>>>>> happen is that we'll end up telling the user that a warning is
>>>>>> controlled by -Winvalid-commandline-option, but
>>>>>> -Wno-invalid-commandline-option won't actually turn it off.
>>>>>> 
>>>>>> -Eli
>>>>>> 
>>>>>>> From: Eli Friedman
>>>>>>> Sent: 10/10/2011 5:32 PM
>>>>>>> To: Ahmed Charles
>>>>>>> Cc: kremenek at apple.com; cfe-commits at cs.uiuc.edu
>>>>>>> Subject: Re: [cfe-commits] Warning flags
>>>>>>> Does -Wno-invalid-commandline-option actually work?  If not, it seems
>>>>>>> confusing to advertise it.
>>>>>>> 
>>>>>>> -Eli
>>>>>>> 
>>>>>>> On Sat, Oct 8, 2011 at 9:31 AM, Ahmed Charles <ahmedcharles at gmail.com> wrote:
>>>>>>>> Here we go.
>>>>>>>> 
>>>>>>>> On Fri, Oct 7, 2011 at 2:11 PM, Ahmed Charles <ahmedcharles at gmail.com> wrote:
>>>>>>>>> Yes, I used git, so it's easy to manage lots of small patches, but one
>>>>>>>>> large one is fine as well. I'll resend later.
>>>>>>>>> 
>>>>>>>>> On Fri, Oct 7, 2011 at 1:59 PM, Ted Kremenek <kremenek at apple.com> wrote:
>>>>>>>>>> Do you have one aggregate patch that will make this easier to review?
>>>>>>>>>> 
>>>>>>>>>> On Oct 7, 2011, at 1:53 AM, Ahmed Charles wrote:
>>>>>>>>>> 
>>>>>>>>>>> Here is the first few. They have to be applied in order, or the
>>>>>>>>>>> changes in the test will conflict. And hopefully the naming is
>>>>>>>>>>> appealing enough. :)
>>>>>>>>>>> 
>>>>>>>>>>> On Thu, Oct 6, 2011 at 2:10 PM, Ahmed Charles <ahmedcharles at gmail.com> wrote:
>>>>>>>>>>>> On Thu, Oct 6, 2011 at 1:16 PM, Ted Kremenek <kremenek at apple.com> wrote:
>>>>>>>>>>>>> On Oct 6, 2011, at 10:21 AM, Ahmed Charles wrote:
>>>>>>>>>>>>> 
>>>>>>>>>>>>> I'm looking into adding flags for the various warnings without them and was
>>>>>>>>>>>>> wondering what the bar is in terms of test cases? It seems like existing
>>>>>>>>>>>>> flags don't have explicit test cases and in some cases neither do the
>>>>>>>>>>>>> warnings.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> Good questions.  These are two separate issues.  It's simply bad that we
>>>>>>>>>>>>> have warnings that aren't being tested at all (behaviorally).  For those we
>>>>>>>>>>>>> should continue to add test cases to improve our coverage of the compiler's
>>>>>>>>>>>>> behavior.
>>>>>>>>>>>>> For testing coverage of warning flags, the only thing you could really test
>>>>>>>>>>>>> from a behavior perspective is whether passing -W/-Wno<warning>
>>>>>>>>>>>>> enables/disables the warning (or use pragmas that accomplish the same
>>>>>>>>>>>>> thing).  Many warnings are on by default, so many of the tests would need to
>>>>>>>>>>>>> go for the "disable warning" route.  We are pretty confident that the
>>>>>>>>>>>>> general warning suppression/enabling mechanism works (it is well tested), so
>>>>>>>>>>>>> we only really need to add specific tests like these for warnings where it
>>>>>>>>>>>>> is clear we want to tease out some warning from a larger class of warnings
>>>>>>>>>>>>> and have the ability to disable it (e.g., a user explicitly requested this
>>>>>>>>>>>>> functionality).
>>>>>>>>>>>>> So, for testing whether or not a warning has a flag, we have
>>>>>>>>>>>>> test/Misc/warning-flags.c.  Essentially we run diagtool to list all the
>>>>>>>>>>>>> warnings that are not covered by a flag.  Whenever a warning that was
>>>>>>>>>>>>> previously not covered by a flag gets a flag, this test needs to be updated
>>>>>>>>>>>>> (i.e., remove the entry).  That's usually sufficient in my opinion to test
>>>>>>>>>>>>> that a warning is covered by a flag.
>>>>>>>>>>>> 
>>>>>>>>>>>> Thanks, that's what I thought.
>>>>>>>>>>>> 
>>>>>>>>>>>> --
>>>>>>>>>>>> Ahmed Charles
>>>>>>>>>>>> 
>>>>>>>>>>> <0003-Place-diagnostic-backslash_newline_space-under-the-W.patch><0004-Place-diagnostics-null_in_string-null_in_char-and-nu.patch><0005-Place-renamed-diagnostic-ext_charize_microsoft-under.patch><0007-Place-diagnostic-ext_dollar_in_identifier-under-the-.patch><0008-Place-diagnostics-ext_c99_array_usage-ext_c99_compou.patch><0009-Place-diagnostic-ext_auto_storage_class-under-the-Wa.patch><0010-Place-diagnostics-ext_catch_incomplete_ref-and-ext_c.patch><0011-Place-diagnostics-ext_flexible_array_in_array-and-ex.patch><0012-Place-diagnostic-warn_delete_incomplete-under-the-Wd.patch><0013-Place-diagnostics-warn_c_kext-warn_drv_assuming_mflo.patch><0014-Place-diagnostics-warn_ucn_escape_too_large-and-warn.patch>
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> --
>>>>>>>>> Ahmed Charles
>>>>>>>>> 
>>>>>>>> 
>>>>>>>> _______________________________________________
>>>>>>>> cfe-commits mailing list
>>>>>>>> cfe-commits at cs.uiuc.edu
>>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>> --
>>>>> Ahmed Charles
>>>>> <0001-Place-various-warnings-under-new-W-flags.patch>
>>>> 
>>>> 
>>> 
>>> 
>>> 
>>> --
>>> Ahmed Charles
>>> <0001-Place-various-warnings-under-new-W-flags.patch>
>> 
>> 
> 
> 
> 
> -- 
> Ahmed Charles
> <0001-Place-various-warnings-under-new-W-flags.patch>