And this blocks PR52952 because with the *correct* offset, we get this in C++:

/home/manuel/test2/src/gcc/testsuite/g++.dg/ext/builtin4.C:9:16: warning: format ‘%d’ expects a matching ‘int’ argument [-Wformat=]
   printf("%d");   // { dg-warning "expects a matching" }
                ^

which is even worse than without offset locations.

*** Bug 68052 has been marked as a duplicate of this bug. ***

*** Bug 71863 has been marked as a duplicate of this bug. ***

Confirmed.  See the duplicate bug 71863 for another test case.

I thought I'd look into this bug since it affects the testing of my patch for bug 49905 and I'm finding out that it seems to be a general problem with C++ and function arguments.  From what I can see, the C front end tracks the locations of function arguments through the parser, passing them from function to function, while the C++ front end doesn't.  Sadly, it doesn't look like a simple fix...

$ cat xyz.c && /build/gcc-trunk-svn/gcc/xgcc -B /build/gcc-trunk-svn/gcc -S -Wall -Wextra -Wpedantic -Wformat -xc++ xyz.c
void f (int, int, int);

void g (void)
{
  f (0, "", 1);
}
xyz.c: In function ‘void g()’:
xyz.c:5:14: error: invalid conversion from ‘const char*’ to ‘int’ [-fpermissive]
   f (0, "", 1);
              ^
xyz.c:1:6: note:   initializing argument 2 of ‘void f(int, int, int)’
 void f (int, int, int);
      ^

(In reply to Martin Sebor from comment #5)
> I thought I'd look into this bug since it affects the testing of my patch
> for bug 49905 and I'm finding out that it seems to be a general problem with
> C++ and function arguments.  From what I can see, the C front end tracks the
> locations of function arguments through the parser, passing them from
> function to function, while the C++ front end doesn't.  Sadly, it doesn't
> look like a simple fix...

This happens for "expressions" that don't have a location (such as constants, variable uses, etc.). I'm not even sure that passing the location is enough, since the C++ parser will do a lot of tentative parsing and, by the time you call the function, input_location is too far away and the original location of the argument is lost. You may need to store the locations together with the arguments when they are parsed and somehow keep them together throughout the C++ FE. Which is of course redundant when the expressions actually have a location.


At the end, everything comes back to fixing PR43486

Thanks for the background and the pointer.  Is this report then a duplicate of bug 43486?

(In reply to Martin Sebor from comment #7)
> Thanks for the background and the pointer.  Is this report then a duplicate
> of bug 43486?

It may be possible to fix this one imperfectly without fixing PR43486. As you say, one can start passing locations from the parsing point to the warning point case by case and eventually you will cover most testcases.

Many people have tried to fix PR43486 and eventually desisted. It doesn't seem to be a problem that can be solved piecemeal: either you have a thoroughly designed solution that is supported by the maintainers and you commit yourself to do the transition in one go and fix all the fall-out or it will never happen.

*** Bug 85948 has been marked as a duplicate of this bug. ***

Candidate patch: https://gcc.gnu.org/ml/gcc-patches/2018-10/msg00269.html

Author: dmalcolm
Date: Fri Oct  5 19:02:17 2018
New Revision: 264887

URL: https://gcc.gnu.org/viewcvs?rev=264887&root=gcc&view=rev
Log:
Support string locations for C++ in -Wformat (PR c++/56856)

-Wformat in the C++ FE doesn't work as well as it could:
(a) it doesn't report precise locations within the string literal, and
(b) it doesn't underline arguments for those arguments !CAN_HAVE_LOCATION_P,
despite having location wrapper nodes.

For example:

  Wformat-ranges.C:32:10: warning: format '%s' expects argument of type 'char*', but argument 2 has type 'int' [-Wformat=]
  32 |   printf("hello %s", 42);
     |          ^~~~~~~~~~

(a) is due to not wiring up the langhook for extracting substring
    locations.

    This patch uses the one in c-family; it also fixes string literal
    parsing so that it records string concatenations (needed for
    extracting substring locations from concatenated strings).

(b) is due to the call to maybe_constant_value here:
       fargs[j] = maybe_constant_value (argarray[j]);
    within build_over_call.

    The patch fixes this by building a vec of location_t values when
    calling check_function_arguments.
    I attempted to eliminate the maybe_constant_value call here, but
    it's needed by e.g. check_function_sentinel for detecting NULL,
    and that code is in "c-family", so it can't simply call into
    maybe_constant_value (which is in "cp").

With this patch, the output for the above example is improved to:

  Wformat-ranges.C:32:18: warning: format '%s' expects argument of type 'char*', but argument 2 has type 'int' [-Wformat=]
  32 |   printf("hello %s", 42);
     |                 ~^   ~~
     |                  |   |
     |                  |   int
     |                  char*
     |                 %d

gcc/cp/ChangeLog:
	PR c++/56856
	* call.c (build_over_call): Build a vec of locations of the
	arguments before the call to maybe_constant_value, and pass to
	check_function_arguments.
	* cp-lang.c (LANG_HOOKS_GET_SUBSTRING_LOCATION): Define as
	c_get_substring_location.
	* parser.c (cp_parser_string_literal): Capture string
	concatenation locations.

gcc/ChangeLog:
	PR c++/56856
	* input.c (expand_location_to_spelling_point): Add param "aspect"
	and use rather than hardcoding LOCATION_ASPECT_CARET.
	(get_substring_ranges_for_loc): Handle the case of a single token
	within a macro expansion.
	* input.h (expand_location_to_spelling_point): Add "aspect" param,
	defaulting to LOCATION_ASPECT_CARET.

gcc/testsuite/ChangeLog:
	PR c++/56856
	* g++.dg/ext/builtin4.C: Set expected location for warning to the
	correct location within the format string.
	* g++.dg/plugin/plugin.exp (plugin_test_list): Add the plugin and
	files for testing locations within string literal locations from
	the C frontend.
	* g++.dg/warn/Wformat-method.C: New test.
	* g++.dg/warn/Wformat-pr71863.C: New test.
	* g++.dg/warn/Wformat-ranges-c++11.C: New test.
	* g++.dg/warn/Wformat-ranges.C: New test, based on
	gcc.dg/format/diagnostic-ranges.c.
	* gcc.dg/plugin/diagnostic-test-string-literals-1.c
	(test_multitoken_macro): Generalize expected output to work with
	both C and C++.
	* gcc.dg/plugin/diagnostic-test-string-literals-2.c
	(test_stringified_token_1): Likewise.
	(test_stringified_token_3): Likewise.


Added:
    trunk/gcc/testsuite/g++.dg/warn/Wformat-method.C
    trunk/gcc/testsuite/g++.dg/warn/Wformat-pr71863.C
    trunk/gcc/testsuite/g++.dg/warn/Wformat-ranges-c++11.C
    trunk/gcc/testsuite/g++.dg/warn/Wformat-ranges.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/call.c
    trunk/gcc/cp/cp-lang.c
    trunk/gcc/cp/parser.c
    trunk/gcc/input.c
    trunk/gcc/input.h
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/g++.dg/ext/builtin4.C
    trunk/gcc/testsuite/g++.dg/plugin/plugin.exp
    trunk/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c
    trunk/gcc/testsuite/gcc.dg/plugin/diagnostic-test-string-literals-1.c
    trunk/gcc/testsuite/gcc.dg/plugin/diagnostic-test-string-literals-2.c

Should be fixed for gcc 9 by r264887.

Author: dmalcolm
Date: Mon Oct 29 23:44:10 2018
New Revision: 265609

URL: https://gcc.gnu.org/viewcvs?rev=265609&root=gcc&view=rev
Log:
Folding and check_function_arguments

This patch eliminates the arglocs array I introduced to build_over_call
in r264887, and eliminates the call to maybe_constant_value when building
"fargs" (thus retaining location wrapper nodes).

Instead, this patch requires that any checks within
check_function_arguments that need folded arguments do their own folding.

Of the various checks:
(a) check_function_nonnull already calls fold_for_warn,
(b) check_function_format doesn't need folding
(c) check_function_sentinel needs fold_for_warn in one place, which the
patch adds, and
(d) check_function_restrict needs per-argument folding, which the patch
adds.  Given that it scans before and after resetting TREE_VISITED on
each argument, it seemed best to make a copy of the array, folding each
argument from the outset, rather than repeatedly calling fold_for_warn;

gcc/c-family/ChangeLog:
	PR c++/56856
	* c-common.c (check_function_sentinel): Call fold_for_warn on the
	argument.
	(check_function_restrict): Rename param "argarray" to
	"unfolded_argarray", and make a copy named "argarray", calling
	fold_for_warn on each argument.
	(check_function_arguments): Add note about responsibility for
	folding the arguments.

gcc/cp/ChangeLog:
	PR c++/56856
	* call.c (build_over_call): Eliminate the "arglocs" array, and the
	call to maybe_constant_value when building "fargs".


Modified:
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c-common.c
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/call.c