public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug c++/93480] Defaulted <=> doesn't expand array elements
       [not found] <bug-93480-4@http.gcc.gnu.org/bugzilla/>
@ 2020-12-18 19:28 ` wjwray at gmail dot com
  2020-12-19 22:17 ` wjwray at gmail dot com
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 10+ messages in thread
From: wjwray at gmail dot com @ 2020-12-18 19:28 UTC (permalink / raw)
  To: gcc-bugs

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

Will Wray <wjwray at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |wjwray at gmail dot com

--- Comment #1 from Will Wray <wjwray at gmail dot com> ---
I'm keen to see this fixed (and open to contribute)
(defaulted array comparison now works on Clang & MSVC).

Workaround for lack of compiler-generated array<=>array
is awkward and brittle: https://godbolt.org/z/xr668E

 * Preprocessor conditional compilation is required
   (at least there seems no way to detect array<=>array
    support and dispatch to a user-defined comparison
    only as needed - iff array members are present):

    # if ! defined(__GNUC__) || defined(__clang__)
        auto operator<=>(C const&) const = default;
    # else
        constexpr auto operator<=>(C const& r) const {
            return three_way_compare(x,r.x);
        }
    #endif

Then:
 * A generic 3-way comparison for array should be recursive.
 * Achieving efficient/vector codegen is not straightforward.
 * Deducing the return type is subtle:

    template <typename E, int N>
    constexpr auto three_way_compare(E const(&l)[N],
                                     E const(&r)[N]) {
        auto c = l[0] <=> r[0];
        for (int i = 0; ++i != N; c = l[i] <=> r[i])
            if (c != 0)
                return c;
        return c;
    }

So it'd be better for all if this were compiler-generated.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Bug c++/93480] Defaulted <=> doesn't expand array elements
       [not found] <bug-93480-4@http.gcc.gnu.org/bugzilla/>
  2020-12-18 19:28 ` [Bug c++/93480] Defaulted <=> doesn't expand array elements wjwray at gmail dot com
@ 2020-12-19 22:17 ` wjwray at gmail dot com
  2020-12-20 14:23 ` jakub at gcc dot gnu.org
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 10+ messages in thread
From: wjwray at gmail dot com @ 2020-12-19 22:17 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #2 from Will Wray <wjwray at gmail dot com> ---
For reference, here's a macro-free workaround to provide portable
operator<=> for templated classes with array members, defaulting
where possible (current Clang and MSVC) otherwise dispatching to
a user-defined implementation (current gcc)
https://godbolt.org/z/qbEfh7

First, a trait to check default 3-way array compare:

    inline constexpr bool has_default_array_compare = [] {
        struct C {
            char c[1];
            auto operator<=>(C const&) const = default;
        };
        return std::three_way_comparable<C>;
    }();

Then constrain the operator<=> definitions

    template <typename T, int N> struct array
    {
        T data[N];

        friend auto operator<=>(array const&, array const&)
            requires has_default_array_compare = default;

        friend constexpr auto operator<=>(array const& l,
                                          array const& r)
            requires (!has_default_array_compare)
        {
            return compare_three_way{}(l.data,r.data);
        }
    };

However, this only works for templated classes
as it is not (yet) allowed to constrain non-template functions.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Bug c++/93480] Defaulted <=> doesn't expand array elements
       [not found] <bug-93480-4@http.gcc.gnu.org/bugzilla/>
  2020-12-18 19:28 ` [Bug c++/93480] Defaulted <=> doesn't expand array elements wjwray at gmail dot com
  2020-12-19 22:17 ` wjwray at gmail dot com
@ 2020-12-20 14:23 ` jakub at gcc dot gnu.org
  2020-12-21 22:49 ` wjwray at gmail dot com
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 10+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-12-20 14:23 UTC (permalink / raw)
  To: gcc-bugs

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

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

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

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

Untested fix.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Bug c++/93480] Defaulted <=> doesn't expand array elements
       [not found] <bug-93480-4@http.gcc.gnu.org/bugzilla/>
                   ` (2 preceding siblings ...)
  2020-12-20 14:23 ` jakub at gcc dot gnu.org
@ 2020-12-21 22:49 ` wjwray at gmail dot com
  2020-12-22 19:19 ` cvs-commit at gcc dot gnu.org
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 10+ messages in thread
From: wjwray at gmail dot com @ 2020-12-21 22:49 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #4 from Will Wray <wjwray at gmail dot com> ---
Thanks Jakub;

I applied your patch to trunk and ran more test cases for
nested arrays (including zero-size in various positions),
union element type, base classes - all passed as expected.
I tried to grok the patch to look for problems - saw none.

In testing, a worse bug surfaced with defaulted operator==

The current bug was that defaulted <=> was deleted in the
presence of array members, so rel-ops fail to compile.

However, defaulted == is defined (on trunk) for array members
but appears to compare the pointer value of the decayed array
so == or != comparisons compile and give the wrong result
https://godbolt.org/z/Pbr9xr

Your patch fixes this bug also.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Bug c++/93480] Defaulted <=> doesn't expand array elements
       [not found] <bug-93480-4@http.gcc.gnu.org/bugzilla/>
                   ` (3 preceding siblings ...)
  2020-12-21 22:49 ` wjwray at gmail dot com
@ 2020-12-22 19:19 ` cvs-commit at gcc dot gnu.org
  2020-12-22 19:19 ` jakub at gcc dot gnu.org
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 10+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-12-22 19:19 UTC (permalink / raw)
  To: gcc-bugs

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

--- 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:ffd454b92ba6ff5499cf57f82a2b0f4cee59978c

commit r11-6305-gffd454b92ba6ff5499cf57f82a2b0f4cee59978c
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Dec 22 20:18:10 2020 +0100

    c++: Handle array members in build_comparison_op [PR93480]

    http://eel.is/c++draft/class.compare.default#6 says for the
    expanded list of subobjects:
    "In that list, any subobject of array type is recursively expanded
    to the sequence of its elements, in the order of increasing subscript."
    but build_comparison_op just tried to compare the whole arrays, which
    failed and therefore the defaulted comparison was deleted.

    The following patch instead compares the array elements, and
    if info.defining, adds runtime loops around it so that it iterates
    over increasing subscripts.

    For flexible array members it punts, we don't know how large those will be,
    for zero sized arrays it doesn't even try to compare the elements,
    because if there are no elements, there is nothing to compare, and
    for [1] arrays it will not emit a loop because it is enough to use
    [0] array ref to cover everything.

    2020-12-21  Jakub Jelinek  <jakub@redhat.com>

            PR c++/93480
            * method.c (common_comparison_type): If comps[i] is a TREE_LIST,
            use its TREE_VALUE instead.
            (build_comparison_op): Handle array members.

            * g++.dg/cpp2a/spaceship-synth10.C: New test.
            * g++.dg/cpp2a/spaceship-synth-neg5.C: New test.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Bug c++/93480] Defaulted <=> doesn't expand array elements
       [not found] <bug-93480-4@http.gcc.gnu.org/bugzilla/>
                   ` (4 preceding siblings ...)
  2020-12-22 19:19 ` cvs-commit at gcc dot gnu.org
@ 2020-12-22 19:19 ` jakub at gcc dot gnu.org
  2021-01-18 15:18 ` ppalka at gcc dot gnu.org
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 10+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-12-22 19:19 UTC (permalink / raw)
  To: gcc-bugs

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

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

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for GCC 11.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Bug c++/93480] Defaulted <=> doesn't expand array elements
       [not found] <bug-93480-4@http.gcc.gnu.org/bugzilla/>
                   ` (5 preceding siblings ...)
  2020-12-22 19:19 ` jakub at gcc dot gnu.org
@ 2021-01-18 15:18 ` ppalka at gcc dot gnu.org
  2021-04-22 19:32 ` ppalka at gcc dot gnu.org
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 10+ messages in thread
From: ppalka at gcc dot gnu.org @ 2021-01-18 15:18 UTC (permalink / raw)
  To: gcc-bugs

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

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |vermaelen.wouter at gmail dot com

--- Comment #7 from Patrick Palka <ppalka at gcc dot gnu.org> ---
*** Bug 95608 has been marked as a duplicate of this bug. ***

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Bug c++/93480] Defaulted <=> doesn't expand array elements
       [not found] <bug-93480-4@http.gcc.gnu.org/bugzilla/>
                   ` (6 preceding siblings ...)
  2021-01-18 15:18 ` ppalka at gcc dot gnu.org
@ 2021-04-22 19:32 ` ppalka at gcc dot gnu.org
  2022-12-26 17:50 ` pinskia at gcc dot gnu.org
  2022-12-26 17:51 ` rhalbersma at gmail dot com
  9 siblings, 0 replies; 10+ messages in thread
From: ppalka at gcc dot gnu.org @ 2021-04-22 19:32 UTC (permalink / raw)
  To: gcc-bugs

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

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rhalbersma at gmail dot com

--- Comment #8 from Patrick Palka <ppalka at gcc dot gnu.org> ---
*** Bug 94924 has been marked as a duplicate of this bug. ***

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Bug c++/93480] Defaulted <=> doesn't expand array elements
       [not found] <bug-93480-4@http.gcc.gnu.org/bugzilla/>
                   ` (7 preceding siblings ...)
  2021-04-22 19:32 ` ppalka at gcc dot gnu.org
@ 2022-12-26 17:50 ` pinskia at gcc dot gnu.org
  2022-12-26 17:51 ` rhalbersma at gmail dot com
  9 siblings, 0 replies; 10+ messages in thread
From: pinskia at gcc dot gnu.org @ 2022-12-26 17:50 UTC (permalink / raw)
  To: gcc-bugs

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.0

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Bug c++/93480] Defaulted <=> doesn't expand array elements
       [not found] <bug-93480-4@http.gcc.gnu.org/bugzilla/>
                   ` (8 preceding siblings ...)
  2022-12-26 17:50 ` pinskia at gcc dot gnu.org
@ 2022-12-26 17:51 ` rhalbersma at gmail dot com
  9 siblings, 0 replies; 10+ messages in thread
From: rhalbersma at gmail dot com @ 2022-12-26 17:51 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #9 from rhalbersma <rhalbersma at gmail dot com> ---
Could this fix also be back-ported to gcc 10?

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-12-26 17:51 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <bug-93480-4@http.gcc.gnu.org/bugzilla/>
2020-12-18 19:28 ` [Bug c++/93480] Defaulted <=> doesn't expand array elements wjwray at gmail dot com
2020-12-19 22:17 ` wjwray at gmail dot com
2020-12-20 14:23 ` jakub at gcc dot gnu.org
2020-12-21 22:49 ` wjwray at gmail dot com
2020-12-22 19:19 ` cvs-commit at gcc dot gnu.org
2020-12-22 19:19 ` jakub at gcc dot gnu.org
2021-01-18 15:18 ` ppalka at gcc dot gnu.org
2021-04-22 19:32 ` ppalka at gcc dot gnu.org
2022-12-26 17:50 ` pinskia at gcc dot gnu.org
2022-12-26 17:51 ` rhalbersma at gmail dot com

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).