[C++] Avoid bogus -Wunused-but-set* with structured binding (PR c++/85952)

Hi!

Initializing the decomposition temporary from an expression with array type
is a special aggregate initialization path in which we wouldn't mark the
expression as read for the purposes of -Wunused-but-set*.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

2018-05-29  Jakub Jelinek  <jakub@redhat.com>

	PR c++/85952
	* init.c (build_aggr_init): For structured binding initialized from
	array call mark_rvalue_use on the initializer.

	* g++.dg/warn/Wunused-var-33.C: New test.


	Jakub

On Tue, May 29, 2018, 4:31 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> Initializing the decomposition temporary from an expression with array type
> is a special aggregate initialization path in which we wouldn't mark the
> expression as read for the purposes of -Wunused-but-set*.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
>
> 2018-05-29  Jakub Jelinek  <jakub@redhat.com>
>
>         PR c++/85952
>         * init.c (build_aggr_init): For structured binding initialized from
>         array call mark_rvalue_use on the initializer.
>
>         * g++.dg/warn/Wunused-var-33.C: New test.
>
> --- gcc/cp/init.c.jj    2018-05-25 14:34:41.000000000 +0200
> +++ gcc/cp/init.c       2018-05-28 19:04:10.504063972 +0200
> @@ -1678,6 +1678,7 @@ build_aggr_init (tree exp, tree init, in
>        if (VAR_P (exp) && DECL_DECOMPOSITION_P (exp))
>         {
>           from_array = 1;
> +         init = mark_rvalue_use (init);

This should be mark_lvalue_use, since the structured bindings refer to
the elements of the array rather than copying them.  OK with that
change.

Jason

On Tue, May 29, 2018 at 10:16:49AM -0400, Jason Merrill wrote:
> On Tue, May 29, 2018, 4:31 AM Jakub Jelinek <jakub@redhat.com> wrote:
> > Initializing the decomposition temporary from an expression with array type
> > is a special aggregate initialization path in which we wouldn't mark the
> > expression as read for the purposes of -Wunused-but-set*.
> >
> > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> > trunk?
> >
> > 2018-05-29  Jakub Jelinek  <jakub@redhat.com>
> >
> >         PR c++/85952
> >         * init.c (build_aggr_init): For structured binding initialized from
> >         array call mark_rvalue_use on the initializer.
> >
> >         * g++.dg/warn/Wunused-var-33.C: New test.
> >
> > --- gcc/cp/init.c.jj    2018-05-25 14:34:41.000000000 +0200
> > +++ gcc/cp/init.c       2018-05-28 19:04:10.504063972 +0200
> > @@ -1678,6 +1678,7 @@ build_aggr_init (tree exp, tree init, in
> >        if (VAR_P (exp) && DECL_DECOMPOSITION_P (exp))
> >         {
> >           from_array = 1;
> > +         init = mark_rvalue_use (init);
> 
> This should be mark_lvalue_use, since the structured bindings refer to
> the elements of the array rather than copying them.  OK with that
> change.

I think they refer to the elements of the decomposition variable (i.e. exp).

"If the assignment-expression in the initializer has array type A and no
ref-qualifier is present, e has type cv A and each element is copy-initialized or direct-initialized from the
corresponding element of the assignment-expression as specified by the form of the
initializer."

is what applies in this case, and

  int a[2] = {1, 2};
  int D.2131[2] = a;
  int x [value-expr: D.2131[0]];
  int y [value-expr: D.2131[1]];

  <<cleanup_point   int a[2] = {1, 2};>>;
    int D.2131[2] = a;
  return <retval> = x + y;

is what original dump shows as implemented, so I don't see a being used here
as an lvalue, we copy the elements into the temporary and that is all where
it is referenced.

	Jakub

On Tue, May 29, 2018 at 10:26 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, May 29, 2018 at 10:16:49AM -0400, Jason Merrill wrote:
>> On Tue, May 29, 2018, 4:31 AM Jakub Jelinek <jakub@redhat.com> wrote:
>> > Initializing the decomposition temporary from an expression with array type
>> > is a special aggregate initialization path in which we wouldn't mark the
>> > expression as read for the purposes of -Wunused-but-set*.
>> >
>> > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
>> > trunk?
>> >
>> > 2018-05-29  Jakub Jelinek  <jakub@redhat.com>
>> >
>> >         PR c++/85952
>> >         * init.c (build_aggr_init): For structured binding initialized from
>> >         array call mark_rvalue_use on the initializer.
>> >
>> >         * g++.dg/warn/Wunused-var-33.C: New test.
>> >
>> > --- gcc/cp/init.c.jj    2018-05-25 14:34:41.000000000 +0200
>> > +++ gcc/cp/init.c       2018-05-28 19:04:10.504063972 +0200
>> > @@ -1678,6 +1678,7 @@ build_aggr_init (tree exp, tree init, in
>> >        if (VAR_P (exp) && DECL_DECOMPOSITION_P (exp))
>> >         {
>> >           from_array = 1;
>> > +         init = mark_rvalue_use (init);
>>
>> This should be mark_lvalue_use, since the structured bindings refer to
>> the elements of the array rather than copying them.  OK with that
>> change.
>
> I think they refer to the elements of the decomposition variable (i.e. exp).
>
> "If the assignment-expression in the initializer has array type A and no
> ref-qualifier is present, e has type cv A and each element is copy-initialized or direct-initialized from the
> corresponding element of the assignment-expression as specified by the form of the
> initializer."
>
> is what applies in this case, and
>
>   int a[2] = {1, 2};
>   int D.2131[2] = a;
>   int x [value-expr: D.2131[0]];
>   int y [value-expr: D.2131[1]];
>
>   <<cleanup_point   int a[2] = {1, 2};>>;
>     int D.2131[2] = a;
>   return <retval> = x + y;
>
> is what original dump shows as implemented, so I don't see a being used here
> as an lvalue, we copy the elements into the temporary and that is all where
> it is referenced.

Ah, no, you're right for foo, where the structured binding declaration
is not a reference.  And it looks like we shouldn't hit this path for

  auto & [x,y] = a;

but that should be added to the testcase.

Jason

On Tue, May 29, 2018 at 10:38:09AM -0400, Jason Merrill wrote:
> >   int a[2] = {1, 2};
> >   int D.2131[2] = a;
> >   int x [value-expr: D.2131[0]];
> >   int y [value-expr: D.2131[1]];
> >
> >   <<cleanup_point   int a[2] = {1, 2};>>;
> >     int D.2131[2] = a;
> >   return <retval> = x + y;
> >
> > is what original dump shows as implemented, so I don't see a being used here
> > as an lvalue, we copy the elements into the temporary and that is all where
> > it is referenced.
> 
> Ah, no, you're right for foo, where the structured binding declaration
> is not a reference.  And it looks like we shouldn't hit this path for
> 
>   auto & [x,y] = a;
> 
> but that should be added to the testcase.

It is already there (in baz).

	Jakub

On Tue, May 29, 2018 at 10:42 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, May 29, 2018 at 10:38:09AM -0400, Jason Merrill wrote:
>> >   int a[2] = {1, 2};
>> >   int D.2131[2] = a;
>> >   int x [value-expr: D.2131[0]];
>> >   int y [value-expr: D.2131[1]];
>> >
>> >   <<cleanup_point   int a[2] = {1, 2};>>;
>> >     int D.2131[2] = a;
>> >   return <retval> = x + y;
>> >
>> > is what original dump shows as implemented, so I don't see a being used here
>> > as an lvalue, we copy the elements into the temporary and that is all where
>> > it is referenced.
>>
>> Ah, no, you're right for foo, where the structured binding declaration
>> is not a reference.  And it looks like we shouldn't hit this path for
>>
>>   auto & [x,y] = a;
>>
>> but that should be added to the testcase.
>
> It is already there (in baz).

Well, yes, but in baz a is an S, not an array; I see value and
reference cases for S, but only value for int array.

Jason

On Tue, May 29, 2018 at 10:49:03AM -0400, Jason Merrill wrote:
> >>   auto & [x,y] = a;
> >>
> >> but that should be added to the testcase.
> >
> > It is already there (in baz).
> 
> Well, yes, but in baz a is an S, not an array; I see value and
> reference cases for S, but only value for int array.

Oops, yes, you're right.  Added qux then that tests array initializer
and & qualifier.

Ok with that change?

2018-05-29  Jakub Jelinek  <jakub@redhat.com>

	PR c++/85952
	* init.c (build_aggr_init): For structured binding initialized from
	array call mark_rvalue_use on the initializer.

	* g++.dg/warn/Wunused-var-33.C: New test.

--- gcc/cp/init.c.jj	2018-05-25 14:34:41.000000000 +0200
+++ gcc/cp/init.c	2018-05-28 19:04:10.504063972 +0200
@@ -1678,6 +1678,7 @@ build_aggr_init (tree exp, tree init, in
       if (VAR_P (exp) && DECL_DECOMPOSITION_P (exp))
 	{
 	  from_array = 1;
+	  init = mark_rvalue_use (init);
 	  if (init && DECL_P (init)
 	      && !(flags & LOOKUP_ONLYCONVERTING))
 	    {
--- gcc/testsuite/g++.dg/warn/Wunused-var-33.C.jj	2018-05-28 19:32:00.236440573 +0200
+++ gcc/testsuite/g++.dg/warn/Wunused-var-33.C	2018-05-29 16:52:21.322700629 +0200
@@ -0,0 +1,37 @@
+// PR c++/85952
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wunused-but-set-variable" }
+
+int
+foo ()
+{
+  int a[2] = {1, 2};	// { dg-bogus "set but not used" } */
+  auto [x, y] = a;	// { dg-warning "structured bindings only available with" "" { target c++14_down } }
+  return x + y;
+}
+
+struct S { int d, e; };
+
+int
+bar ()
+{
+  S a = {1, 2};
+  auto [x, y] = a;	// { dg-warning "structured bindings only available with" "" { target c++14_down } }
+  return x + y;
+}
+
+int
+baz ()
+{
+  S a = {1, 2};
+  auto & [x, y] = a;	// { dg-warning "structured bindings only available with" "" { target c++14_down } }
+  return x + y;
+}
+
+int
+qux ()
+{
+  int a[2] = {1, 2};
+  auto & [x, y] = a;	// { dg-warning "structured bindings only available with" "" { target c++14_down } }
+  return x + y;
+}

	Jakub

OK.

On Tue, May 29, 2018 at 10:55 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, May 29, 2018 at 10:49:03AM -0400, Jason Merrill wrote:
>> >>   auto & [x,y] = a;
>> >>
>> >> but that should be added to the testcase.
>> >
>> > It is already there (in baz).
>>
>> Well, yes, but in baz a is an S, not an array; I see value and
>> reference cases for S, but only value for int array.
>
> Oops, yes, you're right.  Added qux then that tests array initializer
> and & qualifier.
>
> Ok with that change?
>
> 2018-05-29  Jakub Jelinek  <jakub@redhat.com>
>
>         PR c++/85952
>         * init.c (build_aggr_init): For structured binding initialized from
>         array call mark_rvalue_use on the initializer.
>
>         * g++.dg/warn/Wunused-var-33.C: New test.
>
> --- gcc/cp/init.c.jj    2018-05-25 14:34:41.000000000 +0200
> +++ gcc/cp/init.c       2018-05-28 19:04:10.504063972 +0200
> @@ -1678,6 +1678,7 @@ build_aggr_init (tree exp, tree init, in
>        if (VAR_P (exp) && DECL_DECOMPOSITION_P (exp))
>         {
>           from_array = 1;
> +         init = mark_rvalue_use (init);
>           if (init && DECL_P (init)
>               && !(flags & LOOKUP_ONLYCONVERTING))
>             {
> --- gcc/testsuite/g++.dg/warn/Wunused-var-33.C.jj       2018-05-28 19:32:00.236440573 +0200
> +++ gcc/testsuite/g++.dg/warn/Wunused-var-33.C  2018-05-29 16:52:21.322700629 +0200
> @@ -0,0 +1,37 @@
> +// PR c++/85952
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-Wunused-but-set-variable" }
> +
> +int
> +foo ()
> +{
> +  int a[2] = {1, 2};   // { dg-bogus "set but not used" } */
> +  auto [x, y] = a;     // { dg-warning "structured bindings only available with" "" { target c++14_down } }
> +  return x + y;
> +}
> +
> +struct S { int d, e; };
> +
> +int
> +bar ()
> +{
> +  S a = {1, 2};
> +  auto [x, y] = a;     // { dg-warning "structured bindings only available with" "" { target c++14_down } }
> +  return x + y;
> +}
> +
> +int
> +baz ()
> +{
> +  S a = {1, 2};
> +  auto & [x, y] = a;   // { dg-warning "structured bindings only available with" "" { target c++14_down } }
> +  return x + y;
> +}
> +
> +int
> +qux ()
> +{
> +  int a[2] = {1, 2};
> +  auto & [x, y] = a;   // { dg-warning "structured bindings only available with" "" { target c++14_down } }
> +  return x + y;
> +}
>
>         Jakub