Re: [PATCH 4/4] c-format.c: suggest the correct format string to use (PR c/64955)

[Jeff Law <law@redhat.com>]

On 08/03/2016 09:45 AM, David Malcolm wrote:
> 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).
>
> Successfully bootstrapped&regrtested in conjunction with the rest of the
> patch kit on x86_64-pc-linux-gnu.
>
> (The v2 version of the patch had a successful selftest run for stage 1 on
> powerpc-ibm-aix7.1.3.0 (gcc111) in conjunction with the rest of the patch
> kit, and a successful build of stage1 for all targets via config-list.mk;
> the patch has only been rebased since)
>
> OK for trunk if it passes testing?
>
> 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.
> 	(check_format_info_main): 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.
So presumably we always use the type of the argument as the "correct" 
type and assume the format string is what needs to be fixed (with the 
exception of getting the right amount of *s handled).  That seems 
intuitively the right thing to do, but do we have a hit rate better than 
50% in practice?

This is OK.  I'm really just curious about your thoughts/experiences on 
the heuristics.

jeff