c++: CTAD within template argument [PR102933]

Here when checking for erroneous occurrences of 'auto' inside a template
argument (which is allowed by the concepts TS for class templates),
extract_autos_r picks up the CTAD placeholder for X{T{0}} which causes
check_auto_in_tmpl_args to reject this valid template argument.  This
patch fixes this by making extract_autos_r ignore CTAD placeholders.

However, it seems we don't need to call check_auto_in_tmpl_args at all
outside of the concepts TS since using 'auto' as a type-id is otherwise
rejected more generally at parse time.  So this patch guards calls to
check_auto_in_tmpl_args with flag_concepts_ts instead of flag_concepts.

Relatedly, I think the concepts code paths in do_auto_deduction and
type_uses_auto are also necessary only for the concepts TS, so this
patch also restricts these code paths accordingly.

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

	PR c++/102933

gcc/cp/ChangeLog:

	* parser.c (cp_parser_template_id): Call check_auto_in_tmpl_args
	only for the concepts TS not also for standard concepts.
	(cp_parser_simple_type_specifier): Adjust diagnostic for using
	auto in parameter declaration.
	* pt.c (tsubst_qualified_id): Call check_auto_in_tmpl_args only
	for the concepts TS not also for standard concepts.
	(extract_autos_r): Ignore CTAD placeholders.
	(extract_autos): Use range-based for.
	(do_auto_deduction): Use extract_autos only for the concepts TS
	and not also for standard concepts.
	(type_uses_auto): Likewise with for_each_template_parm.
	(check_auto_in_tmpl_args): Assert that this function is used only
	for the concepts tS.  Simplify.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/nontype-class50.C: New test.
	* g++.dg/cpp2a/nontype-class50a.C: New test.
---
 gcc/cp/parser.c                               |  4 ++--
 gcc/cp/pt.c                                   | 24 +++++++++----------
 gcc/testsuite/g++.dg/cpp2a/nontype-class50.C  | 13 ++++++++++
 gcc/testsuite/g++.dg/cpp2a/nontype-class50a.C |  5 ++++
 4 files changed, 32 insertions(+), 14 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-class50.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-class50a.C

On 10/26/21 13:44, Patrick Palka wrote:
> Here when checking for erroneous occurrences of 'auto' inside a template
> argument (which is allowed by the concepts TS for class templates),
> extract_autos_r picks up the CTAD placeholder for X{T{0}} which causes
> check_auto_in_tmpl_args to reject this valid template argument.  This
> patch fixes this by making extract_autos_r ignore CTAD placeholders.

It also seems questionable that check_auto_in_tmpl_args is looking into 
non-type arguments, which won't have the bad autos this is looking for.

> However, it seems we don't need to call check_auto_in_tmpl_args at all
> outside of the concepts TS since using 'auto' as a type-id is otherwise
> rejected more generally at parse time.  So this patch guards calls to
> check_auto_in_tmpl_args with flag_concepts_ts instead of flag_concepts.
> 
> Relatedly, I think the concepts code paths in do_auto_deduction and
> type_uses_auto are also necessary only for the concepts TS, so this
> patch also restricts these code paths accordingly.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk and perhaps 11?

For 11 (and possibly trunk) maybe return false from check_auto... if 
!flag_concepts_ts rather than asserting and changing the call sites. 
That one change is OK for 11, the whole patch is OK for trunk.

The comment on the test or assert could be elaborated to explain as you 
do above that any bad autos will have been rejected already by the parser.

> 	PR c++/102933
> 
> gcc/cp/ChangeLog:
> 
> 	* parser.c (cp_parser_template_id): Call check_auto_in_tmpl_args
> 	only for the concepts TS not also for standard concepts.
> 	(cp_parser_simple_type_specifier): Adjust diagnostic for using
> 	auto in parameter declaration.
> 	* pt.c (tsubst_qualified_id): Call check_auto_in_tmpl_args only
> 	for the concepts TS not also for standard concepts.
> 	(extract_autos_r): Ignore CTAD placeholders.
> 	(extract_autos): Use range-based for.
> 	(do_auto_deduction): Use extract_autos only for the concepts TS
> 	and not also for standard concepts.
> 	(type_uses_auto): Likewise with for_each_template_parm.
> 	(check_auto_in_tmpl_args): Assert that this function is used only
> 	for the concepts tS.  Simplify.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp2a/nontype-class50.C: New test.
> 	* g++.dg/cpp2a/nontype-class50a.C: New test.
> ---
>   gcc/cp/parser.c                               |  4 ++--
>   gcc/cp/pt.c                                   | 24 +++++++++----------
>   gcc/testsuite/g++.dg/cpp2a/nontype-class50.C  | 13 ++++++++++
>   gcc/testsuite/g++.dg/cpp2a/nontype-class50a.C |  5 ++++
>   4 files changed, 32 insertions(+), 14 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-class50.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-class50a.C
> 
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 49d951cfb19..5052f534d40 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -18168,7 +18168,7 @@ cp_parser_template_id (cp_parser *parser,
>        types.  We reject them in functions, but if what we have is an
>        identifier, even with none_type we can't conclude it's NOT a
>        type, we have to wait for template substitution.  */
> -  if (flag_concepts && check_auto_in_tmpl_args (templ, arguments))
> +  if (flag_concepts_ts && check_auto_in_tmpl_args (templ, arguments))
>       template_id = error_mark_node;
>     /* Build a representation of the specialization.  */
>     else if (identifier_p (templ))
> @@ -19505,7 +19505,7 @@ cp_parser_simple_type_specifier (cp_parser* parser,
>   	  else if (!flag_concepts)
>   	    pedwarn (token->location, 0,
>   		     "use of %<auto%> in parameter declaration "
> -		     "only available with %<-fconcepts-ts%>");
> +		     "only available with %<-std=c++20%> or %<-fconcepts%>");
>   	}
>         else
>   	type = make_auto ();
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index 287cf4ce9d0..3321601e6ff 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -16456,7 +16456,7 @@ tsubst_qualified_id (tree qualified_id, tree args,
>   	 we want to catch is when we couldn't tell then, and can now,
>   	 namely when templ prior to substitution was an
>   	 identifier.  */
> -      if (flag_concepts && check_auto_in_tmpl_args (expr, template_args))
> +      if (flag_concepts_ts && check_auto_in_tmpl_args (expr, template_args))
>   	return error_mark_node;
>   
>         if (variable_template_p (expr))
> @@ -28560,7 +28560,7 @@ static int
>   extract_autos_r (tree t, void *data)
>   {
>     hash_table<auto_hash> &hash = *(hash_table<auto_hash>*)data;
> -  if (is_auto (t))
> +  if (is_auto (t) && !template_placeholder_p (t))
>       {
>         /* All the autos were built with index 0; fix that up now.  */
>         tree *p = hash.find_slot (t, INSERT);
> @@ -28594,10 +28594,8 @@ extract_autos (tree type)
>     for_each_template_parm (type, extract_autos_r, &hash, &visited, true);
>   
>     tree tree_vec = make_tree_vec (hash.elements());
> -  for (hash_table<auto_hash>::iterator iter = hash.begin();
> -       iter != hash.end(); ++iter)
> +  for (tree elt : hash)
>       {
> -      tree elt = *iter;
>         unsigned i = TEMPLATE_PARM_IDX (TEMPLATE_TYPE_PARM_INDEX (elt));
>         TREE_VEC_ELT (tree_vec, i)
>   	= build_tree_list (NULL_TREE, TYPE_NAME (elt));
> @@ -29837,7 +29835,7 @@ do_auto_deduction (tree type, tree init, tree auto_node,
>         tree parms = build_tree_list (NULL_TREE, type);
>         tree tparms;
>   
> -      if (flag_concepts)
> +      if (flag_concepts_ts)
>   	tparms = extract_autos (type);
>         else
>   	{
> @@ -30025,7 +30023,7 @@ type_uses_auto (tree type)
>   {
>     if (type == NULL_TREE)
>       return NULL_TREE;
> -  else if (flag_concepts)
> +  else if (flag_concepts_ts)
>       {
>         /* The Concepts TS allows multiple autos in one type-specifier; just
>   	 return the first one we find, do_auto_deduction will collect all of
> @@ -30048,6 +30046,9 @@ type_uses_auto (tree type)
>   bool
>   check_auto_in_tmpl_args (tree tmpl, tree args)
>   {
> +  /* Only the concepts TS allows 'auto' as a type-id.  */
> +  gcc_assert (flag_concepts_ts);
> +
>     /* If there were previous errors, nevermind.  */
>     if (!args || TREE_CODE (args) != TREE_VEC)
>       return false;
> @@ -30057,11 +30058,10 @@ check_auto_in_tmpl_args (tree tmpl, tree args)
>        We'll only be able to tell during template substitution, so we
>        expect to be called again then.  If concepts are enabled and we
>        know we have a type, we're ok.  */
> -  if (flag_concepts
> -      && (identifier_p (tmpl)
> -	  || (DECL_P (tmpl)
> -	      &&  (DECL_TYPE_TEMPLATE_P (tmpl)
> -		   || DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl)))))
> +  if (identifier_p (tmpl)
> +      || (DECL_P (tmpl)
> +	  &&  (DECL_TYPE_TEMPLATE_P (tmpl)
> +	       || DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl))))
>       return false;
>   
>     /* Quickly search for any occurrences of auto; usually there won't
> diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-class50.C b/gcc/testsuite/g++.dg/cpp2a/nontype-class50.C
> new file mode 100644
> index 00000000000..1c2786a59e7
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/nontype-class50.C
> @@ -0,0 +1,13 @@
> +// PR c++/102933
> +// { dg-do compile { target c++20 } }
> +
> +template<class T> struct X { T t; };
> +
> +template<X> void f();
> +
> +template<class T>
> +void g() {
> +  f<X{T{0}}>();
> +}
> +
> +template void g<int>();
> diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-class50a.C b/gcc/testsuite/g++.dg/cpp2a/nontype-class50a.C
> new file mode 100644
> index 00000000000..eb8a6ad9375
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/nontype-class50a.C
> @@ -0,0 +1,5 @@
> +// PR c++/102933
> +// { dg-do compile { target c++20 } }
> +// { dg-additional-options "-fconcepts-ts" }
> +
> +#include "nontype-class50.C"
>

On Wed, 27 Oct 2021, Jason Merrill wrote:

> On 10/26/21 13:44, Patrick Palka wrote:
> > Here when checking for erroneous occurrences of 'auto' inside a template
> > argument (which is allowed by the concepts TS for class templates),
> > extract_autos_r picks up the CTAD placeholder for X{T{0}} which causes
> > check_auto_in_tmpl_args to reject this valid template argument.  This
> > patch fixes this by making extract_autos_r ignore CTAD placeholders.
> 
> It also seems questionable that check_auto_in_tmpl_args is looking into
> non-type arguments, which won't have the bad autos this is looking for.

Ah yeah, interesting.  Somehow this doesn't cause problems when passing
an auto NTTP as a template argument.

> 
> > However, it seems we don't need to call check_auto_in_tmpl_args at all
> > outside of the concepts TS since using 'auto' as a type-id is otherwise
> > rejected more generally at parse time.  So this patch guards calls to
> > check_auto_in_tmpl_args with flag_concepts_ts instead of flag_concepts.
> > 
> > Relatedly, I think the concepts code paths in do_auto_deduction and
> > type_uses_auto are also necessary only for the concepts TS, so this
> > patch also restricts these code paths accordingly.
> > 
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > trunk and perhaps 11?
> 
> For 11 (and possibly trunk) maybe return false from check_auto... if
> !flag_concepts_ts rather than asserting and changing the call sites. That one
> change is OK for 11, the whole patch is OK for trunk.

Done, thanks a lot.

> 
> The comment on the test or assert could be elaborated to explain as you do
> above that any bad autos will have been rejected already by the parser.

Done, here's what I ended up committing to trunk so far:

-- >8 --

Subject: [PATCH] c++: CTAD within template argument [PR102933]

Here when checking for erroneous occurrences of 'auto' inside a template
argument (which is allowed by the concepts TS for class templates),
extract_autos_r picks up the CTAD placeholder for X{T{0}} which causes
check_auto_in_tmpl_args to reject this valid template argument.  This
patch fixes this by making extract_autos_r ignore CTAD placeholders.

However, it seems we don't need to call check_auto_in_tmpl_args at all
outside of the concepts TS since using 'auto' as a type-id is otherwise
rejected more generally at parse time.  So this patch makes the function
just exit early if !flag_concepts_ts.

Similarly, I think the concepts code paths in do_auto_deduction and
type_uses_auto are only necessary for the concepts TS, so this patch
also restricts these code paths accordingly.

	PR c++/102933

gcc/cp/ChangeLog:

	* parser.c (cp_parser_simple_type_specifier): Adjust diagnostic
	for using auto in parameter declaration.
	* pt.c (extract_autos_r): Ignore CTAD placeholders.
	(extract_autos): Use range-based for.
	(do_auto_deduction): Use extract_autos only for the concepts TS
	and not also for standard concepts.
	(type_uses_auto): Likewise with for_each_template_parm.
	(check_auto_in_tmpl_args): Just return false outside of the
	concepts TS.  Simplify.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/nontype-class50.C: New test.
	* g++.dg/cpp2a/nontype-class50a.C: New test.
---
 gcc/cp/parser.c                               |  2 +-
 gcc/cp/pt.c                                   | 24 ++++++++++---------
 gcc/testsuite/g++.dg/cpp2a/nontype-class50.C  | 13 ++++++++++
 gcc/testsuite/g++.dg/cpp2a/nontype-class50a.C |  5 ++++
 4 files changed, 32 insertions(+), 12 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-class50.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-class50a.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 93335c817d7..4c2075742d6 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -19513,7 +19513,7 @@ cp_parser_simple_type_specifier (cp_parser* parser,
 	  else if (!flag_concepts)
 	    pedwarn (token->location, 0,
 		     "use of %<auto%> in parameter declaration "
-		     "only available with %<-fconcepts-ts%>");
+		     "only available with %<-std=c++20%> or %<-fconcepts%>");
 	}
       else
 	type = make_auto ();
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 287cf4ce9d0..66040035b2f 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -28560,7 +28560,7 @@ static int
 extract_autos_r (tree t, void *data)
 {
   hash_table<auto_hash> &hash = *(hash_table<auto_hash>*)data;
-  if (is_auto (t))
+  if (is_auto (t) && !template_placeholder_p (t))
     {
       /* All the autos were built with index 0; fix that up now.  */
       tree *p = hash.find_slot (t, INSERT);
@@ -28594,10 +28594,8 @@ extract_autos (tree type)
   for_each_template_parm (type, extract_autos_r, &hash, &visited, true);
 
   tree tree_vec = make_tree_vec (hash.elements());
-  for (hash_table<auto_hash>::iterator iter = hash.begin();
-       iter != hash.end(); ++iter)
+  for (tree elt : hash)
     {
-      tree elt = *iter;
       unsigned i = TEMPLATE_PARM_IDX (TEMPLATE_TYPE_PARM_INDEX (elt));
       TREE_VEC_ELT (tree_vec, i)
 	= build_tree_list (NULL_TREE, TYPE_NAME (elt));
@@ -29837,7 +29835,7 @@ do_auto_deduction (tree type, tree init, tree auto_node,
       tree parms = build_tree_list (NULL_TREE, type);
       tree tparms;
 
-      if (flag_concepts)
+      if (flag_concepts_ts)
 	tparms = extract_autos (type);
       else
 	{
@@ -30025,7 +30023,7 @@ type_uses_auto (tree type)
 {
   if (type == NULL_TREE)
     return NULL_TREE;
-  else if (flag_concepts)
+  else if (flag_concepts_ts)
     {
       /* The Concepts TS allows multiple autos in one type-specifier; just
 	 return the first one we find, do_auto_deduction will collect all of
@@ -30048,6 +30046,11 @@ type_uses_auto (tree type)
 bool
 check_auto_in_tmpl_args (tree tmpl, tree args)
 {
+  if (!flag_concepts_ts)
+    /* Only the concepts TS allows 'auto' as a type-id; it'd otherwise
+       have already been rejected by the parser more generally.  */
+    return false;
+
   /* If there were previous errors, nevermind.  */
   if (!args || TREE_CODE (args) != TREE_VEC)
     return false;
@@ -30057,11 +30060,10 @@ check_auto_in_tmpl_args (tree tmpl, tree args)
      We'll only be able to tell during template substitution, so we
      expect to be called again then.  If concepts are enabled and we
      know we have a type, we're ok.  */
-  if (flag_concepts
-      && (identifier_p (tmpl)
-	  || (DECL_P (tmpl)
-	      &&  (DECL_TYPE_TEMPLATE_P (tmpl)
-		   || DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl)))))
+  if (identifier_p (tmpl)
+      || (DECL_P (tmpl)
+	  &&  (DECL_TYPE_TEMPLATE_P (tmpl)
+	       || DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl))))
     return false;
 
   /* Quickly search for any occurrences of auto; usually there won't
diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-class50.C b/gcc/testsuite/g++.dg/cpp2a/nontype-class50.C
new file mode 100644
index 00000000000..1c2786a59e7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/nontype-class50.C
@@ -0,0 +1,13 @@
+// PR c++/102933
+// { dg-do compile { target c++20 } }
+
+template<class T> struct X { T t; };
+
+template<X> void f();
+
+template<class T>
+void g() {
+  f<X{T{0}}>();
+}
+
+template void g<int>();
diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-class50a.C b/gcc/testsuite/g++.dg/cpp2a/nontype-class50a.C
new file mode 100644
index 00000000000..eb8a6ad9375
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/nontype-class50a.C
@@ -0,0 +1,5 @@
+// PR c++/102933
+// { dg-do compile { target c++20 } }
+// { dg-additional-options "-fconcepts-ts" }
+
+#include "nontype-class50.C"