Bug 96926 - [9/10 Regression] Tuple element w/ member reference to incomplete template type rejected
Summary: [9/10 Regression] Tuple element w/ member reference to incomplete template ty...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 10.2.0
: P2 normal
Target Milestone: 11.0
Assignee: Jason Merrill
URL:
Keywords: rejects-valid
Depends on:
Blocks:
 
Reported: 2020-09-03 16:17 UTC by Jonathan Wakely
Modified: 2022-05-13 15:25 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Known to work: 8.3.0
Known to fail: 10.2.1, 11.0, 9.3.1
Last reconfirmed: 2020-09-03 00:00:00


Attachments
Manually reduced code (1.01 KB, text/plain)
2020-09-03 18:26 UTC, Jonathan Wakely
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Wakely 2020-09-03 16:17:08 UTC
+++ This bug was initially created as a clone of Bug #96592 +++

This code seems a regression introduced in g++ 10.1, is still in 10.2 and remains in the trunk.  It works on 9.x and as far back as g++ 6, plus all versions of clang since 6, icc, msvc, etc.

There are many independent changes that can make it work, including: 
  * making SomeQuery constructor a template constructor (on SessionU)
  * changing the reference to a pointer
  * explicitly declaring Session as an incomplete type and not using
    templates


#include <tuple>

template <typename SessionT>
struct SomeQuery {
    SessionT& session_;
    SomeQuery(SessionT& session) : session_(session) {}
};

template <typename SessionT>
struct Handler {
    std::tuple<SomeQuery<SessionT>> queries_;
    Handler(SessionT& session) : queries_(session) {}
};

struct Session {
    Handler<Session> handler_;
    Session() : handler_{*this} {}
};

int main() {
    Session session;
}


It looks like the tuple class is doing some concept checking that isn't quite working, but I haven't dug deeply enough to determine if it's a library or underlying compiler issue.

Live example on Compiler Explorer
https://godbolt.org/z/7naPMx


+++ PR 96592 comment 6 +++

As further evidence the compile is confused, the preprocessed source from GCC 10 can be compiled by GCC 8, but not GCC 9, 10 or trunk.

It started to be rejected with r262172 (fixing PR c++/80290), but the problem was latent until r10-908 made std::tuple vulnerable to it.

I'll start reducing the preprocessed source ...
Comment 1 Jonathan Wakely 2020-09-03 16:18:15 UTC
template <typename> struct b;
template <typename> struct __is_constructible_impl;
template <bool> struct g;
template <bool l, typename> using c = typename g<l>::d;
template <int, typename... e> struct m {
  template <typename> static bool h() {
    return b<__is_constructible_impl<e>...>::i;
  }
};
template <typename... ab> class o {
  template <bool l> using ad = m<l, ab...>;
  template <bool l, typename... j> using k = c<ad<l>::template h<j...>(), bool>;
  template <bool p = 0, k<p, ab...>> o(ab...);
};
struct q {
  o<int> n;
};
struct r {
  q f;
  r();
};
int main() { r a; }

This is accepted by EDG, Clang and GCC 8. Rejected by GCC 9+ since r262172 (fixing PR c++/80290)
Comment 2 Jonathan Wakely 2020-09-03 16:19:24 UTC
Hmm, this might be a bad reduction. __is_constructible_impl really *is* incomplete here. In the real code it isn't.
Comment 3 Jonathan Wakely 2020-09-03 18:26:45 UTC
Created attachment 49180 [details]
Manually reduced code
Comment 4 Jason Merrill 2021-02-12 05:00:18 UTC
Further reduced:

template <typename T> struct A {
  using type = typename T::type;
};
template <typename U> class B {
  template <typename V = int,
            typename A<V>::type X>
  B(U);
};
struct C {
  B<int> b;
  C();
};
int main() {
  C c;
}

We run into trouble trying to implicitly declare the copy constructor for C, which involves overload resolution to find the constructor called for copying B<int>.

After r262172, when we consider the constructor template, we try to substitute the default template argument for V into the type of X, which involves instantiating A<int>, which fails.  Before r262172, we would see that we can't convert B<int> to int, and give up on the candidate before getting to this substitution.  But r262172 moves the convertibility checking until after we're done with deduction, as per http://wg21.link/cwg2369

I think perhaps it's wrong to do substitution at this point because X has no default argument.  Giving it a default argument causes clang 10 to also reject the testcase.
Comment 5 Jason Merrill 2021-02-12 05:09:18 UTC
(In reply to Jason Merrill from comment #4)
> I think perhaps it's wrong to do substitution at this point because X has no
> default argument.  Giving it a default argument causes clang 10 to also
> reject the testcase.

...but changing that doesn't help real testcases, since the tuple constructors do have default args for their constructor parms.
Comment 6 Jason Merrill 2021-02-13 06:12:40 UTC
So, for the second example, the compiler's process is

test() initializes the tuple member
looks for tuple(Test) ctor
considers tuple(tuple&&)
looks for conversion from Test to tuple
considers _ImplicitCtor
instantiates __is_implicitly_constructible
instantiates is_constructible<DependsOnT<Test>, const DependsOnT<Test>&>
considers DependsOnT(Test&)
tries to convert to Test
lazily declares Test copy ctor
looks for tuple(tuple) ctor
considers _ExplicitCtor
instantiates __is_explicitly_constructible
instantiates is_constructible<DependsOnT<Test>, const DependsOnT<Test>&>
fails

So, another problem with trying unsuitable candidates when declaring a copy constructor.  I wonder what rule other compilers are using to break this recursive dependency.  I'm going to try something that zygoloid suggested GCC already did: if we see a non-template perfect match, don't consider any templates.
Comment 7 GCC Commits 2021-02-19 02:22:26 UTC
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>:

https://gcc.gnu.org/g:187d0d5871b1fa572b0238b4989fa067df56778f

commit r11-7287-g187d0d5871b1fa572b0238b4989fa067df56778f
Author: Jason Merrill <jason@redhat.com>
Date:   Sat Feb 13 00:40:11 2021 -0500

    c++: Tuple of self-dependent classes [PR96926]
    
    When compiling this testcase, trying to resolve the initialization for the
    tuple member ends up recursively considering the same set of tuple
    constructor overloads, and since two of them separately depend on
    is_constructible, the one we try second fails to instantiate
    is_constructible because we're still in the middle of instantiating it the
    first time.
    
    Fixed by implementing an optimization that someone suggested we were already
    doing: if we see a non-template candidate that is a perfect match for all
    arguments, we can skip considering template candidates at all.  It would be
    enough to do this only when LOOKUP_DEFAULTED, but it shouldn't hurt in other
    cases.
    
    gcc/cp/ChangeLog:
    
            PR c++/96926
            * call.c (perfect_conversion_p): New.
            (perfect_candidate_p): New.
            (add_candidates): Ignore templates after a perfect non-template.
    
    gcc/testsuite/ChangeLog:
    
            PR c++/96926
            * g++.dg/cpp0x/overload4.C: New test.
Comment 8 GCC Commits 2021-02-19 04:01:25 UTC
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>:

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

commit r11-7289-gd909ead68214042e9876a8df136d0835273d4b86
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Feb 18 21:27:37 2021 -0500

    c++: Tweak PR969626 patch
    
    It occurred to me that other types of conversions use rvaluedness_matches_p,
    but those uses don't affect overload resolution, so we shouldn't look at the
    flag for them.  Fixing that made decltype64.C compile successfully, because
    the non-template candidate was a perfect match, so we now wouldn't consider
    the broken template.  Changing the argument to const& makes it no longer a
    perfect match (because of the added const), so we again get the infinite
    recursion.
    
    This illustrates the limited nature of this optimization/recursion break; it
    works for most copy/move constructors because the constructor we're looking
    for is almost always a perfect match.  If it happens to help improve compile
    time for other calls, that's just a bonus.
    
    gcc/cp/ChangeLog:
    
            PR c++/96926
            * call.c (perfect_conversion_p): Limit rvalueness
            test to reference bindings.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/cpp0x/decltype64.C: Change argument to const&.
Comment 9 Jason Merrill 2021-02-25 16:34:46 UTC
Fixed for GCC 11.  I want to give this one a lot of time before backporting.
Comment 10 Richard Biener 2021-06-01 08:18:28 UTC
GCC 9.4 is being released, retargeting bugs to GCC 9.5.
Comment 11 Jason Merrill 2022-05-13 15:25:57 UTC
Not backporting.