public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Patrick Palka <ppalka@redhat.com>
To: Jason Merrill <jason@redhat.com>
Cc: Patrick Palka <ppalka@redhat.com>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] c++: detecting copy-init context during CTAD [PR102137]
Date: Mon, 7 Mar 2022 09:47:14 -0500 (EST)	[thread overview]
Message-ID: <a6b54333-a7ad-67ec-a04b-c5a6f5b1f444@idea> (raw)
In-Reply-To: <e7aa6013-6337-d30a-ac45-4852610e1449@redhat.com>

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.

-- >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
is unaffected and seems to work correctly thanks to code in build_aggr_init
that sets LOOKUP_ONLYCONVERTING when the initializer represents
copy-initialization.

This patch factors out the copy-initialization test from build_aggr_init
and reuses it in tsubst_expr for sake of cp_finish_decl.  And we might
as well use it in do_class_deduction too, since doing so lets us get rid
of its flags parameter (which is used only to control whether explicit
deduction guides are considered).

Bootstrapped and regtestd on x86_64-pc-linux-gnu, does this look OK for
trunk?

	PR c++/102137
	PR c++/87820

gcc/cp/ChangeLog:

	* cp-tree.h (is_copy_initialization): Declare.
	(do_auto_deduction): Remove flags parameter.
	* decl.cc (cp_finish_decl): Adjust call to do_auto_deduction.
	* init.cc (build_aggr_init): Split out copy-initialization
	check into ...
	(is_copy_initialization): ... here.
	* pt.cc (convert_template_argument): Adjust call to
	do_auto_deduction.
	(tsubst_expr) <case VAR_DECL>: Pass LOOKUP_ONLYCONVERTING
	to cp_finish_decl when is_copy_initialization.
	(do_class_deduction): Remove flags parameter, and instead
	call is_copy_initialization to determine if we're in a copy-init
	context.
	(do_auto_deduction): Adjust call to do_class_deduction.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/explicit15.C: New test.
	* g++.dg/cpp1z/class-deduction108.C: New test.
---
 gcc/cp/cp-tree.h                              |  4 +--
 gcc/cp/decl.cc                                |  2 +-
 gcc/cp/init.cc                                | 20 +++++++++---
 gcc/cp/pt.cc                                  | 23 ++++++++------
 gcc/testsuite/g++.dg/cpp0x/explicit15.C       | 31 +++++++++++++++++++
 .../g++.dg/cpp1z/class-deduction108.C         | 31 +++++++++++++++++++
 6 files changed, 93 insertions(+), 18 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..be556d2c441 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);
@@ -7279,8 +7280,7 @@ extern tree do_auto_deduction                   (tree, tree, tree,
 						 = tf_warning_or_error,
                                                  auto_deduction_context
 						 = adc_unspecified,
-						 tree = NULL_TREE,
-						 int = LOOKUP_NORMAL);
+						 tree = NULL_TREE);
 extern tree type_uses_auto			(tree);
 extern tree type_uses_auto_or_concept		(tree);
 extern void append_type_to_template_for_access_check (tree, tree, tree,
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 199ac768d43..152f657e9f2 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -8039,7 +8039,7 @@ cp_finish_decl (tree decl, tree init, bool init_const_expr_p,
 	outer_targs = DECL_TI_ARGS (decl);
       type = TREE_TYPE (decl) = do_auto_deduction (type, d_init, auto_node,
 						   tf_warning_or_error, adc,
-						   outer_targs, flags);
+						   outer_targs);
       if (type == error_mark_node)
 	return;
       if (TREE_CODE (type) == FUNCTION_TYPE)
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 d94d4538faa..a3ff81e2c3a 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -8567,8 +8567,7 @@ convert_template_argument (tree parm,
 	   can happen in the context of -fnew-ttp-matching.  */;
       else if (tree a = type_uses_auto (t))
 	{
-	  t = do_auto_deduction (t, arg, a, complain, adc_unify, args,
-				 LOOKUP_IMPLICIT);
+	  t = do_auto_deduction (t, arg, a, complain, adc_unify, args);
 	  if (t == error_mark_node)
 	    return error_mark_node;
 	}
@@ -18606,7 +18605,11 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
 			TREE_TYPE (asmspec_tree) = char_array_type_node;
 		      }
 
-		    cp_finish_decl (decl, init, const_init, asmspec_tree, 0);
+
+		    int flags = LOOKUP_NORMAL;
+		    if (VAR_P (decl) && is_copy_initialization (init))
+		      flags |= LOOKUP_ONLYCONVERTING;
+		    cp_finish_decl (decl, init, const_init, asmspec_tree, flags);
 
 		    if (ndecl != error_mark_node)
 		      cp_finish_decomp (ndecl, first, cnt);
@@ -29832,8 +29835,7 @@ ctad_template_p (tree tmpl)
    type.  */
 
 static tree
-do_class_deduction (tree ptype, tree tmpl, tree init,
-		    int flags, tsubst_flags_t complain)
+do_class_deduction (tree ptype, tree tmpl, tree init, tsubst_flags_t complain)
 {
   /* We should have handled this in the caller.  */
   if (DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl))
@@ -29885,6 +29887,7 @@ do_class_deduction (tree ptype, tree tmpl, tree init,
 
   bool try_list_ctor = false;
   bool list_init_p = false;
+  bool copy_init_p = is_copy_initialization (init);
 
   releasing_vec rv_args = NULL;
   vec<tree,va_gc> *&args = *&rv_args;
@@ -29929,7 +29932,7 @@ do_class_deduction (tree ptype, tree tmpl, tree init,
   /* Prune explicit deduction guides in copy-initialization context (but
      not copy-list-initialization).  */
   bool elided = false;
-  if (!list_init_p && (flags & LOOKUP_ONLYCONVERTING))
+  if (!list_init_p && copy_init_p)
     {
       for (lkp_iterator iter (cands); !elided && iter; ++iter)
 	if (DECL_NONCONVERTING_P (STRIP_TEMPLATE (*iter)))
@@ -30007,7 +30010,7 @@ do_class_deduction (tree ptype, tree tmpl, tree init,
     }
   /* [over.match.list]/1: In copy-list-initialization, if an explicit
      constructor is chosen, the initialization is ill-formed.  */
-  else if (flags & LOOKUP_ONLYCONVERTING)
+  else if (copy_init_p)
     {
       if (DECL_NONCONVERTING_P (fndecl))
 	{
@@ -30045,7 +30048,7 @@ do_class_deduction (tree ptype, tree tmpl, tree init,
 /* Replace occurrences of 'auto' in TYPE with the appropriate type deduced
    from INIT.  AUTO_NODE is the TEMPLATE_TYPE_PARM used for 'auto' in TYPE.
    The CONTEXT determines the context in which auto deduction is performed
-   and is used to control error diagnostics.  FLAGS are the LOOKUP_* flags.
+   and is used to control error diagnostics.
 
    OUTER_TARGS is used during template argument deduction (context == adc_unify)
    to properly substitute the result.  It's also used in the adc_unify and
@@ -30058,7 +30061,7 @@ do_class_deduction (tree ptype, tree tmpl, tree init,
 tree
 do_auto_deduction (tree type, tree init, tree auto_node,
                    tsubst_flags_t complain, auto_deduction_context context,
-		   tree outer_targs, int flags)
+		   tree outer_targs)
 {
   if (init == error_mark_node)
     return error_mark_node;
@@ -30079,7 +30082,7 @@ do_auto_deduction (tree type, tree init, tree auto_node,
 
   if (tree tmpl = CLASS_PLACEHOLDER_TEMPLATE (auto_node))
     /* C++17 class template argument deduction.  */
-    return do_class_deduction (type, tmpl, init, flags, complain);
+    return do_class_deduction (type, tmpl, init, complain);
 
   if (init == NULL_TREE || TREE_TYPE (init) == NULL_TREE)
     /* Nothing we can do with this, even in deduction context.  */
diff --git a/gcc/testsuite/g++.dg/cpp0x/explicit15.C b/gcc/testsuite/g++.dg/cpp0x/explicit15.C
new file mode 100644
index 00000000000..f6c584d5f88
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/explicit15.C
@@ -0,0 +1,31 @@
+// PR c++/87820
+// { dg-do compile { target c++11 } }
+
+struct A {
+  explicit operator int();
+};
+
+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};
+}
+
+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};
+}
+
+template void f<int>();
+template void g<A>();
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..9e36c53c1a4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/class-deduction108.C
@@ -0,0 +1,31 @@
+// PR c++/102137
+// { dg-do compile { target c++17 } }
+
+template<class T>
+struct A {
+  A();
+  A(int);
+};
+
+explicit A(...) -> A<int>;
+
+template<template<class> class T>
+void f() {
+  T x1 = 0; // { dg-error "deduction|no match" }
+  T x2 = {0}; // { dg-error "explicit deduction guide" }
+  T x3(0);
+  T x4{0};
+  T x5;
+}
+
+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;
+}
+
+template void f<A>();
+template void g(int);
-- 
2.35.1.354.g715d08a9e5


  reply	other threads:[~2022-03-07 14:47 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-04 18:24 Patrick Palka
2022-03-04 22:34 ` Jason Merrill
2022-03-07 14:47   ` Patrick Palka [this message]
2022-03-07 19:14     ` Jason Merrill
2022-03-08 15:36       ` Patrick Palka
2022-03-08 16:24         ` Jason Merrill
2022-03-08 18:38           ` Patrick Palka
2022-03-08 20:07             ` Jason Merrill
2022-03-08 21:17               ` Patrick Palka
2022-03-08 22:26                 ` Jason Merrill

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a6b54333-a7ad-67ec-a04b-c5a6f5b1f444@idea \
    --to=ppalka@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jason@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).