[Bug c++/91780] Discrepancy between gcc 7.4, through 9.2, compared to clang.

[Bug c++/91780] Discrepancy between gcc 7.4, through 9.2, compared to clang.

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |needs-bisection,
                   |                            |needs-reduction

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Looks to be fixed in GCC 10.2.0 and above.

[Bug c++/91780] Discrepancy between gcc 7.4, through 9.2, compared to clang.

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|needs-bisection             |

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed by r11-1571 and r10-8343, although I'm not sure how that affected it.

[Bug c++/91780] Discrepancy between gcc 7.4, through 9.2, compared to clang.

[pinskia at gcc dot gnu.org]

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

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #4)
> Fixed by r11-1571 and r10-8343, although I'm not sure how that affected it.

I suspect it is the tsubst_pack_expansion change.

[Bug c++/91780] Discrepancy between gcc 7.4, through 9.2, compared to clang.

[pinskia at gcc dot gnu.org]

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

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #5)
> (In reply to Jonathan Wakely from comment #4)
> > Fixed by r11-1571 and r10-8343, although I'm not sure how that affected it.
> 
> I suspect it is the tsubst_pack_expansion change.

Just like what helped PR 58354 (and all of those dups).

[Bug c++/91780] Discrepancy between gcc 7.4, through 9.2, compared to clang.

[redi at gcc dot gnu.org]

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

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Before that commit, the zip(as, bs) deduced the type as zip<vector<int>,
vector<int>>, rather than zip<vector<int>&, vector<int>&>.

So:

(In reply to Mikael Persson from comment #0)
> GCC does not change values in tuple of references

Is wrong. There isn't a tuple of references there at all, that's why the
changes were not visible in the original obejcts. The zip created its own
copies, and then those were modified.

[Bug c++/91780] Discrepancy between gcc 7.4, through 9.2, compared to clang.

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|needs-reduction             |

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Reduced:

template<typename T> struct add_pointer { using type = T*; };
template<typename T> struct add_pointer<T&> { using type = T*; };
template<typename T> struct add_pointer<T&&> { using type = T*; };
template<typename T> using add_pointer_t = typename add_pointer<T>::type;

template<typename... T> struct first { };
template<typename T, typename... U> struct first<T, U...> { using type = T; };
template<typename... T> using first_t = typename first<T...>::type;

template <typename... Containers>
struct zip
{
    template <typename... Container_types>
    zip(Containers... containers) : containers_{ &containers... } { }

    add_pointer_t<first_t<Containers...>> containers_[sizeof...(Containers)];
};

template <typename... Container_types>
zip(Container_types&&... containers) -> zip<Container_types...>;

struct vector { int i = 0; };

int main()
{
  vector v1, v2;
  zip z(v1, v2);
  z.containers_[0]->i = 1;
  z.containers_[1]->i = 2;
  if (v1.i == 0)
    throw 1;
}

[Bug c++/91780] Discrepancy between gcc 7.4, through 9.2, compared to clang.

[redi at gcc dot gnu.org]

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

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
GCC 10.1 isn't using the deduction guide at all, it seems to prefer to deduce
the class type using the zip(Containers...) constructor, which deduces
non-reference types.

If I change the deduction guide to:

template <typename... Container_types>
zip(Container_types&&... containers) -> zip<int>;

Then GCC 10.1 still compiles it, because it doesn't use that guide anyway.
Whereas GCC 10.2 gives an error, because it does deduce as zip<int> and so
z.containers_[0]->i is invalid.

[Bug c++/91780] Discrepancy between gcc 7.4, through 9.2, compared to clang.

[mikael.p.persson at gmail dot com]

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

--- Comment #10 from Mikael Persson <mikael.p.persson at gmail dot com> ---
Thats great.
Saw the dates on the other pr, jikes this has been around a while.

Testing
z.containers_[0]->i is clever, ill add a test for that ( or more generally
z.containers_[0]->begin(), static_assert if i can figure out how.

Is there a way to make the syntax for a zip iterator
for(auto& [a,b...] :zip(...))?  So
for(auto [a,...] : zip(...)) Would create local copies as expected?

As is i renamed it to zip::view, which seemed easier to read. ( And fixed a
bunch of other things too, but adding static assert clang only meant its
been practically unused. In particular as clang is still horrendously slow
for std::random compared to gcc. )


On Fri, 17 Dec 2021, 11:51 redi at gcc dot gnu.org, <
gcc-bugzilla@gcc.gnu.org> wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91780
>
> --- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
> GCC 10.1 isn't using the deduction guide at all, it seems to prefer to
> deduce
> the class type using the zip(Containers...) constructor, which deduces
> non-reference types.
>
> If I change the deduction guide to:
>
> template <typename... Container_types>
> zip(Container_types&&... containers) -> zip<int>;
>
> Then GCC 10.1 still compiles it, because it doesn't use that guide anyway.
> Whereas GCC 10.2 gives an error, because it does deduce as zip<int> and so
> z.containers_[0]->i is invalid.
>
> --
> You are receiving this mail because:
> You reported the bug.

[Bug c++/91780] Discrepancy between gcc 7.4, through 9.2, compared to clang.

[redi at gcc dot gnu.org]

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

--- Comment #11 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Can't you just change your constructor to this?

    zip(Containers&&... containers) :
containers_(std::forward<Containers>(containers)...) { }


The problematic deduction seems to only happen because of the odd constructor
with an unused parameter pack that gets deduced as empty.

[Bug c++/91780] Discrepancy between gcc 7.4, through 9.2, compared to clang.

[mikael.p.persson at gmail dot com]

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

--- Comment #12 from Mikael Persson <mikael.p.persson at gmail dot com> ---
Looking at it now, not sure why i didnt.

On Sun, 19 Dec 2021, 14:00 redi at gcc dot gnu.org, <
gcc-bugzilla@gcc.gnu.org> wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91780
>
> --- Comment #11 from Jonathan Wakely <redi at gcc dot gnu.org> ---
> Can't you just change your constructor to this?
>
>     zip(Containers&&... containers) :
> containers_(std::forward<Containers>(containers)...) { }
>
>
> The problematic deduction seems to only happen because of the odd
> constructor
> with an unused parameter pack that gets deduced as empty.
>
> --
> You are receiving this mail because:
> You reported the bug.

[Bug c++/91780] Discrepancy between gcc 7.4, through 9.2, compared to clang.

[mikael.p.persson at gmail dot com]

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

--- Comment #13 from Mikael Persson <mikael.p.persson at gmail dot com> ---
Ah right lifetime

On Sun, 19 Dec 2021, 16:02 Mikael Persson, <mikael.p.persson@gmail.com>
wrote:

> Looking at it now, not sure why i didnt.
>
> On Sun, 19 Dec 2021, 14:00 redi at gcc dot gnu.org, <
> gcc-bugzilla@gcc.gnu.org> wrote:
>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91780
>>
>> --- Comment #11 from Jonathan Wakely <redi at gcc dot gnu.org> ---
>> Can't you just change your constructor to this?
>>
>>     zip(Containers&&... containers) :
>> containers_(std::forward<Containers>(containers)...) { }
>>
>>
>> The problematic deduction seems to only happen because of the odd
>> constructor
>> with an unused parameter pack that gets deduced as empty.
>>
>> --
>> You are receiving this mail because:
>> You reported the bug.
>
>