[Bug libstdc++/101263] New: non-propagating-cache::emplace-deref missing constexpr

[Bug libstdc++/101263] New: non-propagating-cache::emplace-deref missing constexpr

[hewillk at gmail dot com]

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

            Bug ID: 101263
           Summary: non-propagating-cache::emplace-deref missing constexpr
           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: ---

Hi, P2328R1 is marked as completed in https://gcc.gnu.org/wiki/LibstdcxxTodo,
but it seems that the constexpr keyword added by R1 has not been added in
libstdc++.

https://godbolt.org/z/d7r6cbTYj

[Bug libstdc++/101263] non-propagating-cache::emplace-deref missing constexpr

[ppalka at gcc dot gnu.org]

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

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

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

--- Comment #1 from Patrick Palka <ppalka at gcc dot gnu.org> ---
It looks like emplace-deref was made constexpr in R1 of the paper, but we  only
implemented R0.  Fortunately this added constexpr seems to be the only
functional change in R1 vs R0.

We might first need to implement P2231 (for constexpr optional) before this
function can be properly constexpr.

[Bug libstdc++/101263] non-propagating-cache::emplace-deref missing constexpr

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |barry.revzin at gmail dot com

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
*** Bug 102236 has been marked as a duplicate of this bug. ***

[Bug libstdc++/101263] non-propagating-cache::emplace-deref missing constexpr

[redi at gcc dot gnu.org]

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

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Patrick Palka from comment #1)
> We might first need to implement P2231 (for constexpr optional) before this
> function can be properly constexpr.

I have a patch for that, but it's not upstream yet.

[Bug libstdc++/101263] non-propagating-cache::emplace-deref missing constexpr

[redi at gcc dot gnu.org]

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

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Patrick Palka from comment #1)
> We might first need to implement P2231 (for constexpr optional) before this
> function can be properly constexpr.

Implemented in r12-4389

[Bug libstdc++/101263] non-propagating-cache::emplace-deref missing constexpr

[ppalka at gcc dot gnu.org]

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

--- Comment #5 from Patrick Palka <ppalka at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #4)
> (In reply to Patrick Palka from comment #1)
> > We might first need to implement P2231 (for constexpr optional) before this
> > function can be properly constexpr.
> 
> Implemented in r12-4389

Yay, thanks!  I thought this would be enough to straightforwardly make
emplace-deref constexpr, but I ran into an unrelated wrinkle.  During constexpr
evaluation we can't use placement new to initialize the contained value
directly from *__i and instead have to use the equivalent of
construct_at(&value, *__i), but the latter incurs an extra move due to the
temporary materialization of *__i, which is contrary to the effects of
emplace-deref ([range.nonprop.cache]/1.6):

  Calls reset().  Then initializes the contained value as if
direct-non-list-initializing an object of type T with the argument *i.

Am I missing something or is it not possible to perform this direct-init in the
constexpr case?

[Bug libstdc++/101263] non-propagating-cache::emplace-deref missing constexpr

[barry.revzin at gmail dot com]

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

--- Comment #6 from Barry Revzin <barry.revzin at gmail dot com> ---
The "real" answer is allowing constexpr placement new, but that obviously
doesn't help you right now.

But I think the helpful answer is that you can add a constructor to your
storage like storage(init_from_invoke_t, Args&&... args) that initializes the
underlying value from invoke((Args&&)args...), and then construct_at(&storage,
init_from_invoke, [&]() -> decltype(auto) { return *i; }).

Something like that?

[Bug libstdc++/101263] non-propagating-cache::emplace-deref missing constexpr

[rs2740 at gmail dot com]

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

TC <rs2740 at gmail dot com> changed:

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

--- Comment #7 from TC <rs2740 at gmail dot com> ---
(In reply to Barry Revzin from comment #6)
> The "real" answer is allowing constexpr placement new, but that obviously
> doesn't help you right now.
> 
> But I think the helpful answer is that you can add a constructor to your
> storage like storage(init_from_invoke_t, Args&&... args) that initializes
> the underlying value from invoke((Args&&)args...), and then
> construct_at(&storage, init_from_invoke, [&]() -> decltype(auto) { return
> *i; }).
> 
> Something like that?

Yes. Something at that level of generality will be needed for the new
optional::transform, so it seems the better approach.

In my proof-of-concept implementation (which didn't have that concern), I used
something tailored to this specific case, along the lines of 

struct __deref_tag {};

template<class _Tp>
struct __cache_wrapper {
    template<class _Iter>
    constexpr __cache_wrapper(__deref_tag, const _Iter& __i)
        : __t(*__i) {}
    _Tp __t;
};

and then stored a __non_propagating_cache<__cache_wrapper<range_value_t<...>>>
__cache, so that emplace-deref(i) is __cache.emplace(__deref_tag{}, i);

[Bug libstdc++/101263] non-propagating-cache::emplace-deref missing constexpr

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:2c564e813c0626802e5bfb066c094933d5e6a774

commit r12-4448-g2c564e813c0626802e5bfb066c094933d5e6a774
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Oct 15 14:49:21 2021 +0100

    libstdc++: Make non-propagating-cache fully constexpr [PR101263]

    libstdc++-v3/ChangeLog:

            PR libstdc++/101263
            * include/std/ranges (__cached): New wrapper struct.
            (__non_propagating_cache): Use __cached for contained value.
            (__non_propagating_cache::_M_emplace_deref): Add constexpr. Use
            std::construct_at instead of placement new.
            * testsuite/std/ranges/adaptors/join.cc: Check constexpr works.

[Bug libstdc++/101263] non-propagating-cache::emplace-deref missing constexpr

[redi at gcc dot gnu.org]

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

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

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

[Bug libstdc++/101263] non-propagating-cache::emplace-deref missing constexpr

[redi at gcc dot gnu.org]

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

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed on trunk. It can't be backported without also backporting r12-1068 to fix
PR100368.

[Bug libstdc++/101263] non-propagating-cache::emplace-deref missing constexpr

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 11.3 is being released, retargeting bugs to GCC 11.4.

[Bug libstdc++/101263] non-propagating-cache::emplace-deref missing constexpr

[redi at gcc dot gnu.org]

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

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

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

--- Comment #11 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I don't think we're going to do this for gcc-11, so closing.