c++: Fix class NTTP with template arguments [PR92948]

Message ID 20200128233440.1538083-1-polacek@redhat.com
State New
Headers show
Series
  • c++: Fix class NTTP with template arguments [PR92948]
Related show

Commit Message

Marek Polacek Jan. 28, 2020, 11:34 p.m.
This PR points out an ICE with an alias template and class NTTP, but I
found that there are more issues.  Trouble arise when we use a
(non-type) template parameter as an argument to the template arg list of
a template that accepts a class NTTP and a conversion to a class is
involved, e.g.

  struct A { A(int) { } };
  template<A a> struct B { };

  template<int X> void fn () {
      B<X> b;
  }

Normally for such a conversion we create a TARGET_EXPR with an
AGGR_INIT_EXPR that calls a __ct_comp with some arguments, but not in
this case: when converting X to type A in convert_nontype_argument we
are in a template and the template parameter 'X' is value-dependent, and
AGGR_INIT_EXPR don't work in templates.  So it just creates a TARGET_EXPR
that contains "A::A(*this, X)", but with that overload resolution fails:
  error: no matching function for call to 'A::A(A*, int)'
That happens because finish_call_expr for a BASELINK creates a dummy
object, so we have 'this' twice.  I thought for the value-dependent case
we could use IMPLICIT_CONV_EXPR, as in the patch below.  Note that I
only do this when we convert to a different type than the type of the
expr.  The point is to avoid the call to a converting ctor.

The second issue was an ICE in tsubst_copy: when there's a conversion
like the above involved then
 /* Wrapper to make a C++20 template parameter object const.  */
  op = tsubst_copy (op, args, complain, in_decl);
might not produce a _ZT... VAR_DECL with const type, so the assert
  gcc_assert (CP_TYPE_CONST_P (TREE_TYPE (op)));
fails.  Allowing IMPLICIT_CONV_EXPR there probably makes sense.

And since convert_nontype_argument uses value_dependent_expression_p
a lot, I used a dedicated bool to speed things up.

Bootstrapped/regtested on x86_64-linux, ok for trunk and maybe 9?

2020-01-28  Marek Polacek  <polacek@redhat.com>

	PR c++/92948 - Fix class NTTP with template arguments.
	* pt.c (convert_nontype_argument): Use IMPLICIT_CONV_EXPR when
	converting a value-dependent expression to a class type.
	(tsubst_copy) <case VIEW_CONVERT_EXPR>: Allow IMPLICIT_CONV_EXPR
	as the result of the tsubst_copy call.

	* g++.dg/cpp2a/nontype-class28.C: New test.
	* g++.dg/cpp2a/nontype-class29.C: New test.
---
 gcc/cp/pt.c                                  | 36 +++++++++++--------
 gcc/testsuite/g++.dg/cpp2a/nontype-class28.C | 37 ++++++++++++++++++++
 gcc/testsuite/g++.dg/cpp2a/nontype-class29.C | 26 ++++++++++++++
 3 files changed, 85 insertions(+), 14 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-class28.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-class29.C


base-commit: ad690d79cfbb905c5546c9333c5fd089d906505b
-- 
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA

Comments

Jason Merrill Jan. 29, 2020, 6:11 p.m. | #1
On 1/28/20 6:34 PM, Marek Polacek wrote:
> This PR points out an ICE with an alias template and class NTTP, but I

> found that there are more issues.  Trouble arise when we use a

> (non-type) template parameter as an argument to the template arg list of

> a template that accepts a class NTTP and a conversion to a class is

> involved, e.g.

> 

>    struct A { A(int) { } };

>    template<A a> struct B { };

> 

>    template<int X> void fn () {

>        B<X> b;

>    }

> 

> Normally for such a conversion we create a TARGET_EXPR with an

> AGGR_INIT_EXPR that calls a __ct_comp with some arguments, but not in

> this case: when converting X to type A in convert_nontype_argument we

> are in a template and the template parameter 'X' is value-dependent, and

> AGGR_INIT_EXPR don't work in templates.  So it just creates a TARGET_EXPR

> that contains "A::A(*this, X)", but with that overload resolution fails:

>    error: no matching function for call to 'A::A(A*, int)'

> That happens because finish_call_expr for a BASELINK creates a dummy

> object, so we have 'this' twice.  I thought for the value-dependent case

> we could use IMPLICIT_CONV_EXPR, as in the patch below.  Note that I

> only do this when we convert to a different type than the type of the

> expr.  The point is to avoid the call to a converting ctor.

> 

> The second issue was an ICE in tsubst_copy: when there's a conversion

> like the above involved then

>   /* Wrapper to make a C++20 template parameter object const.  */

>    op = tsubst_copy (op, args, complain, in_decl);

> might not produce a _ZT... VAR_DECL with const type, so the assert

>    gcc_assert (CP_TYPE_CONST_P (TREE_TYPE (op)));

> fails.  Allowing IMPLICIT_CONV_EXPR there probably makes sense.

> 

> And since convert_nontype_argument uses value_dependent_expression_p

> a lot, I used a dedicated bool to speed things up.

> 

> Bootstrapped/regtested on x86_64-linux, ok for trunk and maybe 9?


OK for both.

The const/non-const issue with class NTTP is a pain, I'm wondering if it 
would have been better to make the template parameter const and deal 
with hiding the const where needed, or if that would be a similar pain 
in a different set of places.

> 2020-01-28  Marek Polacek  <polacek@redhat.com>

> 

> 	PR c++/92948 - Fix class NTTP with template arguments.

> 	* pt.c (convert_nontype_argument): Use IMPLICIT_CONV_EXPR when

> 	converting a value-dependent expression to a class type.

> 	(tsubst_copy) <case VIEW_CONVERT_EXPR>: Allow IMPLICIT_CONV_EXPR

> 	as the result of the tsubst_copy call.

> 

> 	* g++.dg/cpp2a/nontype-class28.C: New test.

> 	* g++.dg/cpp2a/nontype-class29.C: New test.

> ---

>   gcc/cp/pt.c                                  | 36 +++++++++++--------

>   gcc/testsuite/g++.dg/cpp2a/nontype-class28.C | 37 ++++++++++++++++++++

>   gcc/testsuite/g++.dg/cpp2a/nontype-class29.C | 26 ++++++++++++++

>   3 files changed, 85 insertions(+), 14 deletions(-)

>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-class28.C

>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-class29.C

> 

> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c

> index e889c800aca..fccf4e95453 100644

> --- a/gcc/cp/pt.c

> +++ b/gcc/cp/pt.c

> @@ -7074,7 +7074,8 @@ convert_nontype_argument (tree type, tree expr, tsubst_flags_t complain)

>     if (non_dep)

>       expr = instantiate_non_dependent_expr_internal (expr, complain);

>   

> -  if (value_dependent_expression_p (expr))

> +  const bool val_dep_p = value_dependent_expression_p (expr);

> +  if (val_dep_p)

>       expr = canonicalize_expr_argument (expr, complain);

>   

>     /* 14.3.2/5: The null pointer{,-to-member} conversion is applied

> @@ -7100,10 +7101,17 @@ convert_nontype_argument (tree type, tree expr, tsubst_flags_t complain)

>   	     a conversion function with a value-dependent argument, which

>   	     could invoke taking the address of a temporary representing

>   	     the result of the conversion.  */

> -	  if (COMPOUND_LITERAL_P (expr)

> -	      && CONSTRUCTOR_IS_DEPENDENT (expr)

> -	      && MAYBE_CLASS_TYPE_P (expr_type)

> -	      && TYPE_HAS_CONVERSION (expr_type))

> +	  if (!same_type_ignoring_top_level_qualifiers_p (type, expr_type)

> +	      && ((COMPOUND_LITERAL_P (expr)

> +		   && CONSTRUCTOR_IS_DEPENDENT (expr)

> +		   && MAYBE_CLASS_TYPE_P (expr_type)

> +		   && TYPE_HAS_CONVERSION (expr_type))

> +		  /* Similarly, converting e.g. an integer to a class

> +		     involves a constructor call.  convert_like would

> +		     create a TARGET_EXPR, but in a template we can't

> +		     use AGGR_INIT_EXPR, and the TARGET_EXPR would lead

> +		     to a bogus error.  */

> +		  || (val_dep_p && MAYBE_CLASS_TYPE_P (type))))

>   	    {

>   	      expr = build1 (IMPLICIT_CONV_EXPR, type, expr);

>   	      IMPLICIT_CONV_EXPR_NONTYPE_ARG (expr) = true;

> @@ -7189,8 +7197,7 @@ convert_nontype_argument (tree type, tree expr, tsubst_flags_t complain)

>   

>         /* Notice that there are constant expressions like '4 % 0' which

>   	 do not fold into integer constants.  */

> -      if (TREE_CODE (expr) != INTEGER_CST

> -	  && !value_dependent_expression_p (expr))

> +      if (TREE_CODE (expr) != INTEGER_CST && !val_dep_p)

>   	{

>   	  if (complain & tf_error)

>   	    {

> @@ -7265,7 +7272,7 @@ convert_nontype_argument (tree type, tree expr, tsubst_flags_t complain)

>   	Here, we do not care about functions, as they are invalid anyway

>   	for a parameter of type pointer-to-object.  */

>   

> -      if (value_dependent_expression_p (expr))

> +      if (val_dep_p)

>   	/* Non-type template parameters are OK.  */

>   	;

>         else if (cxx_dialect >= cxx11 && integer_zerop (expr))

> @@ -7333,8 +7340,7 @@ convert_nontype_argument (tree type, tree expr, tsubst_flags_t complain)

>   	    }

>   	}

>   

> -      if (TYPE_REF_OBJ_P (TREE_TYPE (expr))

> -	  && value_dependent_expression_p (expr))

> +      if (TYPE_REF_OBJ_P (TREE_TYPE (expr)) && val_dep_p)

>   	/* OK, dependent reference.  We don't want to ask whether a DECL is

>   	   itself value-dependent, since what we want here is its address.  */;

>         else

> @@ -7412,7 +7418,7 @@ convert_nontype_argument (tree type, tree expr, tsubst_flags_t complain)

>   

>         /* [temp.arg.nontype] bullet 1 says the pointer to member

>            expression must be a pointer-to-member constant.  */

> -      if (!value_dependent_expression_p (expr)

> +      if (!val_dep_p

>   	  && !check_valid_ptrmem_cst_expr (type, expr, complain))

>   	return NULL_TREE;

>   

> @@ -7429,7 +7435,7 @@ convert_nontype_argument (tree type, tree expr, tsubst_flags_t complain)

>       {

>         /* [temp.arg.nontype] bullet 1 says the pointer to member

>            expression must be a pointer-to-member constant.  */

> -      if (!value_dependent_expression_p (expr)

> +      if (!val_dep_p

>   	  && !check_valid_ptrmem_cst_expr (type, expr, complain))

>   	return NULL_TREE;

>   

> @@ -7452,7 +7458,7 @@ convert_nontype_argument (tree type, tree expr, tsubst_flags_t complain)

>       {

>         /* Replace the argument with a reference to the corresponding template

>   	 parameter object.  */

> -      if (!value_dependent_expression_p (expr))

> +      if (!val_dep_p)

>   	expr = get_template_parm_object (expr, complain);

>         if (expr == error_mark_node)

>   	return NULL_TREE;

> @@ -16406,7 +16412,9 @@ tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl)

>   		}

>   	      else

>   		{

> -		  gcc_assert (CP_TYPE_CONST_P (TREE_TYPE (op)));

> +		  gcc_assert (CP_TYPE_CONST_P (TREE_TYPE (op))

> +			      || (TREE_CODE (op) == IMPLICIT_CONV_EXPR

> +				  && IMPLICIT_CONV_EXPR_NONTYPE_ARG (op)));

>   		  return op;

>   		}

>   	    }

> diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-class28.C b/gcc/testsuite/g++.dg/cpp2a/nontype-class28.C

> new file mode 100644

> index 00000000000..80b9b499355

> --- /dev/null

> +++ b/gcc/testsuite/g++.dg/cpp2a/nontype-class28.C

> @@ -0,0 +1,37 @@

> +// PR c++/92948 - Fix class NTTP with template arguments.

> +// { dg-do compile { target c++2a } }

> +

> +struct A {

> +  constexpr A(int) { }

> +};

> +

> +template<A>

> +struct B {

> +  using U = unsigned;

> +};

> +

> +template<A a>

> +using U = B<a>;

> +

> +template<int X, typename Y = typename B<X>::U>

> +void foo()

> +{

> +}

> +

> +template<int X, typename Y = typename U<X>::U>

> +void foo2()

> +{

> +}

> +

> +template<typename Y = typename B<1>::U>

> +void foo3()

> +{

> +}

> +

> +void

> +g ()

> +{

> +  foo<1>();

> +  foo2<1>();

> +  foo3<>();

> +}

> diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-class29.C b/gcc/testsuite/g++.dg/cpp2a/nontype-class29.C

> new file mode 100644

> index 00000000000..2d2e9690a06

> --- /dev/null

> +++ b/gcc/testsuite/g++.dg/cpp2a/nontype-class29.C

> @@ -0,0 +1,26 @@

> +// PR c++/92948 - Fix class NTTP with template arguments.

> +// { dg-do run { target c++2a } }

> +

> +struct A {

> +  int i;

> +  constexpr A(int n) : i(n) { }

> +};

> +

> +template<A a>

> +struct B {

> +  static constexpr int i = a.i;

> +};

> +

> +template<int X>

> +void foo()

> +{

> +  B<X> b;

> +  if (b.i != 42)

> +    __builtin_abort ();

> +}

> +

> +int

> +main ()

> +{

> +  foo<42>();

> +}

> 

> base-commit: ad690d79cfbb905c5546c9333c5fd089d906505b

>

Patch

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index e889c800aca..fccf4e95453 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -7074,7 +7074,8 @@  convert_nontype_argument (tree type, tree expr, tsubst_flags_t complain)
   if (non_dep)
     expr = instantiate_non_dependent_expr_internal (expr, complain);
 
-  if (value_dependent_expression_p (expr))
+  const bool val_dep_p = value_dependent_expression_p (expr);
+  if (val_dep_p)
     expr = canonicalize_expr_argument (expr, complain);
 
   /* 14.3.2/5: The null pointer{,-to-member} conversion is applied
@@ -7100,10 +7101,17 @@  convert_nontype_argument (tree type, tree expr, tsubst_flags_t complain)
 	     a conversion function with a value-dependent argument, which
 	     could invoke taking the address of a temporary representing
 	     the result of the conversion.  */
-	  if (COMPOUND_LITERAL_P (expr)
-	      && CONSTRUCTOR_IS_DEPENDENT (expr)
-	      && MAYBE_CLASS_TYPE_P (expr_type)
-	      && TYPE_HAS_CONVERSION (expr_type))
+	  if (!same_type_ignoring_top_level_qualifiers_p (type, expr_type)
+	      && ((COMPOUND_LITERAL_P (expr)
+		   && CONSTRUCTOR_IS_DEPENDENT (expr)
+		   && MAYBE_CLASS_TYPE_P (expr_type)
+		   && TYPE_HAS_CONVERSION (expr_type))
+		  /* Similarly, converting e.g. an integer to a class
+		     involves a constructor call.  convert_like would
+		     create a TARGET_EXPR, but in a template we can't
+		     use AGGR_INIT_EXPR, and the TARGET_EXPR would lead
+		     to a bogus error.  */
+		  || (val_dep_p && MAYBE_CLASS_TYPE_P (type))))
 	    {
 	      expr = build1 (IMPLICIT_CONV_EXPR, type, expr);
 	      IMPLICIT_CONV_EXPR_NONTYPE_ARG (expr) = true;
@@ -7189,8 +7197,7 @@  convert_nontype_argument (tree type, tree expr, tsubst_flags_t complain)
 
       /* Notice that there are constant expressions like '4 % 0' which
 	 do not fold into integer constants.  */
-      if (TREE_CODE (expr) != INTEGER_CST
-	  && !value_dependent_expression_p (expr))
+      if (TREE_CODE (expr) != INTEGER_CST && !val_dep_p)
 	{
 	  if (complain & tf_error)
 	    {
@@ -7265,7 +7272,7 @@  convert_nontype_argument (tree type, tree expr, tsubst_flags_t complain)
 	Here, we do not care about functions, as they are invalid anyway
 	for a parameter of type pointer-to-object.  */
 
-      if (value_dependent_expression_p (expr))
+      if (val_dep_p)
 	/* Non-type template parameters are OK.  */
 	;
       else if (cxx_dialect >= cxx11 && integer_zerop (expr))
@@ -7333,8 +7340,7 @@  convert_nontype_argument (tree type, tree expr, tsubst_flags_t complain)
 	    }
 	}
 
-      if (TYPE_REF_OBJ_P (TREE_TYPE (expr))
-	  && value_dependent_expression_p (expr))
+      if (TYPE_REF_OBJ_P (TREE_TYPE (expr)) && val_dep_p)
 	/* OK, dependent reference.  We don't want to ask whether a DECL is
 	   itself value-dependent, since what we want here is its address.  */;
       else
@@ -7412,7 +7418,7 @@  convert_nontype_argument (tree type, tree expr, tsubst_flags_t complain)
 
       /* [temp.arg.nontype] bullet 1 says the pointer to member
          expression must be a pointer-to-member constant.  */
-      if (!value_dependent_expression_p (expr)
+      if (!val_dep_p
 	  && !check_valid_ptrmem_cst_expr (type, expr, complain))
 	return NULL_TREE;
 
@@ -7429,7 +7435,7 @@  convert_nontype_argument (tree type, tree expr, tsubst_flags_t complain)
     {
       /* [temp.arg.nontype] bullet 1 says the pointer to member
          expression must be a pointer-to-member constant.  */
-      if (!value_dependent_expression_p (expr)
+      if (!val_dep_p
 	  && !check_valid_ptrmem_cst_expr (type, expr, complain))
 	return NULL_TREE;
 
@@ -7452,7 +7458,7 @@  convert_nontype_argument (tree type, tree expr, tsubst_flags_t complain)
     {
       /* Replace the argument with a reference to the corresponding template
 	 parameter object.  */
-      if (!value_dependent_expression_p (expr))
+      if (!val_dep_p)
 	expr = get_template_parm_object (expr, complain);
       if (expr == error_mark_node)
 	return NULL_TREE;
@@ -16406,7 +16412,9 @@  tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 		}
 	      else
 		{
-		  gcc_assert (CP_TYPE_CONST_P (TREE_TYPE (op)));
+		  gcc_assert (CP_TYPE_CONST_P (TREE_TYPE (op))
+			      || (TREE_CODE (op) == IMPLICIT_CONV_EXPR
+				  && IMPLICIT_CONV_EXPR_NONTYPE_ARG (op)));
 		  return op;
 		}
 	    }
diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-class28.C b/gcc/testsuite/g++.dg/cpp2a/nontype-class28.C
new file mode 100644
index 00000000000..80b9b499355
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/nontype-class28.C
@@ -0,0 +1,37 @@ 
+// PR c++/92948 - Fix class NTTP with template arguments.
+// { dg-do compile { target c++2a } }
+
+struct A {
+  constexpr A(int) { }
+};
+
+template<A>
+struct B {
+  using U = unsigned;
+};
+
+template<A a>
+using U = B<a>;
+
+template<int X, typename Y = typename B<X>::U>
+void foo()
+{
+}
+
+template<int X, typename Y = typename U<X>::U>
+void foo2()
+{
+}
+
+template<typename Y = typename B<1>::U>
+void foo3()
+{
+}
+
+void
+g ()
+{
+  foo<1>();
+  foo2<1>();
+  foo3<>();
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-class29.C b/gcc/testsuite/g++.dg/cpp2a/nontype-class29.C
new file mode 100644
index 00000000000..2d2e9690a06
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/nontype-class29.C
@@ -0,0 +1,26 @@ 
+// PR c++/92948 - Fix class NTTP with template arguments.
+// { dg-do run { target c++2a } }
+
+struct A {
+  int i;
+  constexpr A(int n) : i(n) { }
+};
+
+template<A a>
+struct B {
+  static constexpr int i = a.i;
+};
+
+template<int X>
+void foo()
+{
+  B<X> b;
+  if (b.i != 42)
+    __builtin_abort ();
+}
+
+int
+main ()
+{
+  foo<42>();
+}