On Tue, 8 Mar 2022, Jason Merrill wrote:
> On 3/8/22 14:38, Patrick Palka wrote:
> > On Tue, 8 Mar 2022, Jason Merrill wrote:
> >
> > > On 3/8/22 11:36, Patrick Palka wrote:
> > > > On Mon, 7 Mar 2022, Jason Merrill wrote:
> > > >
> > > > > On 3/7/22 10:47, Patrick Palka wrote:
> > > > > > On Fri, 4 Mar 2022, Jason Merrill wrote:
> > > > > >
> > > > > > > On 3/4/22 14:24, Patrick Palka wrote:
> > > > > > > > Here we're failing to communicate to cp_finish_decl from
> > > > > > > > tsubst_expr
> > > > > > > > that we're in a copy-initialization context (via the
> > > > > > > > LOOKUP_ONLYCONVERTING
> > > > > > > > flag), which causes do_class_deduction to always consider
> > > > > > > > explicit
> > > > > > > > deduction guides when performing CTAD for a templated variable
> > > > > > > > initializer.
> > > > > > > >
> > > > > > > > We could fix this by passing LOOKUP_ONLYCONVERTING appropriately
> > > > > > > > when
> > > > > > > > calling cp_finish_decl from tsubst_expr, but it seems
> > > > > > > > do_class_deduction
> > > > > > > > can determine if we're in a copy-init context by simply
> > > > > > > > inspecting
> > > > > > > > the
> > > > > > > > initializer, and thus render its flags parameter unnecessary,
> > > > > > > > which
> > > > > > > > is
> > > > > > > > what this patch implements. (If we were to fix this in
> > > > > > > > tsubst_expr
> > > > > > > > instead, I think we'd have to inspect the initializer in the
> > > > > > > > same
> > > > > > > > way
> > > > > > > > in order to detect a copy-init context?)
> > > > > > >
> > > > > > > Hmm, does this affect conversions as well?
> > > > > > >
> > > > > > > Looks like it does:
> > > > > > >
> > > > > > > struct A
> > > > > > > {
> > > > > > > explicit operator int();
> > > > > > > };
> > > > > > >
> > > > > > > template <class T> void f()
> > > > > > > {
> > > > > > > T t = A();
> > > > > > > }
> > > > > > >
> > > > > > > int main()
> > > > > > > {
> > > > > > > f<int>(); // wrongly accepted
> > > > > > > }
> > > > > > >
> > > > > > > The reverse, initializing via an explicit constructor, is caught
> > > > > > > by
> > > > > > > code
> > > > > > > in
> > > > > > > build_aggr_init much like the code your patch adds to
> > > > > > > do_auto_deduction;
> > > > > > > perhaps we should move/copy that code to cp_finish_decl?
> > > > > >
> > > > > > Ah, makes sense. Moving that code from build_aggr_init to
> > > > > > cp_finish_decl broke things, but using it in both spots seems to
> > > > > > work
> > > > > > well. And I suppose we might as well use it in do_class_deduction
> > > > > > too,
> > > > > > since doing so lets us remove the flags parameter.
> > > > >
> > > > > Before removing the flags parameter please try asserting that it now
> > > > > matches
> > > > > is_copy_initialization and see if anything breaks.
> > > >
> > > > I added to do_class_deduction:
> > > >
> > > > gcc_assert (bool(flags & LOOKUP_ONLYCONVERTING) ==
> > > > is_copy_initialization
> > > > (init));
> > > >
> > > > Turns out removing the flags parameter breaks CTAD for new-expressions
> > > > of the form 'new TT(x)' because in this case build_new passes just 'x'
> > > > as the initializer to do_auto_deduction (as opposed to a single
> > > > TREE_LIST),
> > > > for which is_copy_initialization returns true even though it's really
> > > > direct initalization.
> > > >
> > > > Also turns out we're similarly not passing the right LOOKUP_* flags to
> > > > cp_finish_decl from instantiate_body, which breaks consideration of
> > > > explicit conversions/deduction guides when instantiating the initializer
> > > > of a static data member. I added some xfailed testcases for these
> > > > situations.
> > >
> > > Maybe we want to check is_copy_initialization in cp_finish_decl?
> >
> > That seems to work nicely :) All xfailed tests for the static data
> > member initialization case now also pass.
> >
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > trunk?
> >
> > -- >8 --
> >
> > Subject: [PATCH] c++: detecting copy-init context during CTAD [PR102137]
> >
> > Here we're failing to communicate to cp_finish_decl from tsubst_expr
> > that we're in a copy-initialization context (via the LOOKUP_ONLYCONVERTING
> > flag), which causes us to always consider explicit deduction guides when
> > performing CTAD for a templated variable initializer.
> >
> > It turns out this bug also affects consideration of explicit conversion
> > operators for the same reason. But consideration of explicit constructors
> > seems to do the right thing thanks to code in build_aggr_init that sets
> > LOOKUP_ONLYCONVERTING when the initializer represents copy-initialization.
> >
> > This patch fixes this by making cp_finish_decl set LOOKUP_ONLYCONVERTING
> > by inspecting the initializer like build_aggr_init does, so that callers
> > don't need to explicitly pass this flag.
> >
> > PR c++/102137
> > PR c++/87820
> >
> > gcc/cp/ChangeLog:
> >
> > * cp-tree.h (is_copy_initialization): Declare.
> > * decl.cc (cp_finish_decl): Set LOOKUP_ONLYCONVERTING
> > when is_copy_initialization is true.
> > * init.cc (build_aggr_init): Split out copy-initialization
> > check into ...
> > (is_copy_initialization): ... here.
> > * pt.cc (instantiate_decl): Pass 0 instead of
> > LOOKUP_ONLYCONVERTING as flags to cp_finish_decl.
>
> Any reason not to use LOOKUP_NORMAL, both here and in tsubst_expr? OK either
> way.
Thanks a lot. I'm not really sure what the consequences of using
LOOKUP_NORMAL (which implies LOOKUP_PROTECT) instead of 0 would be here,
and there's a bunch of other callers of cp_finish_decl which pass 0 or
some other value that excludes LOOKUP_PROTECT. So I figure such a change
would be better off as a separate patch that changes/audits all such
cp_finish_decl at once.
>
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/cpp0x/explicit15.C: New test.
> > * g++.dg/cpp1z/class-deduction108.C: New test.
> > ---
> > gcc/cp/cp-tree.h | 1 +
> > gcc/cp/decl.cc | 3 +
> > gcc/cp/init.cc | 20 +++--
> > gcc/cp/pt.cc | 3 +-
> > gcc/testsuite/g++.dg/cpp0x/explicit15.C | 83 +++++++++++++++++++
> > .../g++.dg/cpp1z/class-deduction108.C | 78 +++++++++++++++++
> > 6 files changed, 181 insertions(+), 7 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/cpp0x/explicit15.C
> > create mode 100644 gcc/testsuite/g++.dg/cpp1z/class-deduction108.C
> >
> > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> > index ac723901098..fd76909ca75 100644
> > --- a/gcc/cp/cp-tree.h
> > +++ b/gcc/cp/cp-tree.h
> > @@ -7039,6 +7039,7 @@ extern void emit_mem_initializers
> > (tree);
> > extern tree build_aggr_init (tree, tree, int,
> > tsubst_flags_t);
> > extern int is_class_type (tree, int);
> > +extern bool is_copy_initialization (tree);
> > extern tree build_zero_init (tree, tree, bool);
> > extern tree build_value_init (tree,
> > tsubst_flags_t);
> > extern tree build_value_init_noctor (tree, tsubst_flags_t);
> > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> > index 199ac768d43..5452f7d9cb3 100644
> > --- a/gcc/cp/decl.cc
> > +++ b/gcc/cp/decl.cc
> > @@ -7968,6 +7968,9 @@ cp_finish_decl (tree decl, tree init, bool
> > init_const_expr_p,
> > if (type == error_mark_node)
> > return;
> > + if (VAR_P (decl) && is_copy_initialization (init))
> > + flags |= LOOKUP_ONLYCONVERTING;
> > +
> > /* Warn about register storage specifiers except when in GNU global
> > or local register variable extension. */
> > if (VAR_P (decl) && DECL_REGISTER (decl) && asmspec_tree == NULL_TREE)
> > diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
> > index 545d904c0f9..d5faa0dc519 100644
> > --- a/gcc/cp/init.cc
> > +++ b/gcc/cp/init.cc
> > @@ -2019,11 +2019,7 @@ build_aggr_init (tree exp, tree init, int flags,
> > tsubst_flags_t complain)
> > return stmt_expr;
> > }
> > - if (init && init != void_type_node
> > - && TREE_CODE (init) != TREE_LIST
> > - && !(TREE_CODE (init) == TARGET_EXPR
> > - && TARGET_EXPR_DIRECT_INIT_P (init))
> > - && !DIRECT_LIST_INIT_P (init))
> > + if (is_copy_initialization (init))
> > flags |= LOOKUP_ONLYCONVERTING;
> > is_global = begin_init_stmts (&stmt_expr, &compound_stmt);
> > @@ -2331,6 +2327,20 @@ is_class_type (tree type, int or_else)
> > return 1;
> > }
> > +/* Returns true iff the variable initializer INIT represents
> > + copy-initialization (and therefore we must set LOOKUP_ONLYCONVERTING
> > + when processing it). */
> > +
> > +bool
> > +is_copy_initialization (tree init)
> > +{
> > + return (init && init != void_type_node
> > + && TREE_CODE (init) != TREE_LIST
> > + && !(TREE_CODE (init) == TARGET_EXPR
> > + && TARGET_EXPR_DIRECT_INIT_P (init))
> > + && !DIRECT_LIST_INIT_P (init));
> > +}
> > +
> > /* Build a reference to a member of an aggregate. This is not a C++
> > `&', but really something which can have its address taken, and
> > then act as a pointer to member, for example TYPE :: FIELD can have
> > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > index 402365285cf..e8b5d8fbb73 100644
> > --- a/gcc/cp/pt.cc
> > +++ b/gcc/cp/pt.cc
> > @@ -26602,8 +26602,7 @@ instantiate_decl (tree d, bool defer_ok, bool
> > expl_inst_class_mem_p)
> > const_init
> > = DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (code_pattern);
> > cp_finish_decl (d, init, /*init_const_expr_p=*/const_init,
> > - /*asmspec_tree=*/NULL_TREE,
> > - LOOKUP_ONLYCONVERTING);
> > + /*asmspec_tree=*/NULL_TREE, 0);
> > }
> > if (enter_context)
> > pop_nested_class ();
> > diff --git a/gcc/testsuite/g++.dg/cpp0x/explicit15.C
> > b/gcc/testsuite/g++.dg/cpp0x/explicit15.C
> > new file mode 100644
> > index 00000000000..076d8b31631
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp0x/explicit15.C
> > @@ -0,0 +1,83 @@
> > +// PR c++/87820
> > +// { dg-do compile { target c++11 } }
> > +
> > +struct A {
> > + constexpr explicit operator int() const { return 0; }
> > +};
> > +
> > +template<class T>
> > +void f() {
> > + A a;
> > + T t1 = a; // { dg-error "cannot convert" }
> > + T t2 = {a}; // { dg-error "cannot convert" }
> > + T t3(a);
> > + T t4{a};
> > + T t5 = T(a);
> > + T t6 = T{a};
> > + new T(a);
> > + new T{a};
> > +}
> > +
> > +template<class T>
> > +void g() {
> > + T t;
> > + int n1 = t; // { dg-error "cannot convert" }
> > + int n2 = {t}; // { dg-error "cannot convert" }
> > + int n3(t);
> > + int n4{t};
> > + int n5 = int(t);
> > + int n6 = int{t};
> > + new int(t);
> > + new int{t};
> > +}
> > +
> > +template void f<int>();
> > +template void g<A>();
> > +
> > +template<class T>
> > +struct B {
> > + static constexpr A a{};
> > + static constexpr T t1 = a; // { dg-error "cannot convert" }
> > + static constexpr T t2 = {a}; // { dg-error "cannot convert" }
> > + static constexpr T t4{a}; // { dg-bogus "cannot convert" }
> > + static constexpr T t5 = T(a);
> > + static constexpr T t6 = T{a};
> > +};
> > +
> > +template<class T>
> > +struct C {
> > + static constexpr T t{};
> > + static constexpr int n1 = t; // { dg-error "cannot convert" }
> > + static constexpr int n2 = {t}; // { dg-error "cannot convert" }
> > + static constexpr int n4{t}; // { dg-bogus "cannot convert" }
> > + static constexpr int n5 = int(t);
> > + static constexpr int n6 = int{t};
> > +};
> > +
> > +template struct B<int>;
> > +template struct C<A>;
> > +
> > +#if __cpp_inline_variables
> > +template<class T>
> > +struct D {
> > + static inline A a;
> > + static inline T t1 = a; // { dg-error "cannot convert" "" { target c++17
> > } }
> > + static inline T t2 = {a}; // { dg-error "cannot convert" "" { target
> > c++17 } }
> > + static inline T t4{a};
> > + static inline T t5 = T(a);
> > + static inline T t6 = T{a};
> > +};
> > +
> > +template<class T>
> > +struct E {
> > + static inline T t;
> > + static inline int n1 = t; // { dg-error "cannot convert" "" { target
> > c++17 } }
> > + static inline int n2 = {t}; // { dg-error "cannot convert" "" { target
> > c++17 } }
> > + static inline int n4{t};
> > + static inline int n5 = int(t);
> > + static inline int n6 = int{t};
> > +};
> > +
> > +template struct D<int>;
> > +template struct E<A>;
> > +#endif
> > diff --git a/gcc/testsuite/g++.dg/cpp1z/class-deduction108.C
> > b/gcc/testsuite/g++.dg/cpp1z/class-deduction108.C
> > new file mode 100644
> > index 00000000000..e82c4bafb04
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp1z/class-deduction108.C
> > @@ -0,0 +1,78 @@
> > +// PR c++/102137
> > +// { dg-do compile { target c++17 } }
> > +
> > +template<class T>
> > +struct A {
> > + constexpr A() { }
> > + constexpr A(int) { }
> > +};
> > +
> > +explicit A(...) -> A<int>;
> > +
> > +template<template<class> class TT>
> > +void f() {
> > + TT x1 = 0; // { dg-error "deduction|no match" }
> > + TT x2 = {0}; // { dg-error "explicit deduction guide" }
> > + TT x3(0);
> > + TT x4{0};
> > + TT x5;
> > + new TT(0);
> > + new TT{0};
> > + new TT();
> > + new TT{};
> > + new TT;
> > +}
> > +
> > +template<class T>
> > +void g(T t) {
> > + A a1 = t; // { dg-error "deduction|no match" }
> > + A a2 = {t}; // { dg-error "explicit deduction guide" }
> > + A a3(t);
> > + A a4{t};
> > + A a5;
> > + new A(t);
> > + new A{t};
> > +}
> > +
> > +template void f<A>();
> > +template void g(int);
> > +
> > +template<template<class> class TT>
> > +struct B {
> > + static inline TT x1 = 0; // { dg-error "deduction|no match" }
> > + static inline TT x2 = {0}; // { dg-error "explicit deduction guide" }
> > + static inline TT x4{0};
> > + static inline TT x5;
> > +};
> > +
> > +template<class T>
> > +struct C {
> > + static inline T t;
> > + static inline A a1 = t; // { dg-error "deduction|no match" }
> > + static inline A a2 = {t}; // { dg-error "explicit deduction guide" }
> > + static inline A a4{t};
> > + static inline A a5{};
> > +};
> > +
> > +template struct B<A>;
> > +template struct C<int>;
> > +
> > +template<template<class> class TT>
> > +struct E {
> > + static constexpr TT x1 = 0; // { dg-error "deduction|no match" }
> > + static constexpr TT x2 = {0}; // { dg-error "explicit deduction guide" }
> > + static constexpr TT x4{0};
> > + static constexpr TT x5{};
> > +};
> > +
> > +template<class T>
> > +struct F {
> > + static constexpr T t{};
> > + static constexpr A a1 = t; // { dg-error "deduction|no match" }
> > + static constexpr A a2 = {t}; // { dg-error "explicit deduction guide" }
> > + static constexpr A a4{t};
> > + static constexpr A a5{};
> > +};
> > +
> > +template struct E<A>;
> > +template struct F<int>;
>
>
