Looks good to me. On Mar 8, 2012, at 2:13 AM, Hans Wennborg <[email protected]> wrote:
> On Wed, Mar 7, 2012 at 19:35, Ted Kremenek <[email protected]> wrote: >> Hi Hans, >> >> I think I only have a minor nit: >> >> + "the '%0' %1 is not supported by ISO C">, InGroup<FormatNonStandard>, >> DefaultIgnore; >> >> It seems weird to start a diagnostic with "the". Why include it? Dropping >> it changes: >> >> the 'q' length modifier is not supported by ISO C >> >> to >> >> 'q' length modifier is not supported by ISO C >> >> The second seems just fine for a warning. > Agreed. > > >> We can also probably be a bit more succinct with this diagnostic: >> >> "using the length modifier '%0' with the conversion specifier '%1' is not >> supported by ISO C" >> >> can instead be: >> >> "using length modifier '%0' with conversion specifier '%1' is not supported >> by ISO C" >> >> The second wording is more direct, and a bit more clinical, but the more >> verbose version doesn't add much value and just makes the warning longer. > Done. > > New patch attached. OK to commit? > The clang-tests suite will need a small update because of the > re-wording of the warnings. (Attached.) > > Thanks, > Hans > <positional-arguments-warning3.diff><clang-tests.diff> _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
