Summary: | RFE: have -Wformat suggest the correct format string to use | ||
---|---|---|---|
Product: | gcc | Reporter: | David Malcolm <dmalcolm> |
Component: | c | Assignee: | David Malcolm <dmalcolm> |
Status: | RESOLVED FIXED | ||
Severity: | enhancement | CC: | jakub, manu, msebor |
Priority: | P3 | Keywords: | diagnostic, easyhack |
Version: | 5.0 | ||
Target Milestone: | --- | ||
Host: | Target: | ||
Build: | Known to work: | ||
Known to fail: | Last reconfirmed: | 2015-02-27 00:00:00 |
Description
David Malcolm
2015-02-05 21:33:29 UTC
(In reply to David Malcolm from comment #0) > Currently, -Wformat tells me that I got something wrong but doesn't tell we > what the correct format string is: I think this should not be hard given the tables already available when checking formats. This could be an EasyHack. About generating a patch, we would first need precise locations within format strings (PR52952), then implement the fix-it hints (PR62314) and editors like Emacs could parse them to patch the code. Unfortunately, the time I can dedicate to GCC has been very much reduced lately and I haven't been able to find anyone to continue this work. The problem here is that usually there are more possibilities of fixing the code, and so it is a question what to suggest, it might as well just annoy users when it constantly suggests the opposite hint the user wants to take. In some cases, it is desirable to change the format specifier, in others to change the argument (cast it to something, ...), etc. FWIW, if printing a new note is a concern, perhaps simply enhancing the existing warning to mention the appropriate format specifier for the type along the following lines might be less nannying: warning: format ‘%i’ expects argument of type ‘int’, but argument 2 has type ‘long long int’ which requires ’%lli’ [-Wformat=] For what is worth, Clang is already doing this: prog.cc:5:19: warning: format specifies type 'int' but the argument has type 'long long' [-Wformat] printf ("%i\n", ll); ~~ ^~ %lli (In reply to David Malcolm from comment #0) > (Bonus points: have GCC generate a patch to fix it ;) ) Clang fix-it hints can automatically fix code: http://clang.llvm.org/extra/clang-tidy/ I guess generating a patch should not be big deal to them. Candidate patch: https://gcc.gnu.org/ml/gcc-patches/2016-07/msg01735.html Author: dmalcolm Date: Mon Aug 8 22:50:47 2016 New Revision: 239260 URL: https://gcc.gnu.org/viewcvs?rev=239260&root=gcc&view=rev Log: c-format.c: suggest the correct format string to use (PR c/64955) This adds fix-it hints to c-format.c so that it can (sometimes) suggest the format string the user should have used. The patch adds selftests for the new code in c-format.c. These selftests are thus lang-specific. This is the first time we've had lang-specific selftests, and hence the patch also adds a langhook for running them. (Note that currently the Makefile only invokes the selftests for cc1). gcc/c-family/ChangeLog: PR c/64955 * c-common.h (selftest::c_format_c_tests): New declaration. (selftest::run_c_tests): New declaration. * c-format.c: Include "selftest.h. (format_warning_va): Add param "corrected_substring" and use it to add a replacement fix-it hint. (format_warning_at_substring): Likewise. (format_warning_at_char): Update for new param of format_warning_va. (argument_parser::check_argument_type): Pass "fki" to check_format_types. (check_format_types): Add param "fki" and pass it to format_type_warning. (deref_n_times): New function. (get_modifier_for_format_len): New function. (selftest::test_get_modifier_for_format_len): New function. (get_format_for_type): New function. (format_type_warning): Add param "fki" and use it to attempt to provide hints for argument types when calling format_warning_at_substring. (selftest::get_info): New function. (selftest::assert_format_for_type_streq): New function. (ASSERT_FORMAT_FOR_TYPE_STREQ): New macro. (selftest::test_get_format_for_type_printf): New function. (selftest::test_get_format_for_type_scanf): New function. (selftest::c_format_c_tests): New function. gcc/c/ChangeLog: PR c/64955 * c-lang.c (LANG_HOOKS_RUN_LANG_SELFTESTS): If CHECKING_P, wire this up to selftest::run_c_tests. (selftest::run_c_tests): New function. gcc/ChangeLog: PR c/64955 * langhooks-def.h (LANG_HOOKS_RUN_LANG_SELFTESTS): New default do-nothing langhook. (LANG_HOOKS_INITIALIZER): Add LANG_HOOKS_RUN_LANG_SELFTESTS. * langhooks.h (struct lang_hooks): Add run_lang_selftests. * selftest-run-tests.c: Include "tree.h" and "langhooks.h". (selftest::run_tests): Call lang_hooks.run_lang_selftests. gcc/testsuite/ChangeLog: PR c/64955 * gcc.dg/format/diagnostic-ranges.c: Add fix-it hints to expected output. Modified: trunk/gcc/ChangeLog trunk/gcc/c-family/ChangeLog trunk/gcc/c-family/c-common.h trunk/gcc/c-family/c-format.c trunk/gcc/c/ChangeLog trunk/gcc/c/c-lang.c trunk/gcc/langhooks-def.h trunk/gcc/langhooks.h trunk/gcc/selftest-run-tests.c trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c Marking as resolved. |