Fix auto deduction for template specialization scopes [114915].

Message ID 20240501225217.59069-1-sska1377@gmail.com
State New
Headers
Series Fix auto deduction for template specialization scopes [114915]. |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Testing passed

Commit Message

Seyed Sajad Kahani May 1, 2024, 10:52 p.m. UTC
  When deducing auto for `adc_return_type`, `adc_variable_type`, and `adc_decomp_type` contexts (at the usage time), we try to resolve the outermost template arguments to be used for satisfaction. This is done by one of the following, depending on the scope:

1. Checking the `DECL_TEMPLATE_INFO` of the current function scope and extracting DECL_TI_ARGS from it for function scope deductions (pt.cc:31236).
2. Checking the `DECL_TEMPLATE_INFO` of the declaration (alongside with other conditions) for non-function scope variable declaration deductions (decl.cc:8527).

Then, we do not retrieve the deeper layers of the template arguments; instead, we fill the missing levels with dummy levels (pt.cc:31260).

The problem (that is shown in PR114915) is that we do not consider the case where the deduction happens in a template specialization scope. In this case, the type is not dependent on the outermost template arguments (which are the specialization arguments). Yet, we still resolve the outermost template arguments, and then the number of layers in the template arguments exceeds the number of levels in the type. This causes the missing levels to be negative. This leads to the rejection of valid code and ICEs (like segfault) in the release mode. In the debug mode, it is possible to show as an assertion failure (when creating a tree_vec with a negative size).
The code that generates the issue is added to the test suite as `g++.dg/cpp2a/concepts-placeholder14.C`.

This patch fixes the issue by checking that the template usage, whose arguments are going to be used for satisfaction, is not a partial or explicit specialization (and therefore it is an implicit or explicit instantiation). This check is done in the two only places that affect the `outer_targs` for the mentioned contexts.

One might ask why this is not implemented as a simple `missing_level > 0` check. The reason is that the recovery from the negative `missing_levels` will not be easy, and it is not clear how to recover from it. Therefore, it is better to prevent it from happening.
---
 gcc/cp/decl.cc                                |  1 +
 gcc/cp/pt.cc                                  | 16 ++++++++++-----
 .../g++.dg/cpp2a/concepts-placeholder14.C     | 20 +++++++++++++++++++
 3 files changed, 32 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-placeholder14.C
  

Comments

Patrick Palka May 3, 2024, 4:24 p.m. UTC | #1
Thanks for the patch!

On Wed, 1 May 2024, Seyed Sajad Kahani wrote:

> When deducing auto for `adc_return_type`, `adc_variable_type`, and `adc_decomp_type` contexts (at the usage time), we try to resolve the outermost template arguments to be used for satisfaction. This is done by one of the following, depending on the scope:
> 
> 1. Checking the `DECL_TEMPLATE_INFO` of the current function scope and extracting DECL_TI_ARGS from it for function scope deductions (pt.cc:31236).
> 2. Checking the `DECL_TEMPLATE_INFO` of the declaration (alongside with other conditions) for non-function scope variable declaration deductions (decl.cc:8527).
> 
> Then, we do not retrieve the deeper layers of the template arguments; instead, we fill the missing levels with dummy levels (pt.cc:31260).
> 
> The problem (that is shown in PR114915) is that we do not consider the case where the deduction happens in a template specialization scope. In this case, the type is not dependent on the outermost template arguments (which are the specialization arguments). Yet, we still resolve the outermost template arguments, and then the number of layers in the template arguments exceeds the number of levels in the type. This causes the missing levels to be negative. This leads to the rejection of valid code and ICEs (like segfault) in the release mode. In the debug mode, it is possible to show as an assertion failure (when creating a tree_vec with a negative size).
> The code that generates the issue is added to the test suite as `g++.dg/cpp2a/concepts-placeholder14.C`.
> 
> This patch fixes the issue by checking that the template usage, whose arguments are going to be used for satisfaction, is not a partial or explicit specialization (and therefore it is an implicit or explicit instantiation). This check is done in the two only places that affect the `outer_targs` for the mentioned contexts.
> 
> One might ask why this is not implemented as a simple `missing_level > 0` check. The reason is that the recovery from the negative `missing_levels` will not be easy, and it is not clear how to recover from it. Therefore, it is better to prevent it from happening.
> ---
>  gcc/cp/decl.cc                                |  1 +
>  gcc/cp/pt.cc                                  | 16 ++++++++++-----
>  .../g++.dg/cpp2a/concepts-placeholder14.C     | 20 +++++++++++++++++++
>  3 files changed, 32 insertions(+), 5 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-placeholder14.C
> 
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index 65ab64885..7e51f926e 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -8527,6 +8527,7 @@ cp_finish_decl (tree decl, tree init, bool init_const_expr_p,
>        if (PLACEHOLDER_TYPE_CONSTRAINTS_INFO (auto_node)
>  	  && DECL_LANG_SPECIFIC (decl)
>  	  && DECL_TEMPLATE_INFO (decl)
> +	  && DECL_USE_TEMPLATE (decl) != 2

We should check !DECL_TEMPLATE_SPECIALIZATION instead of checking
DECL_USE_TEMPLATE directly.

And since DECL_TEMPLATE_SPECIALIZATION is true for both partial and
explicit specializations, I think we should check !in_template_context
as well in order to single out explicit specializations?

>  	  && !DECL_FUNCTION_SCOPE_P (decl))
>  	/* The outer template arguments might be needed for satisfaction.
>  	   (For function scope variables, do_auto_deduction will obtain the
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 3b2106dd3..fd646d873 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -31044,7 +31044,8 @@ unparenthesized_id_or_class_member_access_p (tree init)
>     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
>     adc_requirement contexts to communicate the necessary template arguments
> -   to satisfaction.  OUTER_TARGS is ignored in other contexts.
> +   to satisfaction.  OUTER_TARGS will be used for other contexts if it is a
> +   function scope deduction. Otherwise it is ignored.
>  
>     Additionally for adc_unify contexts TMPL is the template for which TYPE
>     is a template parameter type.
> @@ -31235,8 +31236,11 @@ do_auto_deduction (tree type, tree init, tree auto_node,
>  	if (tree fn = current_function_decl)
>  	  if (DECL_TEMPLATE_INFO (fn) || LAMBDA_FUNCTION_P (fn))
>  	    {
> -	      outer_targs = DECL_TEMPLATE_INFO (fn)
> -		? DECL_TI_ARGS (fn) : NULL_TREE;
> +	      outer_targs = NULL_TREE; 
> +	      if (DECL_TEMPLATE_INFO (fn) && DECL_USE_TEMPLATE (fn) != 2)
> +	      {
> +		  outer_targs = DECL_TI_ARGS (fn);
> +	      }

Same here.

>  	      if (LAMBDA_FUNCTION_P (fn))
>  		{
>  		  /* As in satisfy_declaration_constraints.  */
> @@ -31260,8 +31264,10 @@ do_auto_deduction (tree type, tree init, tree auto_node,
>  	 these missing levels, but this hack otherwise allows us to handle a
>  	 large subset of possible constraints (including all non-dependent
>  	 constraints).  */
> -      if (int missing_levels = (TEMPLATE_TYPE_ORIG_LEVEL (auto_node)
> -				- TMPL_ARGS_DEPTH (full_targs)))
> +      int missing_levels = (TEMPLATE_TYPE_ORIG_LEVEL (auto_node)
> +				    - TMPL_ARGS_DEPTH (full_targs));
> +
> +      if (missing_levels > 0)

IIUC this change is not necessary with the above changes, right?  Maybe
we could assert than missing_levels is nonnegative.

Another more context-unaware approach to fix this might be to only
use the innermost level from 'full_targs' for satisfaction if
TEMPLATE_TYPE_ORIG_LEVEL is 1 (which means this constrained auto
appeared in a context that doesn't have template parameters such as an
explicit specialization or ordinary non-template function, and so
only the level corresponding to the deduced type is needed for
satisfaction.)

Generalizing on that, another approach might be to handle missing_levels < 0
by removing -missing_levels from full_targs via get_innermost_template_args.
But AFAICT it should suffice to handle the TEMPLATE_TYPE_ORIG_LEVEL == 1
case specifically.

I wonder what Jason thinks?

>  	{
>  	  tree dummy_levels = make_tree_vec (missing_levels);
>  	  for (int i = 0; i < missing_levels; ++i)
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-placeholder14.C b/gcc/testsuite/g++.dg/cpp2a/concepts-placeholder14.C
> new file mode 100644
> index 000000000..4a98ec7b3
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-placeholder14.C
> @@ -0,0 +1,20 @@
> +// PR c++/114915
> +// { dg-do compile { target c++20 } }
> +
> +template<typename T>
> +concept TheConcept = __is_same(T, int);
> +
> +template<typename T>
> +void f() {
> +  TheConcept auto x = 1;
> +}
> +
> +template<>
> +void f<int>() {
> +  TheConcept auto x = 1;
> +}
> +
> +int main() {
> +  f<int>();
> +  return 0;
> +}

It would be good to also test an explicit variable tmpl spec and
an explicit spec of a member template of a class template.

> -- 
> 2.44.0
> 
>
  
Seyed Sajad Kahani May 4, 2024, 2:11 p.m. UTC | #2
Thanks for your helpful feedback. It has totally shaped my understanding.

While I was trying to develop other tests, as you suggested:

> It would be good to also test an explicit variable tmpl spec and
> an explicit spec of a member template of a class template.

I realized that for the case where we have a member function template
of a class template, and a specialization of the enclosing class only
(like below),

template <>
template <typename U>
void S<int>::f() {
  // some constrained auto
}

When using S<int>::f<double>, DECL_TEMPLATE_INFO(fn) is non-zero, and
DECL_TEMPLATE_SPECIALIZATION(fn) is zero, while
DECL_TEMPLATE_SPECIALIZATION(DECL_TI_TEMPLATE(fn)) is non-zero.
So it means that the patch will extract DECL_TI_ARGS(fn) as
outer_targs, and it would be <int> <double> while the type of the
constrained auto will be as template <typename U> ... and will not be
dependent on the parameters of the enclosing class.
This means that again (outer_targs + targs) will have more depth than
auto_node levels.
This means that for the case where the function is not an explicit
specialization, but it is defined in an explicitly specialized scope,
the patch will not work.

I have thought of two ideas to fully solve the problem:

1. Trimming the full_targs by - missing_level, as you have mentioned.
2. Traversing the TI_TEMPLATE associated with different levels of
outer_targs, finding the first one that is
DECL_TEMPLATE_SPECIALIZATION, then trimming outer_targs by that point.

For the first idea,

> Another more context-unaware approach to fix this might be to only
> use the innermost level from 'full_targs' for satisfaction if
> TEMPLATE_TYPE_ORIG_LEVEL is 1 (which means this constrained auto
> appeared in a context that doesn't have template parameters such as an
> explicit specialization or ordinary non-template function, and so
> only the level corresponding to the deduced type is needed for
> satisfaction.)
>
> Generalizing on that, another approach might be to handle missing_levels < 0
> by removing -missing_levels from full_targs via get_innermost_template_args.
> But AFAICT it should suffice to handle the TEMPLATE_TYPE_ORIG_LEVEL == 1
> case specifically.

I was unable to understand why you think that it might not handle
TEMPLATE_TYPE_ORIG_LEVEL > 1 cases, so I tried to formulate my
reasoning as follows.

Assuming contexts adc_variable_type, adc_return_type, adc_decomp_type:
For any case where missing_level < 0, it means that the type depends
on fewer levels than the template arguments used to materialize it.
This can only happen when the type is defined in an explicit
specialization scope. This explicit specialization might not occur in
its immediate scope.
Note that partial specialization (while changing the set of
parameters) cannot reduce the number of levels for the type.
Because of the fact that the enclosing scope of any explicit
specialization is explicitly specialized
(https://eel.is/c++draft/temp.expl.spec#16), the type will not depend
on template parameters outside of its innermost explicit specialized
scope.
Assuming that there are no real missing levels, by removing those
levels, missing_level should be = 0. As a result, by roughly doing

if (missing_levels < 0) {
  tree trimmed_full_args = get_innermost_template_args(full_targs,
TEMPLATE_TYPE_ORIG_LEVEL(auto_node));
  full_targs = trimmed_full_args;
}
in pt.cc:31262, where we calculate and check missing_levels, we would
be able to fix the errors.
Note that, for the case where there are real missing levels, we are
putting irrelevant template arguments for the missing levels instead
of make_tree_vec(0). By this change:
- If the type is independent of those missing levels: it works fine either way.
- If the type is dependent on those missing levels: Instead of raising
an ICE, the compiler exhibits undefined behavior.

Now, for the second idea, we need to do something like

int meaningful_levels = 0;
for (tree sc = fn;
     DECL_TEMPLATE_INFO(sc) && !DECL_TEMPLATE_SPECIALIZATION(sc);
     sc = DECL_TI_TEMPLATE(sc))
  meaningful_levels ++;
outer_targs = get_innermost_template_args(DECL_TI_ARGS(fn), meaningful_levels);

in pt.cc:31238, instead of assigning DECL_TI_ARGS(fn) to outer_targs directly.
Note that we need to do the same thing in decl:8528 and maybe other places.

I would really appreciate your comments on these two ideas.
I will send another patch, applying the first idea in reply to this thread.
  

Patch

diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 65ab64885..7e51f926e 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -8527,6 +8527,7 @@  cp_finish_decl (tree decl, tree init, bool init_const_expr_p,
       if (PLACEHOLDER_TYPE_CONSTRAINTS_INFO (auto_node)
 	  && DECL_LANG_SPECIFIC (decl)
 	  && DECL_TEMPLATE_INFO (decl)
+	  && DECL_USE_TEMPLATE (decl) != 2
 	  && !DECL_FUNCTION_SCOPE_P (decl))
 	/* The outer template arguments might be needed for satisfaction.
 	   (For function scope variables, do_auto_deduction will obtain the
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 3b2106dd3..fd646d873 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -31044,7 +31044,8 @@  unparenthesized_id_or_class_member_access_p (tree init)
    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
    adc_requirement contexts to communicate the necessary template arguments
-   to satisfaction.  OUTER_TARGS is ignored in other contexts.
+   to satisfaction.  OUTER_TARGS will be used for other contexts if it is a
+   function scope deduction. Otherwise it is ignored.
 
    Additionally for adc_unify contexts TMPL is the template for which TYPE
    is a template parameter type.
@@ -31235,8 +31236,11 @@  do_auto_deduction (tree type, tree init, tree auto_node,
 	if (tree fn = current_function_decl)
 	  if (DECL_TEMPLATE_INFO (fn) || LAMBDA_FUNCTION_P (fn))
 	    {
-	      outer_targs = DECL_TEMPLATE_INFO (fn)
-		? DECL_TI_ARGS (fn) : NULL_TREE;
+	      outer_targs = NULL_TREE; 
+	      if (DECL_TEMPLATE_INFO (fn) && DECL_USE_TEMPLATE (fn) != 2)
+	      {
+		  outer_targs = DECL_TI_ARGS (fn);
+	      }
 	      if (LAMBDA_FUNCTION_P (fn))
 		{
 		  /* As in satisfy_declaration_constraints.  */
@@ -31260,8 +31264,10 @@  do_auto_deduction (tree type, tree init, tree auto_node,
 	 these missing levels, but this hack otherwise allows us to handle a
 	 large subset of possible constraints (including all non-dependent
 	 constraints).  */
-      if (int missing_levels = (TEMPLATE_TYPE_ORIG_LEVEL (auto_node)
-				- TMPL_ARGS_DEPTH (full_targs)))
+      int missing_levels = (TEMPLATE_TYPE_ORIG_LEVEL (auto_node)
+				    - TMPL_ARGS_DEPTH (full_targs));
+
+      if (missing_levels > 0)
 	{
 	  tree dummy_levels = make_tree_vec (missing_levels);
 	  for (int i = 0; i < missing_levels; ++i)
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-placeholder14.C b/gcc/testsuite/g++.dg/cpp2a/concepts-placeholder14.C
new file mode 100644
index 000000000..4a98ec7b3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-placeholder14.C
@@ -0,0 +1,20 @@ 
+// PR c++/114915
+// { dg-do compile { target c++20 } }
+
+template<typename T>
+concept TheConcept = __is_same(T, int);
+
+template<typename T>
+void f() {
+  TheConcept auto x = 1;
+}
+
+template<>
+void f<int>() {
+  TheConcept auto x = 1;
+}
+
+int main() {
+  f<int>();
+  return 0;
+}