[Bug c++/99565] New: Bogus identical branches warning when returning references to union members

[Bug c++/99565] New: Bogus identical branches warning when returning references to union members

[johelegp at gmail dot com]

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

            Bug ID: 99565
           Summary: Bogus identical branches warning when returning
                    references to union members
           Product: gcc
           Version: 11.0
               URL: https://godbolt.org/z/9qaz9W
            Status: UNCONFIRMED
          Keywords: diagnostic
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: johelegp at gmail dot com
                CC: johelegp at gmail dot com
  Target Milestone: ---

See https://godbolt.org/z/9qaz9W.
```C++
struct A {
  union {
    int a;
    int b;
  };
  int& x() { return 0 ? a : b; }
};
```

[Bug c++/99565] Bogus identical branches warning when returning references to union members

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2021-03-15
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |WAITING

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Not sure what you are refering to.

[Bug c++/99565] Bogus identical branches warning when returning references to union members

[johelegp at gmail dot com]

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

--- Comment #2 from Johel Ernesto Guerrero Peña <johelegp at gmail dot com> ---
```
<source>: In member function 'int& A::x()':
<source>:6:23: warning: this condition has identical branches
[-Wduplicated-branches]
    6 |   int& x() { return 0 ? a : b; }
      |                     ~~^~~~~~~
```

My actual use case looks more like this: https://godbolt.org/z/W5ajeM.
```C++
struct A {
  char x;
  long long y;
};
struct B {
  double x;
  long long y;
};
struct C {
  int i;
  union {
    A a;
    B b;
  };
  long long& y() { return i ? a.y : b.y; }
};
```
```
<source>: In member function 'long long int& C::y()':
<source>:15:29: warning: this condition has identical branches
[-Wduplicated-branches]
   15 |   long long& y() { return i ? a.y : b.y; }
      |                           ~~^~~~~~~~~~~
```

[Bug c++/99565] [11 Regression] Bogus identical branches warning when returning references to union members

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |NEW
                 CC|                            |hubicka at gcc dot gnu.org,
                   |                            |mpolacek at gcc dot gnu.org
           Priority|P3                          |P1
   Target Milestone|---                         |11.0
            Summary|Bogus identical branches    |[11 Regression] Bogus
                   |warning when returning      |identical branches warning
                   |references to union members |when returning references
                   |                            |to union members

--- Comment #3 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Confirmed then.  It's actually an 11 regression, started with r11-5181.

[Bug c++/99565] [11 Regression] Bogus identical branches warning when returning references to union members

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 50398
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50398&action=edit
gcc11-pr99565.patch

If we don't want to warn in this case (I guess pedantically in both C and C++
it matters which exact union member is used even when it has the same type,
because only one of them can be active, but practically the compiler will treat
them the same anyway and so they are effectively the same), perhaps we could
revert Honza's change for OEP_LEXICOGRAPHIC and use that mode for the COND_EXPR
warnings (where previously it was used just for then/else variant).
But, apparently then we warn for some reason twice on:
int a;

void
foo (bool x)
{
  x ? ++a : ++a;
}
instead of once.  Or add some new OEP_ flag that would be used for
-Wduplicated-branches?

[Bug c++/99565] [11 Regression] Bogus identical branches warning when returning references to union members

[mpolacek at gcc dot gnu.org]

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

--- Comment #5 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
I think I added OEP_LEXICOGRAPHIC specifically for -Wduplicated-branches
(do_warn_duplicated_branches used it first), so we can have it do whatever we
want for the warning.  So your change looks fine to me.

[Bug c++/99565] [11 Regression] Bogus identical branches warning when returning references to union members

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #50398|0                           |1
        is obsolete|                            |
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 50400
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50400&action=edit
gcc11-pr99565.patch

Better patch that doesn't regress the above mentioned testcase.

[Bug c++/99565] [11 Regression] Bogus identical branches warning when returning references to union members

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 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:660eb7e9dee46ef1c986d5a4fa5cbd182b435518

commit r11-7826-g660eb7e9dee46ef1c986d5a4fa5cbd182b435518
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Mar 25 11:33:35 2021 +0100

    c-family: Fix up -Wduplicated-branches for union members [PR99565]

    Honza has fairly recently changed operand_equal_p to compare
    DECL_FIELD_OFFSET for COMPONENT_REFs when comparing addresses.
    As the first testcase in this patch shows, while that is very nice
    for optimizations, for the -Wduplicated-branches warning it causes
    regressions.  Pedantically a union in both C and C++ has only one
    active member at a time, so using some other union member even if it has
the
    same type is UB, so I think the warning shouldn't warn when it sees access
    to different fields that happen to have the same offset and should consider
    them different.
    In my first attempt to fix this I've keyed the old behavior on
    OEP_LEXICOGRAPHIC, but unfortunately that has various problems, the warning
    has a quick non-lexicographic compare in build_conditional_expr* and
another
    lexicographic more expensive one later during genericization and turning
the
    first one into lexicographic would mean wasting compile time on large
    conditionals.
    So, this patch instead introduces a new OEP_ flag and makes sure to pass it
    to operand_equal_p in all -Wduplicated-branches cases.

    The cvt.c changes are because on the other testcase we were warning with
    UNKNOWN_LOCATION, so the user wouldn't really know where the questionable
    code is.

    2021-03-25  Jakub Jelinek  <jakub@redhat.com>

            PR c++/99565
            * tree-core.h (enum operand_equal_flag): Add
OEP_ADDRESS_OF_SAME_FIELD.
            * fold-const.c (operand_compare::operand_equal_p): Don't compare
            field offsets if OEP_ADDRESS_OF_SAME_FIELD.

            * c-warn.c (do_warn_duplicated_branches): Pass also
            OEP_ADDRESS_OF_SAME_FIELD to operand_equal_p.

            * c-typeck.c (build_conditional_expr): Pass
OEP_ADDRESS_OF_SAME_FIELD
            to operand_equal_p.

            * call.c (build_conditional_expr_1): Pass OEP_ADDRESS_OF_SAME_FIELD
            to operand_equal_p.
            * cvt.c (convert_to_void): Preserve location_t on COND_EXPR or
            or COMPOUND_EXPR.

            * g++.dg/warn/Wduplicated-branches6.C: New test.
            * g++.dg/warn/Wduplicated-branches7.C: New test.

[Bug c++/99565] [11 Regression] Bogus identical branches warning when returning references to union members

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.