public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Patrick Palka <ppalka@redhat.com>
To: gcc-patches@gcc.gnu.org
Subject: [PATCH] c++: parameter pack inside constexpr if [PR101764]
Date: Mon, 30 Aug 2021 22:05:31 -0400	[thread overview]
Message-ID: <20210831020531.3295265-1-ppalka@redhat.com> (raw)

Here when partially substituting into the pack expansion, substitution
into the constexpr if yields a still-dependent tree, so tsubst_expr
returns an IF_STMT with an unsubstituted IF_COND and with
IF_STMT_EXTRA_ARGS added to.  Hence after partial substitution
the pack expansion pattern still refers to the parameter pack 'ts' of
level 2 (and it's thus represented in the new PACK_EXPANSION_PARAMETER_PACKS)
even though the partially instantiated generic lambda admits only one
level of template arguments.  This causes us to crash during the
subsequent instantiation with the lambda's template arguments because of
the level mismatch.  (Likewise when the constexpr if is replaced by a
requires-expr, which too uses the extra args mechanism for delaying
partial instantiation.)

So essentially, a pack expansion pattern that contains a parameter pack
inside an "extra args" tree doesn't play well with partial substitution
thereof.  This patch fixes this by forcing such pack expansions to use
the extra args mechanism as well.

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

	PR c++/101764

gcc/cp/ChangeLog:

	* cp-tree.h (PACK_EXPANSION_FORCE_EXTRA_ARGS_P): New accessor
	macro.
	* pt.c (uses_extra_args_mechanism_p): New function.
	(find_parameter_pack_data::found_pack_within_extra_args_tree_p):
	New data member.
	(find_parameter_pack_data::inside_extra_args_tree_p): Likewise.
	(find_parameter_packs_r): Detect parameter packs within "extra
	args" trees and set found_pack_within_extra_args_tree_p
	appropriately.
	(make_pack_expansion): Set PACK_EXPANSION_FORCE_EXTRA_ARGS_P if
	found_pack_within_extra_args_tree_p.
	(use_pack_expansion_extra_args_p): Return true if there were
	unsubstituted packs and PACK_EXPANSION_FORCE_EXTRA_ARGS_P.
	(tsubst_pack_expansion): Pass the pack expansion to
	use_pack_expansion_extra_args_p.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp1z/constexpr-if35.C: New test.
---
 gcc/cp/cp-tree.h                            |  5 ++
 gcc/cp/pt.c                                 | 69 ++++++++++++++++++++-
 gcc/testsuite/g++.dg/cpp1z/constexpr-if35.C | 18 ++++++
 3 files changed, 90 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/constexpr-if35.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index ce7ca53a113..06dec495428 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -493,6 +493,7 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX];
       CONSTRUCTOR_C99_COMPOUND_LITERAL (in CONSTRUCTOR)
       OVL_NESTED_P (in OVERLOAD)
       DECL_MODULE_EXPORT_P (in _DECL)
+      PACK_EXPANSION_FORCE_EXTRA_ARGS_P (in *_PACK_EXPANSION)
    4: IDENTIFIER_MARKED (IDENTIFIER_NODEs)
       TREE_HAS_CONSTRUCTOR (in INDIRECT_REF, SAVE_EXPR, CONSTRUCTOR,
 	  CALL_EXPR, or FIELD_DECL).
@@ -3902,6 +3903,10 @@ struct GTY(()) lang_decl {
 /* True iff this pack expansion is for auto... in lambda init-capture.  */
 #define PACK_EXPANSION_AUTO_P(NODE) TREE_LANG_FLAG_2 (NODE)
 
+/* True if we must use PACK_EXPANSION_EXTRA_ARGS to avoid partial
+   substitution into this pack expansion.  */
+#define PACK_EXPANSION_FORCE_EXTRA_ARGS_P(NODE) TREE_LANG_FLAG_3 (NODE)
+
 /* True iff the wildcard can match a template parameter pack.  */
 #define WILDCARD_PACK_P(NODE) TREE_LANG_FLAG_0 (NODE)
 
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index fcf3ac31b25..a92dff88d9d 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -3855,6 +3855,20 @@ expand_builtin_pack_call (tree call, tree args, tsubst_flags_t complain,
   return NULL_TREE;
 }
 
+/* Return true if the tree T uses the extra args mechanism for
+   deferring partial substitution into it.  */
+
+static bool
+uses_extra_args_mechanism_p (tree t)
+{
+  return (PACK_EXPANSION_P (t)
+	  || TREE_CODE (t) == REQUIRES_EXPR
+	  || (TREE_CODE (t) == IF_STMT
+	      && IF_STMT_CONSTEXPR_P (t)));
+}
+
+static tree find_parameter_packs_r (tree *, int *, void*);
+
 /* Structure used to track the progress of find_parameter_packs_r.  */
 struct find_parameter_pack_data
 {
@@ -3867,6 +3881,16 @@ struct find_parameter_pack_data
 
   /* True iff we're making a type pack expansion.  */
   bool type_pack_expansion_p;
+
+  /* True iff we found a pack inside a subtree that uses the extra
+     args mechanism.  */
+  bool found_pack_within_extra_args_tree_p = false;
+
+private:
+  /* True iff find_parameter_packs_r is currently visiting a tree
+     that uses the extra args mechanism or a subtree thereof.  */
+  bool inside_extra_args_tree_p = false;
+  friend tree find_parameter_packs_r (tree *, int *, void*);
 };
 
 /* Identifies all of the argument packs that occur in a template
@@ -3968,6 +3992,37 @@ find_parameter_packs_r (tree *tp, int *walk_subtrees, void* data)
       *ppd->parameter_packs = tree_cons (NULL_TREE, t, *ppd->parameter_packs);
     }
 
+  if (!ppd->inside_extra_args_tree_p
+      && uses_extra_args_mechanism_p (t)
+      && !PACK_EXPANSION_P (t))
+    {
+      /* This tree is using the extra args mechanism.  Update *ppd and recurse
+	 so that we can try to detect a parameter pack within.  */
+      tree parameter_packs = NULL_TREE;
+      hash_set<tree> visited;
+
+	{
+	  auto iovr = make_temp_override (ppd->inside_extra_args_tree_p,
+					  true);
+	  auto povr = make_temp_override (ppd->parameter_packs,
+					  &parameter_packs);
+	  auto vovr = make_temp_override (ppd->visited,
+					  &visited);
+	  WALK_SUBTREE (t);
+	}
+
+      if (parameter_packs)
+	{
+	  /* We found some parameter packs within this extra args tree.  */
+	  ppd->found_pack_within_extra_args_tree_p = true;
+	  /* Walk them so that they get added to the overall list.  */
+	  WALK_SUBTREE (parameter_packs);
+	}
+
+      *walk_subtrees = 0;
+      return NULL_TREE;
+    }
+
   if (TYPE_P (t))
     cp_walk_tree (&TYPE_CONTEXT (t),
 		  &find_parameter_packs_r, ppd, ppd->visited);
@@ -4229,6 +4284,11 @@ make_pack_expansion (tree arg, tsubst_flags_t complain)
   PACK_EXPANSION_PARAMETER_PACKS (result) = parameter_packs;
 
   PACK_EXPANSION_LOCAL_P (result) = at_function_scope_p ();
+  if (ppd.found_pack_within_extra_args_tree_p)
+    /* If the pattern of this pack expansion contains a parameter pack within
+       a subtree that uses the extra args mechanism, then the pack expansion
+       must also use the extra args mechanism (101764).  */
+    PACK_EXPANSION_FORCE_EXTRA_ARGS_P (result) = true;
 
   return result;
 }
@@ -12392,10 +12452,15 @@ make_argument_pack_select (tree arg_pack, unsigned index)
     substitution.  */
 
 static bool
-use_pack_expansion_extra_args_p (tree parm_packs,
+use_pack_expansion_extra_args_p (tree t,
+				 tree parm_packs,
 				 int arg_pack_len,
 				 bool has_empty_arg)
 {
+  if (has_empty_arg
+      && PACK_EXPANSION_FORCE_EXTRA_ARGS_P (t))
+    return true;
+
   /* If one pack has an expansion and another pack has a normal
      argument or if one pack has an empty argument and an another
      one hasn't then tsubst_pack_expansion cannot perform the
@@ -13148,7 +13213,7 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
 
   /* We cannot expand this expansion expression, because we don't have
      all of the argument packs we need.  */
-  if (use_pack_expansion_extra_args_p (packs, len, unsubstituted_packs))
+  if (use_pack_expansion_extra_args_p (t, packs, len, unsubstituted_packs))
     {
       /* We got some full packs, but we can't substitute them in until we
 	 have values for all the packs.  So remember these until then.  */
diff --git a/gcc/testsuite/g++.dg/cpp1z/constexpr-if35.C b/gcc/testsuite/g++.dg/cpp1z/constexpr-if35.C
new file mode 100644
index 00000000000..cae690ba82c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/constexpr-if35.C
@@ -0,0 +1,18 @@
+// PR c++/101764
+// { dg-do compile { target c++17 } }
+
+void g(...);
+
+template<class>
+auto f() {
+  return [](auto... ts) {
+    g([] { if constexpr (decltype(ts){0}); }...);
+#if __cpp_concepts
+    g(requires { decltype(ts){0}; }...);
+#endif
+  };
+}
+
+int main() {
+  f<int>()('a', true);
+}
-- 
2.33.0.69.gc4203212e3


             reply	other threads:[~2021-08-31  2:05 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-31  2:05 Patrick Palka [this message]
2021-09-02 20:09 ` Jason Merrill
2021-09-12 23:48   ` Patrick Palka
2021-09-13  2:29     ` Jason Merrill
2021-09-13 14:51       ` Patrick Palka

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=20210831020531.3295265-1-ppalka@redhat.com \
    --to=ppalka@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    /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).