[Bug c++/110122] New: using an aggregate with a member variable with a user defined copy constructor in a class NTTP causes capture and use of the `this` pointer in a generic lambda to produce the following error "-copy constructor- used before its definition"

[Bug c++/110122] New: using an aggregate with a member variable with a user defined copy constructor in a class NTTP causes capture and use of the `this` pointer in a generic lambda to produce the following error "-copy constructor- used before its definition"

[waffl3x at protonmail dot com]

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

            Bug ID: 110122
           Summary: using an aggregate with a member variable with a user
                    defined copy constructor in a class NTTP causes
                    capture and use of the `this` pointer in a generic
                    lambda to produce the following error "-copy
                    constructor- used before its definition"
           Product: gcc
           Version: 13.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: waffl3x at protonmail dot com
  Target Milestone: ---

https://godbolt.org/z/c7nnfjzGK

struct Foo {
    constexpr Foo() = default;
    constexpr Foo(Foo const&) {}
};

struct Bar {
    Foo _;
};

template<Bar v = Bar{}>
struct Doppelganger
{
    void disguise() {
        [this]<typename = void>(){ this; }();
    }
};

void execute() {
    Doppelganger<>{}.disguise();
}

error: 'constexpr Bar::Bar(const Bar&)' used before its definition

The bug is present in GCC 13.0 and 13.1 and in the current trunk.
This was a dreadful one, I am pretty sure there are 2 different bugs at play
here. It took a while to whittle it down to this example, as I was reducing my
code, I was shocked to find that removing a innocuous looking copy constructor
"fixed" the bug. It seemed like the user declared copy constructor is being
produced late.
The bug does not manifest when passing `Foo` directly to `Doppelganger`'s
non-type template parameter, which I also found odd.
Finally, capturing and using the `this` pointer in the body of a generic lambda
(the bug does not manifest with a regular lambda) finally causes the bug to pop
up. I was not able to find a more reduced version.
I found it strange that a generic lambda is required, I would have thought that
it being a generic lambda would delay when the copy constructor definition was
required, but instead it seems to speed it up. As far as I've observed this
seems to be what's happening, as omitting the call to the member function
avoids triggering the bug. 
One final thing to note, declaring the copy constructor as default also avoids
the bug, while a user declared default constructor has no effect at all.

If anyone manages to reduce it further I would be very interested to see it, I
spent a lot of time at it and couldn't seem to find any way to get it to work
when passing an object of `Foo` directly to `Doppelganger` despite that it
feels like it should be irrelevant. Perhaps it being a member of `Bar` moves up
when the definition of the copy constructor is required?

As an aside, even when deleting the copy constructor, the diagnostic for the
correctly rejected code is still rather unhelpful. It confused me for quite a
while as it points to the use of the `this` pointer, or any uses of member
variables, almost as if the compiler was mistaking the expression for the non
type template parameter, `Bar` in this case. I now realize that use of the
`this` pointer probably requires the full type be instantiated from the class
template, or I should say thats my guess.
My point is, perhaps the compiler should be bailing out earlier, like when the
class template is being instantiated with a NTTP that would be invalid.
Upon saying that I realized I should test whether it does or not, and I found
that it was previously the case, maybe not as early as it could be, but I'm
sure the trade off was made for a reason.
Unfortunately, this is not the case in GCC 13.1 and GCC trunk.
https://godbolt.org/z/8b7Mj91xM
I haven't checked if this bug was reported yet, or if it is a bug at all, I
will have to look into whether the decision to require copy constructors for
NTTP's got overturned or not. You can find that topic being discussed here,
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104577 , as of a few months ago it
seems like NTTP's require copy constructors. Once I have confirmed that I can
make another report for this bug if there are no objections, as despite being
relevant to this bug, it seems to be slightly different.

To recap, I believe there are at least 2 bugs here, the first being the delayed
(or perhaps completely omitted) definition of class `Foo`'s user defined copy
constructor. The second being a class with an undefined copy constructor is
incorrectly accepted as a NTTP, even when an object of that type is
initialized. (I have also confirmed that accessing said object does not change
that outcome. https://godbolt.org/z/fhsoPGEda )
And finally, there is a 3rd possible bug here involving generic lambdas (as a
regular lambda does not trigger the bug), however, despite how problematic
generic lambdas seem to be, I am not confident that it is actually responsible
for any problems here. It might just be that the time the generic lambda's call
operator is instantiated is before the delayed definition of the copy
constructor of `Foo`, which isn't because the lambda is instantiating things
too early, it's just that the copy constructor is being instantiated too late.

I apologize for the long winded write up, I spent about 4 hours trying to
troubleshoot my original code (not including yesterday) and another 4 hours
trying to reduce it once I confirmed that the bug was with GCC, so I wanted to
be thorough with relaying my thoughts of it. Hopefully my musings on it are
somewhat helpful rather than not at all.

[Bug c++/110122] [13/14 Regression] using an aggregate with a member variable with a user defined copy constructor in a class NTTP causes capture and use of the `this` pointer in a generic lambda to produce the following error "-copy ctor- used before its definition"

[ppalka at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2023-06-05
      Known to work|                            |12.3.0
                 CC|                            |ppalka at gcc dot gnu.org
           Keywords|                            |rejects-valid
      Known to fail|                            |13.1.0, 14.0
     Ever confirmed|0                           |1
            Summary|using an aggregate with a   |[13/14 Regression] using an
                   |member variable with a user |aggregate with a member
                   |defined copy constructor in |variable with a user
                   |a class NTTP causes capture |defined copy constructor in
                   |and use of the `this`       |a class NTTP causes capture
                   |pointer in a generic lambda |and use of the `this`
                   |to produce the following    |pointer in a generic lambda
                   |error "-copy constructor-   |to produce the following
                   |used before its definition" |error "-copy ctor- used
                   |                            |before its definition"
   Target Milestone|---                         |13.2
           Assignee|unassigned at gcc dot gnu.org      |ppalka at gcc dot gnu.org
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=104577
             Status|UNCONFIRMED                 |ASSIGNED

--- Comment #1 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Confirmed, thanks very much for the detailed analysis and reproducer.  Started
with r13-1045-gcb7fd1ea85feea.

Here's another version of the testcase (with a commented out workaround) that
uses a non-capturing generic lambda and a non-member function:

struct Foo {
    constexpr Foo() = default;
    constexpr Foo(Foo const&) {}
};

struct Bar {
    Foo _;

    // Workaround: force Bar's copy ctor to be synthesized early
    // static Bar force_synthesize_copy_ctor(Bar& b) { return b; };
};

template<Bar v>
struct Doppelganger { };

template<Bar v>
void disguise() {
    [](auto){ Doppelganger<v> d; }(0);
};

void execute() {
    disguise<Bar{}>();
}

[Bug c++/110122] [13/14 Regression] using an aggregate with a member variable with a user defined copy constructor in a class NTTP causes capture and use of the `this` pointer in a generic lambda to produce the following error "-copy ctor- used before its definition"

[ppalka at gcc dot gnu.org]

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

--- Comment #2 from Patrick Palka <ppalka at gcc dot gnu.org> ---
It seems only the first testcase exhibits a 13 regression, we never accepted
the second testcase.

[Bug c++/110122] [13/14 Regression] using an aggregate with a member variable with a user defined copy constructor in a class NTTP causes capture and use of the `this` pointer in a generic lambda to produce the following error "-copy ctor- used before its definition"

[waffl3x at protonmail dot com]

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

--- Comment #3 from waffl3x <waffl3x at protonmail dot com> ---
Very cool, thanks, since your test case seems to cause problems in GCC 11 and
GCC 12 does that mean the bug goes deeper than I thought?

(In reply to Patrick Palka from comment #2)
> It seems only the first testcase exhibits a 13 regression, we never accepted
> the second testcase.

Is that what you meant here btw, that since your second test case triggers the
bug in 11 and 12 that it belongs as a different regression test?

Sadly, the workaround doesn't work when `Bar` is a class template.
https://godbolt.org/z/41xWrar5e
So it doesn't work in my specific case :(

I'm still very impressed that you found another way to trigger it as well as a
workaround, it's just a shame I can't use it.

[Bug c++/110122] [13/14 Regression] using an aggregate with a member variable with a user defined copy constructor in a class NTTP causes capture and use of the `this` pointer in a generic lambda to produce the following error "-copy ctor- used before its definition"

[ppalka at gcc dot gnu.org]

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

--- Comment #4 from Patrick Palka <ppalka at gcc dot gnu.org> ---
(In reply to waffl3x from comment #3)
> Very cool, thanks, since your test case seems to cause problems in GCC 11
> and GCC 12 does that mean the bug goes deeper than I thought?
> 
> (In reply to Patrick Palka from comment #2)
> > It seems only the first testcase exhibits a 13 regression, we never accepted
> > the second testcase.
> 
> Is that what you meant here btw, that since your second test case triggers
> the bug in 11 and 12 that it belongs as a different regression test?

Yes, it seems the original testcase is exhibiting two bugs (one of which a GCC
13 regression), whereas the second testcase exhibits one (non-regression) bug.

In your original testcase Bar's copy constructor shouldn't be needed since the
template parameter V isn't being copied anywhere, but we're somehow end up with
an illegitimate use of the constructor (bug #1) and then we're also failing to
synthesize it (bug #2).

In the second testcase Bar's copy constructor is legitimately needed since
we're arguably making a copy of V when writing the specialization
Doppelganger<V>, but we fail to synthesize the constructor (bug #2).

> 
> Sadly, the workaround doesn't work when `Bar` is a class template.
> https://godbolt.org/z/41xWrar5e
> So it doesn't work in my specific case :(

Ah, does it work for you to give Bar an explicitly defaulted copy and default
ctor?

[Bug c++/110122] [13/14 Regression] using an aggregate with a member variable with a user defined copy constructor in a class NTTP causes capture and use of the `this` pointer in a generic lambda to produce the following error "-copy ctor- used before its definition"

[waffl3x at protonmail dot com]

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

--- Comment #5 from waffl3x <waffl3x at protonmail dot com> ---
(In reply to Patrick Palka from comment #4)
> 
> Yes, it seems the original testcase is exhibiting two bugs (one of which a
> GCC 13 regression), whereas the second testcase exhibits one
> (non-regression) bug.
> 
> In your original testcase Bar's copy constructor shouldn't be needed since
> the template parameter V isn't being copied anywhere, but we're somehow end
> up with an illegitimate use of the constructor (bug #1) and then we're also
> failing to synthesize it (bug #2).
> 
> In the second testcase Bar's copy constructor is legitimately needed since
> we're arguably making a copy of V when writing the specialization
> Doppelganger<V>, but we fail to synthesize the constructor (bug #2).
> 

Ah okay got it, bug #1 is pretty harmless then isn't it, because there should
never be a situation where the illegitimate use of the constructor will be an
error, it only popped up because bug #2 happened.
On the other hand, this makes me wonder about what's going on during class
template instantiation, at worst it's probably an inefficiency rather than
anything potentially harmful though.
> 
> Ah, does it work for you to give Bar an explicitly defaulted copy and
> default ctor?
Yes actually, that does seem to do the trick.
https://godbolt.org/z/x7eYzY6dz

[Bug c++/110122] [13/14 Regression] using an aggregate with a member variable with a user defined copy constructor in a class NTTP causes capture and use of the `this` pointer in a generic lambda to produce the following error "-copy ctor- used before its definition"

[cvs-commit at gcc dot gnu.org]

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

--- 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:682d401a6ba723b2bf98779d056efc8ff2640178

commit r14-1697-g682d401a6ba723b2bf98779d056efc8ff2640178
Author: Patrick Palka <ppalka@redhat.com>
Date:   Sun Jun 11 11:09:16 2023 -0400

    c++: extend lookup_template_class shortcut [PR110122]

    Here when substituting the injected class name A during regeneration of
    the lambda, we find ourselves in lookup_template_class for A<V> with
    V=_ZTAXtl3BarEE (i.e. the template parameter object for Foo{}).  The call
    to coerce_template_parms within then undesirably tries to make a copy of
    this class NTTP argument, which fails because Foo is not copyable.  But it
    seems clear that this testcase shouldn't require copyability of Foo.

    lookup_template_class has a shortcut for looking up the current class
    scope, which would avoid the problematic coerce_template_parms call, but
    the shortcut doesn't trigger because it only considers the innermost
    class scope which in this case in the lambda type.  So this patch fixes
    this by extending the lookup_template_class shortcut to consider outer
    class scopes too (and skipping over lambda types since they are never
    specialized from lookup_template_class).  We also need to avoid calling
    coerce_template_parms when specializing a templated non-template nested
    class for the first time (such as A::B in the testcase).  Coercion should
    be unnecessary there because the innermost arguments belong to the context
    and so should have already been coerced.

            PR c++/110122

    gcc/cp/ChangeLog:

            * pt.cc (lookup_template_class): Extend shortcut for looking up the
            current class scope to consider outer class scopes too, and use
            current_nonlambda_class_type instead of current_class_type.  Only
            call coerce_template_parms when specializing a primary template.

    gcc/testsuite/ChangeLog:

            * g++.dg/cpp2a/nontype-class57.C: New test.
            * g++.dg/cpp2a/nontype-class58.C: New test.

[Bug c++/110122] [13/14 Regression] using an aggregate with a member variable with a user defined copy constructor in a class NTTP causes capture and use of the `this` pointer in a generic lambda to produce the following error "-copy ctor- used before its definition"

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 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:59946a4c0c97c842ac5a34de5b1aadb73b738809

commit r14-1698-g59946a4c0c97c842ac5a34de5b1aadb73b738809
Author: Patrick Palka <ppalka@redhat.com>
Date:   Sun Jun 11 11:27:10 2023 -0400

    c++: unsynthesized defaulted constexpr fn [PR110122]

    In this other testcase from PR110122, during regeneration of the generic
    lambda with V=Bar{}, substitution followed by coerce_template_parms for
    A<V>'s template argument naturally yields a copy of V in terms of Bar's
    (implicitly) defaulted copy constructor.

    This however happens inside a template context so although we introduced
    a use of the copy constructor, mark_used didn't actually synthesize it,
    which causes subsequent constant evaluation of the template argument to
    fail with:

      nontype-class59.C: In instantiation of âvoid f() [with Bar V =
Bar{Foo()}]â:
      nontype-class59.C:22:11:   required from here
      nontype-class59.C:18:18: error: âconstexpr Bar::Bar(const Bar&)â used
before its definition

    We already make sure to instantiate templated constexpr functions needed
    for constant evaluation (as per P0859R0).  So this patch fixes this by
    making us synthesize defaulted constexpr functions needed for constant
    evaluation as well.

            PR c++/110122

    gcc/cp/ChangeLog:

            * constexpr.cc (cxx_eval_call_expression): Synthesize defaulted
            functions needed for constant evaluation.
            (instantiate_cx_fn_r): Likewise.

    gcc/testsuite/ChangeLog:

            * g++.dg/cpp2a/nontype-class59.C: New test.

[Bug c++/110122] [13/14 Regression] using an aggregate with a member variable with a user defined copy constructor in a class NTTP causes capture and use of the `this` pointer in a generic lambda to produce the following error "-copy ctor- used before its definition"

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:33a84d438883b5d6616048636a5fc47f6dd4ed28

commit r13-7590-g33a84d438883b5d6616048636a5fc47f6dd4ed28
Author: Patrick Palka <ppalka@redhat.com>
Date:   Sun Jun 11 11:09:16 2023 -0400

    c++: extend lookup_template_class shortcut [PR110122]

    Here when substituting the injected class name A during regeneration of
    the lambda, we find ourselves in lookup_template_class for A<V> with
    V=_ZTAXtl3BarEE (i.e. the template parameter object for Foo{}).  The call
    to coerce_template_parms within then undesirably tries to make a copy of
    this class NTTP argument, which fails because Foo is not copyable.  But it
    seems clear that this testcase shouldn't require copyability of Foo.

    lookup_template_class has a shortcut for looking up the current class
    scope, which would avoid the problematic coerce_template_parms call, but
    the shortcut doesn't trigger because it only considers the innermost
    class scope which in this case in the lambda type.  So this patch fixes
    this by extending the lookup_template_class shortcut to consider outer
    class scopes too (and skipping over lambda types since they are never
    specialized from lookup_template_class).  We also need to avoid calling
    coerce_template_parms when specializing a templated non-template nested
    class for the first time (such as A::B in the testcase).  Coercion should
    be unnecessary there because the innermost arguments belong to the context
    and so should have already been coerced.

            PR c++/110122

    gcc/cp/ChangeLog:

            * pt.cc (lookup_template_class): Extend shortcut for looking up the
            current class scope to consider outer class scopes too, and use
            current_nonlambda_class_type instead of current_class_type.  Only
            call coerce_template_parms when specializing a primary template.

    gcc/testsuite/ChangeLog:

            * g++.dg/cpp2a/nontype-class57.C: New test.
            * g++.dg/cpp2a/nontype-class58.C: New test.

    (cherry picked from commit 682d401a6ba723b2bf98779d056efc8ff2640178)

[Bug c++/110122] [13/14 Regression] using an aggregate with a member variable with a user defined copy constructor in a class NTTP causes capture and use of the `this` pointer in a generic lambda to produce the following error "-copy ctor- used before its definition"

[ppalka at gcc dot gnu.org]

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

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

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

--- Comment #9 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Fixed for GCC 13.2 -- at least, the original testcase is.  The comment #1
testcase isn't a regression and is only fixed on trunk by the followup commit
r14-1698.