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. 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. On Mar 7, 2012, at 10:00 AM, Hans Wennborg <[email protected]> wrote: > On Thu, Mar 1, 2012 at 19:00, Ted Kremenek <[email protected]> wrote: >> On Feb 28, 2012, at 3:03 AM, Hans Wennborg <[email protected]> wrote: >>> On Mon, Feb 27, 2012 at 20:00, Ted Kremenek <[email protected]> wrote: >>>> For me the goal of the warning is to warn about non-portable code, not >>>> annoy >>>> people. Format specifiers and format string extensions covered by POSIX >>>> are >>>> by definition portable on POSIX-compliant systems. So I raise the question >>>> of whether or not we should warn about these at all? >>> >>> I agree that a good warning about non-portable code, that could be >>> turned on by default or as part of -Wall, would be the ideal. >>> >>> However, we're not there yet. In the meantime, I think having a >>> warning under -pedantic that warns about non-ISO C format strings >>> makes sense. I agree that it would be extremely annoying to warn about >>> POSIX extensions by default, but under -pedantic I think users would >>> expect to get warnings about these, just as with GCC. >> >> Ok, I'm fine with this approach (putting under -pedantic), but should we put >> it under a separate warning group (that is activated by >> -Wformat-non-standard). That way people could turn off these warnings if >> all the care about is POSIX compatibility by still keep the rest of the >> portability warnings. >> >>> >>> Maybe the wording of the warnings and the name of the flag could be >>> changed to make this intention more clear. I agree that just saying >>> "non-standard" is a bit vague in the light of some of these features >>> actually being standardized under POSIX. >> >> I think improving the wording would help quite a bit. What is "standard" is >> also a moving target, so saying what the "standard" is might help a great >> deal (e.g., C99). It also may just add confusion. Another way is to have >> -Wformat-posix-extensions, which is activated by -Wformat-non-standard (as I >> suggested above), and just have a parenthetical note in the warning that >> says "POSIX extension". > > > Attaching a new patch that re-names the warning flag and rewords the > warnings a little bit. Hopefully this makes it much more clear. > > Thanks, > Hans > <positional-arguments-warning2.diff> _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
