On Thu, Mar 08, 2018 at 04:20:40PM -0500, Jason Merrill wrote:
> On Thu, Mar 8, 2018 at 1:08 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> > The reporter complains that -Wformat incorrectly reports std::string* passed
> > to "%s" format rather than std::string, which is what the user did.
> >
> > This transformation of non-trivial copy init or dtor classes in ellipsis is
> > done by convert_arg_to_ellipsis; that function does many changes and all
> > but this one look desirable for the -Wnonnull/-Wformat/-Wsentinel/-Wrestrict
> > warnings. We prepare a special argument vector in any case, so this patch
> > just arranges to undo what convert_arg_to_ellipsis did for the classes.
> > I think -Wnonnull shouldn't care, because when passing such a class by
> > value, it will be non-NULL (and -Wnonnull looks only for literal NULLs
> > anyway), -Wrestrict only cares about named arguments and -Wsentinel only
> > cares about NULL arguments passed to ellipsis.
>
> I notice that convert_arg_to_ellipsis generates a pointer-typed
> expression, whereas convert_for_arg_passing generates a reference.
> Does correcting that inconsistency help?
No (though I think it is a good idea anyway).
With this patch the original testcase reports:
pr84076.C:7:16: warning: format ‘%s’ expects argument of type ‘char*’, but
argument 2 has type ‘std::__cxx11::string&’ {aka
‘std::__cxx11::basic_string<char>&’} [-Wformat=]
where the user expects that without the & characters and without the patch
it has * instead of &.
With the patch I've posted it is:
pr84076.C:7:16: warning: format ‘%s’ expects argument of type ‘char*’, but
argument 2 has type ‘std::__cxx11::string’ {aka
‘std::__cxx11::basic_string<char>’} [-Wformat=]
Similarly, the testcase included in the patch with just your patch:
pr84076.C: In function ‘void foo()’:
pr84076.C:13:21: warning: format ‘%s’ expects argument of type ‘char*’, but
argument 2 has type ‘S&’ [-Wformat=]
__builtin_printf ("%s\n", s); // { dg-warning "format '%s' expects argument
of type 'char\\*', but argument 2 has type 'S'" }
^~~~~~
pr84076.C:14:21: warning: format ‘%s’ expects argument of type ‘char*’, but
argument 2 has type ‘T&’ [-Wformat=]
__builtin_printf ("%s\n", t); // { dg-warning "format '%s' expects argument
of type 'char\\*', but argument 2 has type 'T'" }
^~~~~~
pr84076.C:15:21: warning: format ‘%s’ expects argument of type ‘char*’, but
argument 2 has type ‘S*’ [-Wformat=]
__builtin_printf ("%s\n", &s);// { dg-warning "format '%s' expects argument
of type 'char\\*', but argument 2 has type 'S\\*'" }
^~~~~~ ~~
pr84076.C:16:21: warning: format ‘%s’ expects argument of type ‘char*’, but
argument 2 has type ‘T*’ [-Wformat=]
__builtin_printf ("%s\n", &t);// { dg-warning "format '%s' expects argument
of type 'char\\*', but argument 2 has type 'T\\*'" }
^~~~~~ ~~
and with both patches:
pr84076.C: In function ‘void foo()’:
pr84076.C:13:21: warning: format ‘%s’ expects argument of type ‘char*’, but
argument 2 has type ‘S’ [-Wformat=]
__builtin_printf ("%s\n", s); // { dg-warning "format '%s' expects argument
of type 'char\\*', but argument 2 has type 'S'" }
^~~~~~ ~
pr84076.C:14:21: warning: format ‘%s’ expects argument of type ‘char*’, but
argument 2 has type ‘T’ [-Wformat=]
__builtin_printf ("%s\n", t); // { dg-warning "format '%s' expects argument
of type 'char\\*', but argument 2 has type 'T'" }
^~~~~~ ~
pr84076.C:15:21: warning: format ‘%s’ expects argument of type ‘char*’, but
argument 2 has type ‘S*’ [-Wformat=]
__builtin_printf ("%s\n", &s);// { dg-warning "format '%s' expects argument
of type 'char\\*', but argument 2 has type 'S\\*'" }
^~~~~~ ~~
pr84076.C:16:21: warning: format ‘%s’ expects argument of type ‘char*’, but
argument 2 has type ‘T*’ [-Wformat=]
__builtin_printf ("%s\n", &t);// { dg-warning "format '%s' expects argument
of type 'char\\*', but argument 2 has type 'T\\*'" }
^~~~~~ ~~
> diff --git a/gcc/cp/call.c b/gcc/cp/call.c
> index f83d51f3457..617ce18f8cd 100644
> --- a/gcc/cp/call.c
> +++ b/gcc/cp/call.c
> @@ -7209,7 +7209,7 @@ convert_arg_to_ellipsis (tree arg, tsubst_flags_t
> complain)
> "passing objects of non-trivially-copyable "
> "type %q#T through %<...%> is conditionally supported",
> arg_type);
> - return cp_build_addr_expr (arg, complain);
> + return build1 (ADDR_EXPR, build_reference_type (arg_type), arg);
> }
> /* Build up a real lvalue-to-rvalue conversion in case the
> copy constructor is trivial but not callable. */
Jakub
