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 <[email protected]> >>>>> wrote: >>>>>> On Mon, Oct 10, 2011 at 6:52 PM, Ahmed Charles <[email protected]> >>>>>> 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: [email protected]; [email protected] >>>>>>> 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 <[email protected]> >>>>>>> wrote: >>>>>>>> Here we go. >>>>>>>> >>>>>>>> On Fri, Oct 7, 2011 at 2:11 PM, Ahmed Charles <[email protected]> >>>>>>>> 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 <[email protected]> >>>>>>>>> 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 >>>>>>>>>>> <[email protected]> wrote: >>>>>>>>>>>> On Thu, Oct 6, 2011 at 1:16 PM, Ted Kremenek <[email protected]> >>>>>>>>>>>> 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 >>>>>>>> [email protected] >>>>>>>> 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> _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
