Bug 64955 - RFE: have -Wformat suggest the correct format string to use
Summary: RFE: have -Wformat suggest the correct format string to use
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 5.0
: P3 enhancement
Target Milestone: ---
Assignee: David Malcolm
URL:
Keywords: diagnostic, easyhack
Depends on:
Blocks:
 
Reported: 2015-02-05 21:33 UTC by David Malcolm
Modified: 2016-08-08 22:52 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2015-02-27 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Malcolm 2015-02-05 21:33:29 UTC
Currently, -Wformat tells me that I got something wrong but doesn't tell we what the correct format string is:

$ ./xgcc --version
xgcc (GCC) 5.0.0 20150204 (experimental)
(etc)

$ cat test.c
#include <stdio.h>

void test (long long ll)
{
  printf ("%i\n", ll);
}


$ ./xgcc -c -B. test.c -Wformat
test.c: In function ‘test’:
test.c:5:11: warning: format ‘%i’ expects argument of type ‘int’, but argument 2 has type ‘long long int’ [-Wformat=]
   printf ("%i\n", ll);
           ^

It would be helpful if gcc could emit something like:
test.c:5:11: note: ‘printf‘ expects format string ‘%lli’ for type ‘long long int’.

and maybe, if it can't be handled:
test.c:5:11: note: ‘printf‘ does not have a format string for handling type ‘struct foo’.

as an additional clarifying note after the warning/error

(Bonus points: have GCC generate a patch to fix it ;) )
Comment 1 Manuel López-Ibáñez 2015-02-27 14:48:47 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.
Comment 2 Jakub Jelinek 2015-02-27 14:56:35 UTC
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.
Comment 3 Martin Sebor 2015-10-21 22:49:18 UTC
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=]
Comment 4 Manuel López-Ibáñez 2016-06-11 22:12:56 UTC
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.
Comment 5 David Malcolm 2016-07-26 16:48:51 UTC
Candidate patch: https://gcc.gnu.org/ml/gcc-patches/2016-07/msg01735.html
Comment 6 David Malcolm 2016-08-08 22:51:18 UTC
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
Comment 7 David Malcolm 2016-08-08 22:52:02 UTC
Marking as resolved.