[Bug libstdc++/100475] New: semiregular-box's constructor uses wrong list-initialization

[Bug libstdc++/100475] New: semiregular-box's constructor uses wrong list-initialization

[hewillk at gmail dot com]

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

            Bug ID: 100475
           Summary: semiregular-box's constructor uses wrong
                    list-initialization
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: hewillk at gmail dot com
  Target Milestone: ---

The specialization of semiregular-box's constructor in ranges#L166:

template<typename... _Args>
  requires constructible_from<_Tp, _Args...>
  constexpr explicit
  __box(in_place_t, _Args&&... __args)
  noexcept(is_nothrow_constructible_v<_Tp, _Args...>)
  : _M_value{std::forward<_Args>(__args)...}
  { }

uses the wrong list-initialization, which will incorrectly construct the vector
with the initializer list in the following example:

#include <iostream>
#include <ranges>
#include <vector>

int main() {
  std::ranges::single_view<std::vector<int>> single(std::in_place, 100, 0);
  std::cout << single.begin()->size() << "\n"; 
}

https://godbolt.org/z/c88o3WzYP

[Bug libstdc++/100475] semiregular-box's constructor uses wrong list-initialization

[ppalka at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2021-05-07
           Assignee|unassigned at gcc dot gnu.org      |ppalka at gcc dot gnu.org
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1
                 CC|                            |ppalka at gcc dot gnu.org

--- Comment #1 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Confirmed.

[Bug libstdc++/100475] semiregular-box's constructor uses wrong list-initialization

[hewillk at gmail dot com]

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

--- Comment #2 from 康桓瑋 <hewillk at gmail dot com> ---
Reduced example:

#include <ranges>

struct S {
  S() = default;
  S(int, int) {}
  S(std::initializer_list<int>) = delete;
};

std::ranges::single_view<S> single(std::in_place, 0, 0);

https://godbolt.org/z/d1bE8sPdd

[Bug libstdc++/100475] semiregular-box's constructor uses wrong list-initialization

[hewillk at gmail dot com]

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

--- Comment #3 from 康桓瑋 <hewillk at gmail dot com> ---
Also, the operator->() simply uses operator& instead of std::__addressof.

https://godbolt.org/z/zfGnEoePG

[Bug libstdc++/100475] semiregular-box's constructor uses wrong list-initialization

[hewillk at gmail dot com]

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

--- Comment #4 from 康桓瑋 <hewillk at gmail dot com> ---
(In reply to 康桓瑋 from comment #3)
> Also, the operator->() simply uses operator& instead of std::__addressof.
> 
> https://godbolt.org/z/zfGnEoePG

Another issue is that the has_value() of this specialization will always return
true, which will make the precondition __glibcxx_assert(_M_pred.has_value())
for filter_view's begin() almost useless.

Although it is not clear why semiregular-box is partial specialization in
gcc-11, it is obvious that this needs to be treated with care.

[Bug libstdc++/100475] semiregular-box's constructor uses wrong list-initialization

[ppalka at gcc dot gnu.org]

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

--- Comment #5 from Patrick Palka <ppalka at gcc dot gnu.org> ---
(In reply to 康桓瑋 from comment #4)
> (In reply to 康桓瑋 from comment #3)
> > Also, the operator->() simply uses operator& instead of std::__addressof.
> > 
> > https://godbolt.org/z/zfGnEoePG

Good catch, thanks.

> 
> Another issue is that the has_value() of this specialization will always
> return true, which will make the precondition
> __glibcxx_assert(_M_pred.has_value()) for filter_view's begin() almost
> useless.

That's fine I suppose, the assert will just be a no-op in that case.

> 
> Although it is not clear why semiregular-box is partial specialization in
> gcc-11, it is obvious that this needs to be treated with care.

The advantage of the partial specialization is that it's space-optimal whereas
the primary template is not, due to its use of std::optional.

[Bug libstdc++/100475] semiregular-box's constructor uses wrong list-initialization

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

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

commit r12-856-gfe993b469c528230d9a01e1ae2208610f960dd9f
Author: Patrick Palka <ppalka@redhat.com>
Date:   Tue May 18 00:28:44 2021 -0400

    libstdc++: Fix up semiregular-box partial specialization [PR100475]

    This makes the in-place constructor of our partial specialization of
    __box for already-semiregular types perform direct-non-list-initialization
    (in accordance with the specification of the primary template), and
    additionally makes the member function data() use std::__addressof.

    libstdc++-v3/ChangeLog:

            PR libstdc++/100475
            * include/std/ranges (__box::__box): Use non-list-initialization
            in member initializer list of in-place constructor of the
            partial specialization for semiregular types.
            (__box::operator->): Use std::__addressof.
            * testsuite/std/ranges/adaptors/detail/semiregular_box.cc
            (test02): New test.
            * testsuite/std/ranges/single_view.cc (test04): New test.

[Bug libstdc++/100475] semiregular-box's constructor uses wrong list-initialization

[hewillk at gmail dot com]

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

--- Comment #7 from 康桓瑋 <hewillk at gmail dot com> ---
(In reply to CVS Commits from comment #6)
> The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:
> 
> https://gcc.gnu.org/g:fe993b469c528230d9a01e1ae2208610f960dd9f
> 
> commit r12-856-gfe993b469c528230d9a01e1ae2208610f960dd9f
> Author: Patrick Palka <ppalka@redhat.com>
> Date:   Tue May 18 00:28:44 2021 -0400
> 
>     libstdc++: Fix up semiregular-box partial specialization [PR100475]
>     
>     This makes the in-place constructor of our partial specialization of
>     __box for already-semiregular types perform
> direct-non-list-initialization
>     (in accordance with the specification of the primary template), and
>     additionally makes the member function data() use std::__addressof.
>     
>     libstdc++-v3/ChangeLog:
>     
>             PR libstdc++/100475
>             * include/std/ranges (__box::__box): Use non-list-initialization
>             in member initializer list of in-place constructor of the
>             partial specialization for semiregular types.
>             (__box::operator->): Use std::__addressof.
>             * testsuite/std/ranges/adaptors/detail/semiregular_box.cc
>             (test02): New test.
>             * testsuite/std/ranges/single_view.cc (test04): New test.


I think that even list-initialization with a single parameter should be changed
to direct-non-list-initialization to avoid bugs in some uncommon situations.



#include <ranges>

struct S {
  S() = default;
  S(std::initializer_list<S>) = delete;
  S(const S&) {}
};
S obj;

auto l = std::initializer_list<S>{{}, {}};
auto x = std::views::single(obj);
auto y = std::views::single(std::move(l));

https://godbolt.org/z/7nePj6Y57

[Bug libstdc++/100475] semiregular-box's constructor uses wrong list-initialization

[redi at gcc dot gnu.org]

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

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
We should almost never use list-init in the library unless the standard
explicitly specifies it. Even if the standard specifies it, we should consider
whether that's a defect in the standard. Uniform init is uniformly bad.

[Bug libstdc++/100475] semiregular-box's constructor uses wrong list-initialization

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Patrick Palka
<ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:258aedd9ad1ea4597528632e93dee860acc2eaf5

commit r11-8566-g258aedd9ad1ea4597528632e93dee860acc2eaf5
Author: Patrick Palka <ppalka@redhat.com>
Date:   Tue May 18 00:28:44 2021 -0400

    libstdc++: Fix up semiregular-box partial specialization [PR100475]

    This makes the in-place constructor of our partial specialization of
    __box for already-semiregular types perform direct-non-list-initialization
    (in accordance with the specification of the primary template), and
    additionally makes the member function data() use std::__addressof.

    libstdc++-v3/ChangeLog:

            PR libstdc++/100475
            * include/std/ranges (__box::__box): Use non-list-initialization
            in member initializer list of in-place constructor of the
            partial specialization for semiregular types.
            (__box::operator->): Use std::__addressof.
            * testsuite/std/ranges/adaptors/detail/semiregular_box.cc
            (test02): New test.
            * testsuite/std/ranges/single_view.cc (test04): New test.

    (cherry picked from commit fe993b469c528230d9a01e1ae2208610f960dd9f)

[Bug libstdc++/100475] semiregular-box's constructor uses wrong list-initialization

[ppalka at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|---                         |11.3

--- Comment #10 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Fixed for GCC 11.3 and 12.

(10.4 doesn't implement this partial specialization of __box)

[Bug libstdc++/100475] semiregular-box's constructor uses wrong list-initialization

[ppalka at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|11.3                        |11.2

--- Comment #11 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Er, this was actually already fixed for 11.2