This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 3/4] Use class substring_loc in c-format.c (PR c/52952)


On 08/03/2016 09:45 AM, David Malcolm wrote:
This patch updates c-format.c to use the new class substring_loc, added
in the previous patch, replacing location_column_from_byte_offset.
Hence with this patch, Wformat can underline the precise erroneous
format string in many more cases.

The patch also introduces two new functions for emitting Wformat
warnings: format_warning_at_substring and format_warning_at_char,
providing an inform in the face of macros where the pertinent part of
the format string may be separate from the function call.

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 individual testing? (on top of patches 1-2)

gcc/c-family/ChangeLog:
	PR c/52952
	* c-format.c: Include "diagnostic.h".
	(location_column_from_byte_offset): Delete.
	(location_from_offset): Delete.
	(format_warning_va): New function.
	(format_warning_at_substring): New function.
	(format_warning_at_char): New function.
	(check_format_arg): Capture location of format_tree and pass to
	check_format_info_main.
	(check_format_info_main): Add params FMT_PARAM_LOC and
	FORMAT_STRING_CST.  Convert calls to warning_at to calls to
	format_warning_at_char.  Pass a substring_loc instance to
	check_format_types.
	(check_format_types): Convert first param from a location_t
	to a const substring_loc & and rename to "fmt_loc".  Attempt
	to extract the range of the relevant parameter and pass it
	to format_type_warning.
	(format_type_warning): Convert first param from a location_t
	to a const substring_loc & and rename to "fmt_loc".  Add
	params "param_range" and "type".  Replace calls to warning_at
	with calls to format_warning_at_substring.

gcc/testsuite/ChangeLog:
	PR c/52952
	* gcc.dg/cpp/pr66415-1.c: Likewise.
	* gcc.dg/format/asm_fprintf-1.c: Update column numbers.
	* gcc.dg/format/c90-printf-1.c: Likewise.
	* gcc.dg/format/diagnostic-ranges.c: New test case.
---


@@ -1758,6 +1859,7 @@ check_format_info_main (format_check_results *res,
 	  ++format_chars;
 	  continue;
 	}
+      const char *start_of_this_format = format_chars;
Do you realize that this isn't used for ~700 lines after this point? Is there any sensible way to factor some code here to avoid the coding disconnect. I realize the function was huge before you got in here, but if at all possible, I'd like to see a bit of cleanup.

I think this is OK after that cleanup.

jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]