Re: structurally compare type_arg_packs [93933]

2020-02-27 Thread Jason Merrill

On 2/27/20 10:33 AM, Nathan Sidwell wrote:

On 2/26/20 5:00 PM, Jason Merrill wrote:

On 2/25/20 4:09 PM, Nathan Sidwell wrote:
We consider all TYPE_ARGUMENT_PACKS distinct types, leading to 
problems with redeclarations.


I'd think that the bug is that we're treating them as types in the 
first place; they aren't types, so they shouldn't reach comptypes.  
I'd lean toward adding an assert to that effect and fixing the caller 
to use e.g. template_args_equal.


Thanks, this patch implements that approach.


That TYPE_ARGUMENT_PACKS are not types, suggests to me that 
NONTYPE_ARGUMENT_PACKS are not expressions.  Perhaps their 
representation should be unified -- I keep encountering code handling 
them essentially doing the same thing for both kinds.  But that's a 
GCC-11 thing at least.


Agreed.  I took a step in that direction when I removed TREE_TYPE from 
NONTYPE_ARGUMENT_PACK, but going on to unify the packs makes sense to me.


Jason



Re: structurally compare type_arg_packs [93933]

2020-02-27 Thread Nathan Sidwell

On 2/26/20 5:00 PM, Jason Merrill wrote:

On 2/25/20 4:09 PM, Nathan Sidwell wrote:
We consider all TYPE_ARGUMENT_PACKS distinct types, leading to 
problems with redeclarations.


I'd think that the bug is that we're treating them as types in the first 
place; they aren't types, so they shouldn't reach comptypes.  I'd lean 
toward adding an assert to that effect and fixing the caller to use e.g. 
template_args_equal.


Thanks, this patch implements that approach.

That TYPE_ARGUMENT_PACKS are not types, suggests to me that 
NONTYPE_ARGUMENT_PACKS are not expressions.  Perhaps their 
representation should be unified -- I keep encountering code handling 
them essentially doing the same thing for both kinds.  But that's a 
GCC-11 thing at least.


nathan

--
Nathan Sidwell
2020-02-27  Nathan Sidwell  

	PR c++/93933
	* pt.c (template_args_equal): Pass ARGUMENT_PACKS through to
	cp_tree_equal.
	* tree.c (cp_tree_equal): Compare ARGUMENT_PACKS here,
	* typeck.c (comptypes): Assert we don't get any argument packs.

diff --git c/gcc/cp/pt.c w/gcc/cp/pt.c
index 6c9abb8f3d3..622c70b352f 100644
--- c/gcc/cp/pt.c
+++ w/gcc/cp/pt.c
@@ -8999,25 +8999,8 @@ template_args_equal (tree ot, tree nt, bool partial_order /* = false */)
 PACK_EXPANSION_PATTERN (nt))
 	&& template_args_equal (PACK_EXPANSION_EXTRA_ARGS (ot),
 PACK_EXPANSION_EXTRA_ARGS (nt)));
-  else if (ARGUMENT_PACK_P (ot))
-{
-  int i, len;
-  tree opack, npack;
-
-  if (!ARGUMENT_PACK_P (nt))
-	return 0;
-
-  opack = ARGUMENT_PACK_ARGS (ot);
-  npack = ARGUMENT_PACK_ARGS (nt);
-  len = TREE_VEC_LENGTH (opack);
-  if (TREE_VEC_LENGTH (npack) != len)
-	return 0;
-  for (i = 0; i < len; ++i)
-	if (!template_args_equal (TREE_VEC_ELT (opack, i),
-  TREE_VEC_ELT (npack, i)))
-	  return 0;
-  return 1;
-}
+  else if (ARGUMENT_PACK_P (ot) || ARGUMENT_PACK_P (nt))
+return cp_tree_equal (ot, nt);
   else if (ot && TREE_CODE (ot) == ARGUMENT_PACK_SELECT)
 gcc_unreachable ();
   else if (TYPE_P (nt))
diff --git c/gcc/cp/tree.c w/gcc/cp/tree.c
index 72b3a720ee8..3fc6287d566 100644
--- c/gcc/cp/tree.c
+++ w/gcc/cp/tree.c
@@ -3857,12 +3857,27 @@ cp_tree_equal (tree t1, tree t2)
 			 DEFERRED_NOEXCEPT_PATTERN (t2))
 	  && comp_template_args (DEFERRED_NOEXCEPT_ARGS (t1),
  DEFERRED_NOEXCEPT_ARGS (t2)));
-  break;
 
 case LAMBDA_EXPR:
   /* Two lambda-expressions are never considered equivalent.  */
   return false;
 
+case TYPE_ARGUMENT_PACK:
+case NONTYPE_ARGUMENT_PACK:
+  {
+	tree p1 = ARGUMENT_PACK_ARGS (t1);
+	tree p2 = ARGUMENT_PACK_ARGS (t2);
+	int len = TREE_VEC_LENGTH (p1);
+	if (TREE_VEC_LENGTH (p2) != len)
+	  return false;
+
+	for (int ix = 0; ix != len; ix++)
+	  if (!template_args_equal (TREE_VEC_ELT (p1, ix),
+TREE_VEC_ELT (p2, ix)))
+	return false;
+	return true;
+  }
+
 default:
   break;
 }
diff --git c/gcc/cp/typeck.c w/gcc/cp/typeck.c
index 42d0b47cf1b..2a3243f3e81 100644
--- c/gcc/cp/typeck.c
+++ w/gcc/cp/typeck.c
@@ -1485,6 +1485,10 @@ comptypes (tree t1, tree t2, int strict)
 {
   gcc_checking_assert (t1 && t2);
 
+  /* TYPE_ARGUMENT_PACKS are not really types.  */
+  gcc_checking_assert (TREE_CODE (t1) != TYPE_ARGUMENT_PACK
+		   && TREE_CODE (t2) != TYPE_ARGUMENT_PACK);
+
   if (strict == COMPARE_STRICT && comparing_specializations
   && (t1 != TYPE_CANONICAL (t1) || t2 != TYPE_CANONICAL (t2)))
 /* If comparing_specializations, treat dependent aliases as distinct.  */
diff --git c/gcc/testsuite/g++.dg/concepts/pr93933.C w/gcc/testsuite/g++.dg/concepts/pr93933.C
new file mode 100644
index 000..b4f2c36374d
--- /dev/null
+++ w/gcc/testsuite/g++.dg/concepts/pr93933.C
@@ -0,0 +1,31 @@
+// { dg-do compile { target c++17 } }
+// { dg-options "-fconcepts" }
+
+// distilled from , via header units
+
+template
+struct is_invocable;
+
+template
+concept invocable = is_invocable<_Args...>::value;
+
+template
+requires invocable<_Is>
+class BUG;
+
+template
+requires invocable<_Is>
+class BUG {}; // { dg-bogus "different constraints" }
+
+template struct is_invocable_NT;
+
+template
+concept invocable_NT = is_invocable_NT::value;
+
+template
+requires invocable_NT<_Is>
+class BUG_NT;
+
+template
+requires invocable_NT<_Is>
+class BUG_NT {};


Re: structurally compare type_arg_packs [93933]

2020-02-26 Thread Jason Merrill

On 2/25/20 4:09 PM, Nathan Sidwell wrote:
We consider all TYPE_ARGUMENT_PACKS distinct types, leading to problems 
with redeclarations.



This patch fixes things by:
a) marking all such types for structural comparison
b) teaching structural_comptypes how to compare them.

1) It appears that NONTYPE_ARGUMENT_PACKS just work, I think because we 
just compare TREE_OPERANDs, which dtrt.


2) I don't think the ARGUMENT_PACK_INCOMPLETE_P and 
ARGUMENT_PACK_EXPLICIT_ARGS affect the comparison, (icbw?)


I'll apply to trunk shortly, unless someone indicates I'm confused.


I'd think that the bug is that we're treating them as types in the first 
place; they aren't types, so they shouldn't reach comptypes.  I'd lean 
toward adding an assert to that effect and fixing the caller to use e.g. 
template_args_equal.


Jason



structurally compare type_arg_packs [93933]

2020-02-25 Thread Nathan Sidwell
We consider all TYPE_ARGUMENT_PACKS distinct types, leading to problems 
with redeclarations.


This patch fixes things by:
a) marking all such types for structural comparison
b) teaching structural_comptypes how to compare them.

1) It appears that NONTYPE_ARGUMENT_PACKS just work, I think because we 
just compare TREE_OPERANDs, which dtrt.


2) I don't think the ARGUMENT_PACK_INCOMPLETE_P and 
ARGUMENT_PACK_EXPLICIT_ARGS affect the comparison, (icbw?)


I'll apply to trunk shortly, unless someone indicates I'm confused.

As the PR says, this comes from the modules branch, where I need to 
merge global module entities.


nathan
--
Nathan Sidwell
2020-02-25  Nathan Sidwell  

	PR c++/93933
	* pt.c (template_parm_to_arg): TYPE_ARGUMENT_PACKS are structural.
	(coerce_template_parameter_pack, make_argument_pack)
	(tsubst_argument_pack, tsubst, type_unification_real)
	(unify_pack_expansion): Likewise.
	* typeck.c (structural_comptypes): Compare TYPE_ARGUMENT_PACK args.

diff --git c/gcc/cp/pt.c w/gcc/cp/pt.c
index 6c9abb8f3d3..8716f979027 100644
--- c/gcc/cp/pt.c
+++ w/gcc/cp/pt.c
@@ -4670,6 +4670,7 @@ template_parm_to_arg (tree t)
 	  TREE_VEC_ELT (vec, 0) = make_pack_expansion (t);
 
 	  t = cxx_make_type (TYPE_ARGUMENT_PACK);
+	  SET_TYPE_STRUCTURAL_EQUALITY (t);
 	  SET_ARGUMENT_PACK_ARGS (t, vec);
 	}
 }
@@ -8500,7 +8501,10 @@ coerce_template_parameter_pack (tree parms,
 
   if (TREE_CODE (TREE_VALUE (parm)) == TYPE_DECL
   || TREE_CODE (TREE_VALUE (parm)) == TEMPLATE_DECL)
-argument_pack = cxx_make_type (TYPE_ARGUMENT_PACK);
+{
+  argument_pack = cxx_make_type (TYPE_ARGUMENT_PACK);
+  SET_TYPE_STRUCTURAL_EQUALITY (argument_pack);
+}
   else
 {
   argument_pack = make_node (NONTYPE_ARGUMENT_PACK);
@@ -13007,7 +13011,10 @@ make_argument_pack (tree vec)
   tree pack;
   tree elt = TREE_VEC_ELT (vec, 0);
   if (TYPE_P (elt))
-pack = cxx_make_type (TYPE_ARGUMENT_PACK);
+{
+  pack = cxx_make_type (TYPE_ARGUMENT_PACK);
+  SET_TYPE_STRUCTURAL_EQUALITY (pack);
+}
   else
 {
   pack = make_node (NONTYPE_ARGUMENT_PACK);
@@ -13050,19 +13057,24 @@ tsubst_argument_pack (tree orig_arg, tree args, tsubst_flags_t complain,
 		  tree in_decl)
 {
   /* Substitute into each of the arguments.  */
-  tree new_arg = TYPE_P (orig_arg)
-? cxx_make_type (TREE_CODE (orig_arg))
-: make_node (TREE_CODE (orig_arg));
-
   tree pack_args = tsubst_template_args (ARGUMENT_PACK_ARGS (orig_arg),
 	 args, complain, in_decl);
-  if (pack_args == error_mark_node)
-new_arg = error_mark_node;
-  else
-SET_ARGUMENT_PACK_ARGS (new_arg, pack_args);
+  tree new_arg = error_mark_node;
+  if (pack_args != error_mark_node)
+{
+  if (TYPE_P (orig_arg))
+	{
+	  new_arg = cxx_make_type (TREE_CODE (orig_arg));
+	  SET_TYPE_STRUCTURAL_EQUALITY (new_arg);
+	}
+  else
+	{
+	  new_arg = make_node (TREE_CODE (orig_arg));
+	  TREE_CONSTANT (new_arg) = TREE_CONSTANT (orig_arg);
+	}
 
-  if (TREE_CODE (new_arg) == NONTYPE_ARGUMENT_PACK)
-TREE_CONSTANT (new_arg) = TREE_CONSTANT (orig_arg);
+  SET_ARGUMENT_PACK_ARGS (new_arg, pack_args);
+}
 
   return new_arg;
 }
@@ -15804,8 +15816,11 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 	if (code == NONTYPE_ARGUMENT_PACK)
 	  r = make_node (code);
 	else
-	  r = cxx_make_type (code);
-
+	  {
+	r = cxx_make_type (code);
+	SET_TYPE_STRUCTURAL_EQUALITY (r);
+	  }
+	
 	tree pack_args = ARGUMENT_PACK_ARGS (t);
 	pack_args = tsubst_template_args (pack_args, args, complain, in_decl);
 	SET_ARGUMENT_PACK_ARGS (r, pack_args);
@@ -21786,7 +21801,10 @@ type_unification_real (tree tparms,
 		  TREE_CONSTANT (arg) = 1;
 		}
 	  else
-		arg = cxx_make_type (TYPE_ARGUMENT_PACK);
+		{
+		  arg = cxx_make_type (TYPE_ARGUMENT_PACK);
+		  SET_TYPE_STRUCTURAL_EQUALITY (arg);
+		}
 
 	  SET_ARGUMENT_PACK_ARGS (arg, make_tree_vec (0));
 
@@ -22618,7 +22636,10 @@ unify_pack_expansion (tree tparms, tree targs, tree packed_parms,
   TREE_CONSTANT (result) = 1;
 }
   else
-result = cxx_make_type (TYPE_ARGUMENT_PACK);
+	{
+	  result = cxx_make_type (TYPE_ARGUMENT_PACK);
+	  SET_TYPE_STRUCTURAL_EQUALITY (result);
+	}
 
   SET_ARGUMENT_PACK_ARGS (result, new_args);
 
diff --git c/gcc/cp/typeck.c w/gcc/cp/typeck.c
index c0c98dad980..0954c37444e 100644
--- c/gcc/cp/typeck.c
+++ w/gcc/cp/typeck.c
@@ -1453,6 +1453,11 @@ structural_comptypes (tree t1, tree t2, int strict)
   return same_type_p (UNDERLYING_TYPE_TYPE (t1), 
 			  UNDERLYING_TYPE_TYPE (t2));
 
+case TYPE_ARGUMENT_PACK:
+  if (!cp_tree_equal (ARGUMENT_PACK_ARGS (t1), ARGUMENT_PACK_ARGS (t2)))
+	return false;
+  break;
+
 default:
   return false;
 }
diff --git c/gcc/testsuite/g++.dg/concepts/pr93933.C w/gcc/testsuite/g++.dg/concepts/pr93933.C
new file mode 100644
index 000..b4f2c36374d
--- /dev/null
+++