[Bug c++/97197] New: With -O2, Incorrect -Werror=maybe-uninitialized thrown, leads to 'target_mem_ref' and 'dump_expr<expression error>' in message

[Bug c++/97197] New: With -O2, Incorrect -Werror=maybe-uninitialized thrown, leads to 'target_mem_ref' and 'dump_expr<expression error>' in message

[davidfink314 at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97197

            Bug ID: 97197
           Summary: With -O2, Incorrect -Werror=maybe-uninitialized
                    thrown, leads to 'target_mem_ref' and
                    'dump_expr<expression error>' in message
           Product: gcc
           Version: 10.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: davidfink314 at gmail dot com
  Target Milestone: ---

Bug looks like it was introduced in gcc 7
reproduces in gcc trunk



Output:

x86-64 gcc (trunk)
-O2 -Werror=maybe-uninitialized
1
<Compilation failed>
x86-64 gcc (trunk) - cached
In copy constructor 'Normal::Normal(const Normal&)',
    inlined from 'Combo::Combo(const Combo&)' at <source>:21:8,
    inlined from 'void Y::f()' at <source>:36:24:
<source>:18:35: error: ''target_mem_ref' not supported by dump_expr<expression
error>' may be used uninitialized [-Werror=maybe-uninitialized]
   18 |     Normal(Normal const& o) : p(o.p /* warning here for no good reason
*/) { }
      |                                 ~~^
<source>: In member function 'void Y::f()':
<source>:36:16: note: 'combo' declared here
   36 |     for (Combo combo : comboList) { /* implicit copy required to
reproduce bug */
      |                ^~~~~
cc1plus: some warnings being treated as errors
Compiler returned: 1



Input:

https://godbolt.org/z/b73cY8

-O2 -Werror=maybe-uninitialized

#include <vector>

struct Weirdo {
    // nonstandard copy constructor
    bool data{false}; // initializer here required to reproduce bug
    Weirdo() = default;
    Weirdo(const Weirdo& o) {
        // data should still be set to false via {} above and not being listed
        // original code had this in the assignment operator, and the copy
constructor called the assignment operator.
        if (this != &o) { // pointer check required to reproduce bug
            data = o.data;
        }
    }
};

struct Normal {
    bool p{false};
    Normal() = default;
    Normal(Normal const& o) : p(o.p /* warning here for no good reason */) { }
};

struct Combo {
    Combo() : v(), q() {}
    void g();

    Weirdo v;
    Normal q;
};

struct Y {
    void f();

    std::vector<Combo> comboList;
};

void Y::f() {
    for (Combo combo : comboList) { /* implicit copy required to reproduce bug
*/
        combo.v.data && (combo.g(), false);
    }
}

[Bug c++/97197] With -O2, Incorrect -Werror=maybe-uninitialized thrown, leads to 'target_mem_ref' and 'dump_expr<expression error>' in message

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97197

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2020-09-25
                 CC|                            |marxin at gcc dot gnu.org,
                   |                            |rguenth at gcc dot gnu.org

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
Started with r7-6972-ge80facb4afaaa4d4.

[Bug c++/97197] With -O2, Incorrect -Werror=maybe-uninitialized thrown, leads to 'target_mem_ref' and 'dump_expr<expression error>' in message

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97197

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Patch posted.

[Bug c++/97197] With -O2, Incorrect -Werror=maybe-uninitialized thrown, leads to 'target_mem_ref' and 'dump_expr<expression error>' in message

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97197

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The this != &o check makes no sense, invoking a copy constructor on itself is
just wrong and if the compiler isn't able to optimize that bogus check early
enough, that is the reason why you get the warning.  Just don't do that.
The compiler bug is the lack of TARGET_MEM_REF support in the diagnostic of
course.

[Bug c++/97197] With -O2, Incorrect -Werror=maybe-uninitialized thrown, leads to 'target_mem_ref' and 'dump_expr<expression error>' in message

[davidfink314 at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97197

--- Comment #4 from David Fink <davidfink314 at gmail dot com> ---
Hi Jakub,

I tried to reduce the code as much as possible before reporting, which is why
the example has the "this != &o" check in the copy constructor.

The original code where I saw this problem had the "this != &o" check in the
assignment operator, and the copy constructor called the assignment operator.
In the reported version, I manually inlined the assignment operator.

I think the original code is also silly, but not as clearly wrong as the
pointer check in the copy constructor.


Note that the warning happens on a completely different class "Normal" from the
silly code "Weirdo", so I'd consider that a bug in addition to the diagnostic
containing 'target_mem_ref' and 'dump_expr<expression error>'.

[Bug c++/97197] With -O2, Incorrect -Werror=maybe-uninitialized thrown, leads to 'target_mem_ref' and 'dump_expr<expression error>' in message

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97197

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:ac1c65ad1a16d83ec63674efa07c00b062562f15

commit r11-3659-gac1c65ad1a16d83ec63674efa07c00b062562f15
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Mon Oct 5 18:33:17 2020 +0200

    support TARGET_MEM_REF in C/C++ error pretty-printing [PR97197]

    > See my comment above for Martins attempts to improve things.  I don't
    > really want to try decide what to do with those late diagnostic IL
    > printing but my commit was blamed for showing target-mem-ref unsupported.
    >
    > I don't have much time to spend to think what to best print and what not,
    > but yes, printing only the MEM_REF part is certainly imprecise.

    Here is an updated version of the patch that prints TARGET_MEM_REF the way
    it should be printed - as C representation of what it actually means.
    Of course it would be better to have the original expressions, but with the
    late diagnostics we no longer have them.

    2020-10-05  Richard Biener  <rguenther@suse.de>
                Jakub Jelinek  <jakub@redhat.com>

            PR c++/97197
    gcc/cp/
            * error.c (dump_expr): Handle TARGET_MEM_REF.
    gcc/c-family/
            * c-pretty-print.c: Include langhooks.h.
            (c_pretty_printer::postfix_expression): Handle TARGET_MEM_REF as
            expression.
            (c_pretty_printer::expression): Handle TARGET_MEM_REF as
            unary_expression.
            (c_pretty_printer::unary_expression): Handle TARGET_MEM_REF.

[Bug c++/97197] With -O2, Incorrect -Werror=maybe-uninitialized thrown, leads to 'target_mem_ref' and 'dump_expr<expression error>' in message

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97197

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:7790a71c07029546110a039f47611dfa967adc51

commit r10-8900-g7790a71c07029546110a039f47611dfa967adc51
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Mon Oct 5 18:33:17 2020 +0200

    support TARGET_MEM_REF in C/C++ error pretty-printing [PR97197]

    > See my comment above for Martins attempts to improve things.  I don't
    > really want to try decide what to do with those late diagnostic IL
    > printing but my commit was blamed for showing target-mem-ref unsupported.
    >
    > I don't have much time to spend to think what to best print and what not,
    > but yes, printing only the MEM_REF part is certainly imprecise.

    Here is an updated version of the patch that prints TARGET_MEM_REF the way
    it should be printed - as C representation of what it actually means.
    Of course it would be better to have the original expressions, but with the
    late diagnostics we no longer have them.

    2020-10-05  Richard Biener  <rguenther@suse.de>
                Jakub Jelinek  <jakub@redhat.com>

            PR c++/97197
    gcc/cp/
            * error.c (dump_expr): Handle TARGET_MEM_REF.
    gcc/c-family/
            * c-pretty-print.c: Include langhooks.h.
            (c_pretty_printer::postfix_expression): Handle TARGET_MEM_REF as
            expression.
            (c_pretty_printer::expression): Handle TARGET_MEM_REF as
            unary_expression.
            (c_pretty_printer::unary_expression): Handle TARGET_MEM_REF.

    (cherry picked from commit ac1c65ad1a16d83ec63674efa07c00b062562f15)

[Bug c++/97197] With -O2, Incorrect -Werror=maybe-uninitialized thrown, leads to 'target_mem_ref' and 'dump_expr<expression error>' in message

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97197

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.0
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED
                 CC|                            |msebor at gcc dot gnu.org

--- Comment #7 from Martin Sebor <msebor at gcc dot gnu.org> ---
For the test case in comment #0 GCC 11 issues the following so this works as
expected (GCC 10 output is similar).

In copy constructor ‘Normal::Normal(const Normal&)’,
    inlined from ‘Combo::Combo(const Combo&)’ at pr97197.C:23:8,
    inlined from ‘void Y::f()’ at pr97197.C:38:24:
pr97197.C:20:35: warning: ‘*((const bool*)& combo +1)’ may be used
uninitialized [-Wmaybe-uninitialized]
   20 |     Normal(Normal const& o) : p(o.p /* warning here for no good reason
*/) { }
      |                                 ~~^
pr97197.C: In member function ‘void Y::f()’:
pr97197.C:38:16: note: ‘combo’ declared here
   38 |     for (Combo combo : comboList) { /* implicit copy required to
reproduce bug */
      |                ^~~~~