On Fri, 9 Aug 2024, Jason Merrill wrote:
> On 8/9/24 2:32 PM, Patrick Palka wrote:
> > On Fri, 9 Aug 2024, Patrick Palka wrote:
> >
> > > On Fri, 9 Aug 2024, Jason Merrill wrote:
> > >
> > > > On 8/8/24 1:00 PM, Patrick Palka wrote:
> > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this
> > > > > look OK for trunk/14?
> > > > >
> > > > > -- >8 --
> > > > >
> > > > > This implements the inherited vs non-inherited guide tiebreaker
> > > > > specified by P2582R1. In order to track inherited-ness of a guide
> > > > > it seems natural to reuse the lang_decl_fn::context field that already
> > > > > tracks inherited-ness of a constructor.
> > > > >
> > > > > This patch also works around CLASSTYPE_CONSTRUCTORS apparently not
> > > > > always containing all inherited constructors, by iterating over
> > > > > TYPE_FIELDS instead.
> > > > >
> > > > > This patch also makes us recognize another written form of inherited
> > > > > constructor, 'using Base<T>::Base::Base' whose USING_DECL_SCOPE is a
> > > > > TYPENAME_TYPE.
> > > > >
> > > > > PR c++/116276
> > > > >
> > > > > gcc/cp/ChangeLog:
> > > > >
> > > > > * call.cc (joust): Implement P2582R1 inherited vs
> > > > > non-inherited
> > > > > guide tiebreaker.
> > > > > * cp-tree.h (lang_decl_fn::context): Document usage in
> > > > > deduction_guide_p FUNCTION_DECLs.
> > > > > (inherited_guide_p): Declare.
> > > > > * pt.cc (inherited_guide_p): Define.
> > > > > (set_inherited_guide_context): Define.
> > > > > (alias_ctad_tweaks): Use set_inherited_guide_context.
> > > > > (inherited_ctad_tweaks): Recognize some inherited constructors
> > > > > whose scope is a TYPENAME_TYPE.
> > > > > (ctor_deduction_guides_for): For C++23 inherited CTAD, loop
> > > > > over TYPE_FIELDS instead of using CLASSTYPE_CONSTRUCTORS to
> > > > > recognize all relevant using-decls.
> > > > >
> > > > > gcc/testsuite/ChangeLog:
> > > > >
> > > > > * g++.dg/cpp23/class-deduction-inherited4.C: Extend test.
> > > > > * g++.dg/cpp23/class-deduction-inherited5.C: New test.
> > > > > ---
> > > > > gcc/cp/call.cc | 22 +++++++++
> > > > > gcc/cp/cp-tree.h | 8 +++-
> > > > > gcc/cp/pt.cc | 45
> > > > > +++++++++++++++----
> > > > > .../g++.dg/cpp23/class-deduction-inherited4.C | 15 ++++++-
> > > > > .../g++.dg/cpp23/class-deduction-inherited5.C | 25 +++++++++++
> > > > > 5 files changed, 103 insertions(+), 12 deletions(-)
> > > > > create mode 100644
> > > > > gcc/testsuite/g++.dg/cpp23/class-deduction-inherited5.C
> > > > >
> > > > > diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> > > > > index a75e2e5e3af..3287f77b59b 100644
> > > > > --- a/gcc/cp/call.cc
> > > > > +++ b/gcc/cp/call.cc
> > > > > @@ -13261,6 +13261,28 @@ joust (struct z_candidate *cand1, struct
> > > > > z_candidate *cand2, bool warn,
> > > > > else if (cand2->rewritten ())
> > > > > return 1;
> > > > > + /* F1 and F2 are generated from class template argument
> > > > > deduction for a
> > > > > class
> > > > > + D, and F2 is generated from inheriting constructors from a base
> > > > > class
> > > > > of D
> > > > > + while F1 is not, and for each explicit function argument, the
> > > > > corresponding
> > > > > + parameters of F1 and F2 are either both ellipses or have the
> > > > > same type
> > > > > */
> > > > > + if (deduction_guide_p (cand1->fn))
> > > > > + {
> > > > > + bool inherited1 = inherited_guide_p (cand1->fn);
> > > > > + bool inherited2 = inherited_guide_p (cand2->fn);
> > > > > + if (int diff = inherited2 - inherited1)
> > > > > + {
> > > > > + for (i = 0; i < len; ++i)
> > > > > + {
> > > > > + conversion *t1 = cand1->convs[i + off1];
> > > > > + conversion *t2 = cand2->convs[i + off2];
> > > > > + if (!same_type_p (t1->type, t2->type))
> > > >
> > > > I'm not sure this comparison distinguishes between ellipse and
> > > > non-ellipse?
> > > > There doesn't seem to be a testcase for that.
> > >
> > > Unfortunately I haven't been able to come up with a testcase for
> > > the ellipses logic :/ It seems the earlier ICS comparison would always
> > > break the tie first if one parameter is an ellipsis but not the other?
>
> Just mention that in a comment, then.
>
> > > Is there a known example that perhaps motivated the wording?
>
> I'm failing to find anything, it appeared between R0 and R1 without a
> corresponding comment on the wiki.
>
> > > Relatidely I noticed that unlike the inherited guide tiebreaker, the
> > > inherited ctor tiebreaker doesn't mention ellipses, which seems
> > > surprising: https://eel.is/c++draft/over.match.best#general-2.7
> >
> > Here's v2 which is just a tidied up version of v1 along with an extra
> > test (no additional functional change):
> >
> > -- >8 --
> >
> > Subject: [PATCH] c++: inherited CTAD fixes [PR116276]
> >
> > This implements the overlooked inherited vs non-inherited guide
> > tiebreaker from P2582R1. In order to track inherited-ness of a guide
> > it seems natural to reuse the lang_decl_fn::context field which also
> > tracks inherited-ness of a constructor.
> >
> > This patch also works around CLASSTYPE_CONSTRUCTORS not reliably
> > returning all inherited constructors, by iterating over TYPE_FIELDS
> > instead.
>
> Did you investigate why that is?
For e.g.
template<class T>
struct B { };
template<class T>
struct A : B<T> {
A(); // target_bval
using B<T>::B; // decl
};
it's because of an early exit in push_class_level_binding:
else if (TREE_CODE (decl) == USING_DECL
&& DECL_DEPENDENT_P (decl)
&& OVL_P (target_bval))
/* The new dependent using beats an old overload. */
old_decl = bval;
...
if (old_decl && binding->scope == class_binding_level)
{
binding->value = x;
/* It is always safe to clear INHERITED_VALUE_BINDING_P
here. This function is only used to register bindings
from with the class definition itself. */
INHERITED_VALUE_BINDING_P (binding) = 0;
return true;
}
and so we don't call supplement_binding which seems to be what's
responsible for augmenting the binding properly. After the early
exit, CLASSTYPE_CONSTRUCTORS still returns only A().
Note that if we swap the order of the two member declarations above
then CLASSTYPE_CONSTRUCTORS at the end will return only the using.
So the result of name lookup in this case is sensitive to member
declaration order which seems like a bug.
I'm afraid I don't understand the name lookup code enough to
fix this issue..
>
> > This patch also makes us recognize another written form of inherited
> > constructor, 'using Base<T>::Base::Base' whose USING_DECL_SCOPE is a
> > TYPENAME_TYPE.
> >
> > PR c++/116276
> >
> > gcc/cp/ChangeLog:
> >
> > * call.cc (joust): Implement P2582R1 inherited vs non-inherited
> > guide tiebreaker.
> > * cp-tree.h (lang_decl_fn::context): Document usage in
> > deduction_guide_p FUNCTION_DECLs.
> > (inherited_guide_p): Declare.
> > * pt.cc (inherited_guide_p): Define.
> > (set_inherited_guide_context): Define.
> > (alias_ctad_tweaks): Use set_inherited_guide_context.
> > (inherited_ctad_tweaks): Recognize some inherited constructors
> > whose scope is a TYPENAME_TYPE.
> > (ctor_deduction_guides_for): For C++23 inherited CTAD, loop
> > over TYPE_FIELDS instead of using CLASSTYPE_CONSTRUCTORS to
> > recognize all relevant using-decls.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/cpp23/class-deduction-inherited5.C: New test.
> > * g++.dg/cpp23/class-deduction-inherited6.C: New test.
> > ---
> > gcc/cp/call.cc | 23 +++++++++-
> > gcc/cp/cp-tree.h | 8 +++-
> > gcc/cp/pt.cc | 44 ++++++++++++++----
> > .../g++.dg/cpp23/class-deduction-inherited4.C | 4 +-
> > .../g++.dg/cpp23/class-deduction-inherited5.C | 25 ++++++++++
> > .../g++.dg/cpp23/class-deduction-inherited6.C | 46 +++++++++++++++++++
> > 6 files changed, 137 insertions(+), 13 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/cpp23/class-deduction-inherited5.C
> > create mode 100644 gcc/testsuite/g++.dg/cpp23/class-deduction-inherited6.C
> >
> > diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> > index 67d38e2a78a..8ae1312a370 100644
> > --- a/gcc/cp/call.cc
> > +++ b/gcc/cp/call.cc
> > @@ -13262,10 +13262,31 @@ joust (struct z_candidate *cand1, struct
> > z_candidate *cand2, bool warn,
> > else if (cand2->rewritten ())
> > return 1;
> > - /* F1 is generated from a deduction-guide (13.3.1.8) and F2 is not */
> > if (deduction_guide_p (cand1->fn))
> > {
> > gcc_assert (deduction_guide_p (cand2->fn));
> > +
> > + /* F1 and F2 are generated from class template argument deduction for
> > a
> > + class D, and F2 is generated from inheriting constructors from a base
> > + class of D while F1 is not, and for each explicit function argument,
> > + the corresponding parameters of F1 and F2 are either both ellipses or
> > + have the same type. */
> > + bool inherited1 = inherited_guide_p (cand1->fn);
> > + bool inherited2 = inherited_guide_p (cand2->fn);
> > + if (int diff = inherited2 - inherited1)
> > + {
> > + for (i = 0; i < len; ++i)
> > + {
> > + conversion *t1 = cand1->convs[i + off1];
> > + conversion *t2 = cand2->convs[i + off2];
> > + if (!same_type_p (t1->type, t2->type))
> > + break;
> > + }
> > + if (i == len)
> > + return diff;
> > + }
> > +
> > + /* F1 is generated from a deduction-guide (13.3.1.8) and F2 is not */
> > /* We distinguish between candidates from an explicit deduction
> > guide and
> > candidates built from a constructor based on DECL_ARTIFICIAL. */
> > int art1 = DECL_ARTIFICIAL (cand1->fn);
> > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> > index 5807f4e0edb..3e218793ec7 100644
> > --- a/gcc/cp/cp-tree.h
> > +++ b/gcc/cp/cp-tree.h
> > @@ -2973,8 +2973,11 @@ struct GTY(()) lang_decl_fn {
> > chained here. This pointer thunks to return pointer thunks
> > will be chained on the return pointer thunk.
> > For a DECL_CONSTUCTOR_P FUNCTION_DECL, this is the base from
> > - whence we inherit. Otherwise, it is the class in which a
> > - (namespace-scope) friend is defined (if any). */
> > + whence we inherit.
> > + For a deduction_guide_p FUNCTION_DECL, this is the base class
> > + from whence we inherited the guide (if any).
> > + Otherwise, it is the class in which a (namespace-scope) friend
> > + is defined (if any). */
> > tree context;
> > union lang_decl_u5
> > @@ -7662,6 +7665,7 @@ extern bool deduction_guide_p
> > (const_tree);
> > extern bool copy_guide_p (const_tree);
> > extern bool template_guide_p (const_tree);
> > extern bool builtin_guide_p (const_tree);
> > +extern bool inherited_guide_p (const_tree);
> > extern void store_explicit_specifier (tree, tree);
> > extern tree lookup_explicit_specifier (tree);
> > extern tree lookup_imported_hidden_friend (tree);
> > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > index c1d4cdc7e25..d8def23ef3c 100644
> > --- a/gcc/cp/pt.cc
> > +++ b/gcc/cp/pt.cc
> > @@ -29716,6 +29716,25 @@ builtin_guide_p (const_tree fn)
> > return true;
> > }
> > +/* True if FN is a C++23 inherited guide. */
> > +
> > +bool
> > +inherited_guide_p (const_tree fn)
> > +{
> > + gcc_assert (deduction_guide_p (fn));
> > + return LANG_DECL_FN_CHECK (fn)->context != NULL_TREE;
> > +}
> > +
> > +/* Set the base class BASE from which the transformed guide FN
> > + was inherited as part of C++23 inherited CTAD. */
> > +
> > +static void
> > +set_inherited_guide_context (const_tree fn, tree base)
> > +{
> > + gcc_assert (deduction_guide_p (fn));
> > + LANG_DECL_FN_CHECK (fn)->context = base;
> > +}
> > +
> > /* OLDDECL is a _DECL for a template parameter. Return a similar
> > parameter at
> > LEVEL:INDEX, using tsubst_args and complain for substitution into
> > non-type
> > template parameter types. Note that the handling of template template
> > @@ -30486,6 +30505,7 @@ alias_ctad_tweaks (tree tmpl, tree uguides)
> > TREE_TYPE (fprime) = fntype;
> > if (TREE_CODE (fprime) == TEMPLATE_DECL)
> > TREE_TYPE (DECL_TEMPLATE_RESULT (fprime)) = fntype;
> > + set_inherited_guide_context (fprime, utype);
> > }
> > aguides = lookup_add (fprime, aguides);
> > @@ -30517,11 +30537,14 @@ inherited_ctad_tweaks (tree tmpl, tree ctor,
> > tsubst_flags_t complain)
> > template specialization with the template argument list of A but with
> > C as
> > the template. */
> > - /* FIXME: Also recognize inherited constructors of the form 'using
> > C::B::B',
> > - which seem to be represented with TYPENAME_TYPE C::B as
> > USING_DECL_SCOPE?
> > - And recognize constructors inherited from a non-dependent base class,
> > which
> > - seem to be missing from the overload set entirely? */
> > tree scope = USING_DECL_SCOPE (ctor);
> > + if (TREE_CODE (scope) == TYPENAME_TYPE
> > + && (TYPE_IDENTIFIER (TYPE_CONTEXT (scope))
> > + == TYPENAME_TYPE_FULLNAME (scope)))
> > + /* Recognize using B<T>::B::B as an inherited constructor. */
> > + /* FIXME: Also recognize using C::B::B? We might have to call
> > + resolve_typename_type for that. */
> > + scope = TYPE_CONTEXT (scope);
> > if (!CLASS_TYPE_P (scope)
> > || !CLASSTYPE_TEMPLATE_INFO (scope)
> > || !PRIMARY_TEMPLATE_P (CLASSTYPE_TI_TEMPLATE (scope)))
> > @@ -30655,10 +30678,15 @@ ctor_deduction_guides_for (tree tmpl,
> > tsubst_flags_t complain)
> > }
> > if (cxx_dialect >= cxx23)
> > - for (tree ctor : ovl_range (CLASSTYPE_CONSTRUCTORS (type)))
> > - if (TREE_CODE (ctor) == USING_DECL)
> > - {
> > - tree uguides = inherited_ctad_tweaks (tmpl, ctor, complain);
> > + /* FIXME: CLASSTYPE_CONSTRUCTORS doesn't contain (dependent) inherited
> > + constructors if e.g. there's a user-defined constructor. So instead
> > + iterate over TYPE_FIELDS manually to robustly find all relevant
> > using
> > + decls. */
> > + for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN
> > (field))
> > + if (TREE_CODE (field) == USING_DECL
> > + && DECL_NAME (field) == ctor_identifier)
> > + {
> > + tree uguides = inherited_ctad_tweaks (tmpl, field, complain);
> > if (uguides)
> > cands = lookup_add (uguides, cands);
> > }
> > diff --git a/gcc/testsuite/g++.dg/cpp23/class-deduction-inherited4.C
> > b/gcc/testsuite/g++.dg/cpp23/class-deduction-inherited4.C
> > index 5e3a7f42919..806f0167a4e 100644
> > --- a/gcc/testsuite/g++.dg/cpp23/class-deduction-inherited4.C
> > +++ b/gcc/testsuite/g++.dg/cpp23/class-deduction-inherited4.C
> > @@ -13,10 +13,10 @@ using ty1 = B<int>;
> > template<class T=void>
> > struct C : A<int> {
> > - using A<int>::A; // FIXME: we don't notice this one either
> > + using A<int>::A;
> > };
> > -using ty2 = decltype(C(0)); // { dg-bogus "" "" { xfail *-*-* } }
> > +using ty2 = decltype(C(0));
> > using ty2 = C<void>;
> > template<class T>
> > diff --git a/gcc/testsuite/g++.dg/cpp23/class-deduction-inherited5.C
> > b/gcc/testsuite/g++.dg/cpp23/class-deduction-inherited5.C
> > new file mode 100644
> > index 00000000000..d835acb0b79
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp23/class-deduction-inherited5.C
> > @@ -0,0 +1,25 @@
> > +// PR c++/116276
> > +// { dg-do compile { target c++20 } }
> > +
> > +template<class T>
> > +struct Base1 { };
> > +
> > +template<class T>
> > +struct Base2 { };
> > +
> > +template<class T = int>
> > +struct Derived : public Base1<T>, Base2<T> {
> > + using Base1<T>::Base1;
> > + using Base2<T>::Base2;
> > +};
> > +
> > +Derived d;
> > +
> > +template<class T = int>
> > +struct Derived2 : public Base1<T>, Base2<T> {
> > + using Base1<T>::Base1::Base1;
> > + using Base2<T>::Base2::Base2;
> > + Derived2();
> > +};
> > +
> > +Derived2 d2;
> > diff --git a/gcc/testsuite/g++.dg/cpp23/class-deduction-inherited6.C
> > b/gcc/testsuite/g++.dg/cpp23/class-deduction-inherited6.C
> > new file mode 100644
> > index 00000000000..df8199cd3e4
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp23/class-deduction-inherited6.C
> > @@ -0,0 +1,46 @@
> > +// PR c++/116276
> > +// { dg-do compile { target c++23 } }
> > +
> > +template<class T>
> > +struct Base1 {
> > + Base1();
> > + Base1(T);
> > +};
> > +
> > +template<class T>
> > +struct Base2 {
> > + Base2();
> > + Base2(T*);
> > +};
> > +
> > +template<class T = int>
> > +struct Derived : public Base1<T>, Base2<T> {
> > + using Base1<T>::Base1;
> > + using Base2<T>::Base2;
> > +};
> > +
> > +using ty1 = decltype(Derived{});
> > +using ty1 = Derived<int>;
> > +
> > +using ty2 = decltype(Derived{true});
> > +using ty2 = Derived<bool>;
> > +
> > +using ty3 = decltype(Derived{(char*)nullptr});
> > +using ty3 = Derived<char>;
> > +
> > +template<class T = int>
> > +struct Derived2 : public Base1<T>, Base2<T> {
> > + using Base1<T>::Base1;
> > + using Base2<T>::Base2;
> > + Derived2();
> > + Derived2(T);
> > +};
> > +
> > +using ty4 = decltype(Derived2{});
> > +using ty4 = Derived2<int>;
> > +
> > +using ty5 = decltype(Derived2{true});
> > +using ty5 = Derived2<bool>;
> > +
> > +using ty6 = decltype(Derived2{(char*)nullptr});
> > +using ty6 = Derived2<char>;
>
>
