On Mon, 2018-10-08 at 10:37 -0400, Jason Merrill wrote:
> On Thu, Oct 4, 2018 at 10:12 AM David Malcolm <dmalc...@redhat.com>
> wrote:
> >
> > -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.
>
> Maybe we should remove that in favor of fold_for_warn in
> check_function_arguments.
>
> Jason
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;
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
OK for trunk?
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".
---
gcc/c-family/c-common.c | 17 +++++++++++++----
gcc/cp/call.c | 6 ++----
2 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index c0198e1..11fa360 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -5297,7 +5297,7 @@ check_function_sentinel (const_tree fntype, int nargs,
tree *argarray)
}
/* Validate the sentinel. */
- sentinel = argarray[nargs - 1 - pos];
+ sentinel = fold_for_warn (argarray[nargs - 1 - pos]);
if ((!POINTER_TYPE_P (TREE_TYPE (sentinel))
|| !integer_zerop (sentinel))
/* Although __null (in C++) is only an integer we allow it
@@ -5316,11 +5316,16 @@ check_function_sentinel (const_tree fntype, int nargs,
tree *argarray)
static bool
check_function_restrict (const_tree fndecl, const_tree fntype,
- int nargs, tree *argarray)
+ int nargs, tree *unfolded_argarray)
{
int i;
tree parms = TYPE_ARG_TYPES (fntype);
+ /* Call fold_for_warn on all of the arguments. */
+ auto_vec<tree> argarray (nargs);
+ for (i = 0; i < nargs; i++)
+ argarray.quick_push (fold_for_warn (unfolded_argarray[i]));
+
if (fndecl
&& TREE_CODE (fndecl) == FUNCTION_DECL)
{
@@ -5357,7 +5362,7 @@ check_function_restrict (const_tree fndecl, const_tree
fntype,
if (POINTER_TYPE_P (type)
&& TYPE_RESTRICT (type)
&& !TYPE_READONLY (TREE_TYPE (type)))
- warned |= warn_for_restrict (i, argarray, nargs);
+ warned |= warn_for_restrict (i, argarray.address (), nargs);
}
for (i = 0; i < nargs; i++)
@@ -5604,7 +5609,11 @@ attribute_fallthrough_p (tree attr)
/* Check for valid arguments being passed to a function with FNTYPE.
There are NARGS arguments in the array ARGARRAY. LOC should be used
for diagnostics. Return true if either -Wnonnull or -Wrestrict has
- been issued. */
+ been issued.
+
+ The arguments in ARGARRAY may not have been folded yet (e.g. for C++,
+ to preserve location wrappers); checks that require folded arguments
+ should call fold_for_warn on them. */
bool
check_function_arguments (location_t loc, const_tree fndecl, const_tree fntype,
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 747f837..51da771 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -8188,7 +8188,6 @@ build_over_call (struct z_candidate *cand, int flags,
tsubst_flags_t complain)
{
tree *fargs = (!nargs ? argarray
: (tree *) alloca (nargs * sizeof (tree)));
- auto_vec<location_t> arglocs (nargs);
for (j = 0; j < nargs; j++)
{
/* For -Wformat undo the implicit passing by hidden reference
@@ -8197,12 +8196,11 @@ build_over_call (struct z_candidate *cand, int flags,
tsubst_flags_t complain)
&& TYPE_REF_P (TREE_TYPE (argarray[j])))
fargs[j] = TREE_OPERAND (argarray[j], 0);
else
- fargs[j] = maybe_constant_value (argarray[j]);
- arglocs.quick_push (EXPR_LOC_OR_LOC (argarray[j], input_location));
+ fargs[j] = argarray[j];
}
warned_p = check_function_arguments (input_location, fn, TREE_TYPE (fn),
- nargs, fargs, &arglocs);
+ nargs, fargs, NULL);
}
if (DECL_INHERITED_CTOR (fn))
--
1.8.5.3
