[Bug c++/115550] New: [coroutines] Reference to reference in promise constructor template argument corresponding to coroutine *this

[Bug c++/115550] New: [coroutines] Reference to reference in promise constructor template argument corresponding to coroutine *this

[henri.vettenranta at bitwise dot fi]

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

            Bug ID: 115550
           Summary: [coroutines] Reference to reference in promise
                    constructor template argument corresponding to
                    coroutine *this
           Product: gcc
           Version: 11.4.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: henri.vettenranta at bitwise dot fi
  Target Milestone: ---

Created attachment 58467
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58467&action=edit
Preprocessed source that triggers the bug

The attached preprocessed file is from GCC 11.4.0, but the bug can also be
reproduced with GCC 14.1.0 on godbolt.org.

When the coroutine promise type constructor is a template using a forwarding
reference for the first parameter and the coroutine in question is a non-static
member function, GCC seems to deduce the promise constructor template type
argument to be a reference to a reference. In the attached code, it can be seen
that removing one reference results in one reference being left, and removing
two references results in a different type.

This may have the same underlying cause as bug 104981. It would not be
surprising if some internal compiler code removed the outer reference and
expected to be left with a type that is not a reference. I can't preclude this
being just a coincidence, however.

Relevant parts of the code (full preprocessed source in the attachment):

struct coroutine
{
    struct promise_type
    {
        template <typename Arg>
        explicit promise_type(Arg&&)
        {
            static_assert(same_as<
                remove_reference_t<remove_reference_t<Arg>>,
                remove_reference_t<Arg>
            >);
        }

        // ...
    };
};

struct x
{
    coroutine f()
    {
        co_return;
    }
};

===

$ g++ -v -Wall -Wextra -Wpedantic -std=c++20 -c -o /dev/null gcccoroctorbug.i
Using built-in specs.
COLLECT_GCC=g++
OFFLOAD_TARGET_NAMES=nvptx-none:amdgcn-amdhsa
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu
11.4.0-1ubuntu1~22.04' --with-bugurl=file:///usr/share/doc/gcc-11/README.Bugs
--enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++,m2 --prefix=/usr
--with-gcc-major-version-only --program-suffix=-11
--program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id
--libexecdir=/usr/lib --without-included-gettext --enable-threads=posix
--libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu
--enable-libstdcxx-debug --enable-libstdcxx-time=yes
--with-default-libstdcxx-abi=new --enable-gnu-unique-object
--disable-vtable-verify --enable-plugin --enable-default-pie --with-system-zlib
--enable-libphobos-checking=release --with-target-system-zlib=auto
--enable-objc-gc=auto --enable-multiarch --disable-werror --enable-cet
--with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32
--enable-multilib --with-tune=generic
--enable-offload-targets=nvptx-none=/build/gcc-11-XeT9lY/gcc-11-11.4.0/debian/tmp-nvptx/usr,amdgcn-amdhsa=/build/gcc-11-XeT9lY/gcc-11-11.4.0/debian/tmp-gcn/usr
--without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu
--host=x86_64-linux-gnu --target=x86_64-linux-gnu
--with-build-config=bootstrap-lto-lean --enable-link-serialization=2
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 11.4.0 (Ubuntu 11.4.0-1ubuntu1~22.04) 
COLLECT_GCC_OPTIONS='-v' '-Wall' '-Wextra' '-Wpedantic' '-std=c++20' '-c' '-o'
'/dev/null' '-shared-libgcc' '-mtune=generic' '-march=x86-64'
 /usr/lib/gcc/x86_64-linux-gnu/11/cc1plus -fpreprocessed gcccoroctorbug.i
-quiet -dumpbase gcccoroctorbug.i -dumpbase-ext .i -mtune=generic -march=x86-64
-Wall -Wextra -Wpedantic -std=c++20 -version -fasynchronous-unwind-tables
-fstack-protector-strong -Wformat-security -fstack-clash-protection
-fcf-protection -o /tmp/ccWWTO55.s
GNU C++20 (Ubuntu 11.4.0-1ubuntu1~22.04) version 11.4.0 (x86_64-linux-gnu)
        compiled by GNU C version 11.4.0, GMP version 6.2.1, MPFR version
4.1.0, MPC version 1.2.1, isl version isl-0.24-GMP

GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
GNU C++20 (Ubuntu 11.4.0-1ubuntu1~22.04) version 11.4.0 (x86_64-linux-gnu)
        compiled by GNU C version 11.4.0, GMP version 6.2.1, MPFR version
4.1.0, MPC version 1.2.1, isl version isl-0.24-GMP

GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
Compiler executable checksum: d591828bb4d392ae8b7b160e5bb0b95f
gcccoroctorbug.cpp: In instantiation of
‘coroutine::promise_type::promise_type(Arg&&) [with Arg = x&&]’:
gcccoroctorbug.cpp:44:5:   required from here
gcccoroctorbug.cpp:23:27: error: static assertion failed
   23 |             static_assert(same_as<
      |                           ^~~~~~~~
   24 |                 remove_reference_t<remove_reference_t<Arg>>,
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   25 |                 remove_reference_t<Arg>
      |                 ~~~~~~~~~~~~~~~~~~~~~~~
   26 |             >);
      |             ~              
gcccoroctorbug.cpp:23:27: note: ‘same_as<x, x&>’ evaluates to false
gcccoroctorbug.cpp:23:27: note: constraints not satisfied
gcccoroctorbug.cpp:14:9:   required by the constraints of ‘template<class T,
class U> concept same_as’
gcccoroctorbug.cpp:14:34: note: the expression ‘is_same<
<template-parameter-1-1>, <template-parameter-1-2> >::value [with T = x; U =
x&]’ evaluated to ‘false’
   14 | concept same_as = is_same<T, U>::value;
      |                                  ^~~~~

===

Note that even though the error message says Arg = x&&, it seems that this
should be understood as Arg being lvalue reference to lvalue reference to x,
not rvalue reference to x.

[Bug c++/115550] [coroutines] Reference to reference in promise constructor template argument corresponding to coroutine *this

[ppalka at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
         Depends on|                            |104981
   Last reconfirmed|                            |2024-07-17
     Ever confirmed|0                           |1
                 CC|                            |ppalka at gcc dot gnu.org

--- Comment #1 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Confirmed, thanks for the detailed bug report.

> This may have the same underlying cause as bug 104981. It would not be surprising if some internal compiler code removed the outer reference and expected to be left with a type that is not a reference. I can't preclude this being just a coincidence, however.

You're spot on, the proposed patch in PR104981#c3 seems to also fix this.


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104981
[Bug 104981] [coroutines] Internal compiler error when promise object's
constructor takes a base class of the object parameter

[Bug c++/115550] [coroutines] Reference to reference in promise constructor template argument corresponding to coroutine *this

[cvs-commit at gcc dot gnu.org]

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

--- Comment #2 from GCC 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:7c5a9bf1d206fe20cb050200d4a30f11c76b1b19

commit r15-2210-g7c5a9bf1d206fe20cb050200d4a30f11c76b1b19
Author: Patrick Palka <ppalka@redhat.com>
Date:   Mon Jul 22 21:30:49 2024 -0400

    c++/coroutines: correct passing *this to promise type [PR104981]

    When passing *this to the promise type ctor (or to its operator new)
    (as per [dcl.fct.def.coroutine]/4), we add an explicit cast to lvalue
    reference.  But this is unnecessary since *this is already always an
    lvalue.  And doing so means we need to call convert_from_reference
    afterward to lower the reference expression to an implicit dereference,
    which we're currently neglecting to do and which causes overload
    resolution to get confused when computing argument conversions.

    So this patch removes this unneeded reference cast when passing *this
    to the promise ctor, and removes both the cast and implicit deref when
    passing *this to operator new, for consistency.  While we're here, use
    cp_build_fold_indirect_ref instead of directly building INDIRECT_REF.

            PR c++/104981
            PR c++/115550

    gcc/cp/ChangeLog:

            * coroutines.cc (morph_fn_to_coro): Remove unneeded calls
            to convert_to_reference and convert_from_reference when
            passing *this.  Use cp_build_fold_indirect_ref instead
            of directly building INDIRECT_REF.

    gcc/testsuite/ChangeLog:

            * g++.dg/coroutines/pr104981-preview-this.C: New test.
            * g++.dg/coroutines/pr115550-preview-this.C: New test.

    Reviewed-by: Iain Sandoe <iain@sandoe.co.uk>
    Reviewed-by: Jason Merrill <jason@redhat.com>

[Bug c++/115550] [coroutines] Reference to reference in promise constructor template argument corresponding to coroutine *this

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:066c7893eae0bfc7d9b33b931f115f455246c914

commit r14-10498-g066c7893eae0bfc7d9b33b931f115f455246c914
Author: Patrick Palka <ppalka@redhat.com>
Date:   Mon Jul 22 21:30:49 2024 -0400

    c++/coroutines: correct passing *this to promise type [PR104981]

    When passing *this to the promise type ctor (or to its operator new)
    (as per [dcl.fct.def.coroutine]/4), we add an explicit cast to lvalue
    reference.  But this is unnecessary since *this is already always an
    lvalue.  And doing so means we need to call convert_from_reference
    afterward to lower the reference expression to an implicit dereference,
    which we're currently neglecting to do and which causes overload
    resolution to get confused when computing argument conversions.

    So this patch removes this unneeded reference cast when passing *this
    to the promise ctor, and removes both the cast and implicit deref when
    passing *this to operator new, for consistency.  While we're here, use
    cp_build_fold_indirect_ref instead of directly building INDIRECT_REF.

            PR c++/104981
            PR c++/115550

    gcc/cp/ChangeLog:

            * coroutines.cc (morph_fn_to_coro): Remove unneeded calls
            to convert_to_reference and convert_from_reference when
            passing *this.  Use cp_build_fold_indirect_ref instead
            of directly building INDIRECT_REF.

    gcc/testsuite/ChangeLog:

            * g++.dg/coroutines/pr104981-preview-this.C: New test.
            * g++.dg/coroutines/pr115550-preview-this.C: New test.

    Reviewed-by: Iain Sandoe <iain@sandoe.co.uk>
    Reviewed-by: Jason Merrill <jason@redhat.com>
    (cherry picked from commit 7c5a9bf1d206fe20cb050200d4a30f11c76b1b19)

[Bug c++/115550] [coroutines] Reference to reference in promise constructor template argument corresponding to coroutine *this

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 from GCC 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:303bed670af962c01b77a4f0c51de97f70e8167e

commit r15-2932-g303bed670af962c01b77a4f0c51de97f70e8167e
Author: Patrick Palka <ppalka@redhat.com>
Date:   Thu Aug 15 10:20:18 2024 -0400

    c++/coroutines: fix passing *this to promise type, again [PR116327]

    In r15-2210 we got rid of the unnecessary cast to lvalue reference when
    passing *this to the promise type ctor, and as a drive-by change we also
    simplified the code to use cp_build_fold_indirect_ref.

    But it turns out cp_build_fold_indirect_ref does too much here, namely
    it has a shortcut for returning current_class_ref if the operand is
    current_class_ptr.  The problem with that shortcut is current_class_ref
    might have gotten clobbered earlier if it appeared in the function body,
    since rewrite_param_uses walks and rewrites in-place all local variable
    uses to their corresponding frame copy.

    So later cp_build_fold_indirect_ref for *this will instead return the
    clobbered current_class_ref i.e. *frame_ptr->this, which doesn't make
    sense here since we're in the ramp function and not the actor function
    where frame_ptr is in scope.

    This patch fixes this by using the build_fold_indirect_ref instead of
    cp_build_fold_indirect_ref.

            PR c++/116327
            PR c++/104981
            PR c++/115550

    gcc/cp/ChangeLog:

            * coroutines.cc (morph_fn_to_coro): Use build_fold_indirect_ref
            instead of cp_build_fold_indirect_ref.

    gcc/testsuite/ChangeLog:

            * g++.dg/coroutines/pr104981-preview-this.C: Improve coverage by
            adding a non-static data member use within the coroutine member
            function.
            * g++.dg/coroutines/pr116327-preview-this.C: New test.

    Reviewed-by: Jason Merrill <jason@redhat.com>

[Bug c++/115550] [coroutines] Reference to reference in promise constructor template argument corresponding to coroutine *this

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:63c51e09d160a44fdce1199e8efe9d293f773a2c

commit r14-10586-g63c51e09d160a44fdce1199e8efe9d293f773a2c
Author: Patrick Palka <ppalka@redhat.com>
Date:   Thu Aug 15 10:20:18 2024 -0400

    c++/coroutines: fix passing *this to promise type, again [PR116327]

    In r15-2210 we got rid of the unnecessary cast to lvalue reference when
    passing *this to the promise type ctor, and as a drive-by change we also
    simplified the code to use cp_build_fold_indirect_ref.

    But it turns out cp_build_fold_indirect_ref does too much here, namely
    it has a shortcut for returning current_class_ref if the operand is
    current_class_ptr.  The problem with that shortcut is current_class_ref
    might have gotten clobbered earlier if it appeared in the function body,
    since rewrite_param_uses walks and rewrites in-place all local variable
    uses to their corresponding frame copy.

    So later cp_build_fold_indirect_ref for *this will instead return the
    clobbered current_class_ref i.e. *frame_ptr->this, which doesn't make
    sense here since we're in the ramp function and not the actor function
    where frame_ptr is in scope.

    This patch fixes this by using the build_fold_indirect_ref instead of
    cp_build_fold_indirect_ref.

            PR c++/116327
            PR c++/104981
            PR c++/115550

    gcc/cp/ChangeLog:

            * coroutines.cc (morph_fn_to_coro): Use build_fold_indirect_ref
            instead of cp_build_fold_indirect_ref.

    gcc/testsuite/ChangeLog:

            * g++.dg/coroutines/pr104981-preview-this.C: Improve coverage by
            adding a non-static data member use within the coroutine member
            function.
            * g++.dg/coroutines/pr116327-preview-this.C: New test.

    Reviewed-by: Jason Merrill <jason@redhat.com>
    (cherry picked from commit 303bed670af962c01b77a4f0c51de97f70e8167e)

[Bug c++/115550] [coroutines] Reference to reference in promise constructor template argument corresponding to coroutine *this

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115550
Bug 115550 depends on bug 104981, which changed state.

Bug 104981 Summary: [coroutines] Internal compiler error when promise object's constructor takes a base class of the object parameter
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104981

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

[Bug c++/115550] [coroutines] Reference to reference in promise constructor template argument corresponding to coroutine *this

[ppalka at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
   Target Milestone|---                         |14.3
             Status|NEW                         |RESOLVED

--- Comment #6 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Properly fixed for GCC 14.3