[PATCH] c++: extract_local_specs and unevaluated contexts [PR100295]

[PATCH] c++: extract_local_specs and unevaluated contexts [PR100295]

[Patrick Palka]

Here during partial instantiation of the constexpr if, extra_local_specs
walks the statement looking for local specializations within to save and
possibly capture.  However, we're thwarted by the fact that 'ts' first
appears inside an unevaluated context, and so the calls to
process_outer_var_ref for its local specializations are a no-op.  And
since we walk each tree exactly once, we end up not capturing them
despite it later occuring in an evaluated context.

This patch fixes this by making extract_local_specs walk evaluated
contexts first before walking unevaluated contexts.  We could probably
get away with not walking unevaluated contexts at all, but this approach
seems safer.

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

	PR c++/100295
	PR c++/107579

gcc/cp/ChangeLog:

	* pt.cc (el_data::skip_unevaluated_operands): New data member.
	(extract_locals_r): If skip_unevaluated_operands is true,
	don't walk into unevaluated contexts.
	(extract_local_specs): Walk the pattern twice, first with
	skip_unevaluated_operands true followed by it set to false.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp1z/constexpr-if-lambda5.C: New test.
---
 gcc/cp/pt.cc                                  | 19 ++++++++++++++++++-
 .../g++.dg/cpp1z/constexpr-if-lambda5.C       | 15 +++++++++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/constexpr-if-lambda5.C

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index d05a49b1c11..2b22bf14c53 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -13015,17 +13015,26 @@ public:
   /* List of local_specializations used within the pattern.  */
   tree extra;
   tsubst_flags_t complain;
+  /* True iff we don't want to walk into unevaluated contexts.  */
+  bool skip_unevaluated_operands = false;
 
   el_data (tsubst_flags_t c)
     : extra (NULL_TREE), complain (c) {}
 };
 static tree
-extract_locals_r (tree *tp, int */*walk_subtrees*/, void *data_)
+extract_locals_r (tree *tp, int *walk_subtrees, void *data_)
 {
   el_data &data = *reinterpret_cast<el_data*>(data_);
   tree *extra = &data.extra;
   tsubst_flags_t complain = data.complain;
 
+  if (data.skip_unevaluated_operands
+      && unevaluated_p (TREE_CODE (*tp)))
+    {
+      *walk_subtrees = 0;
+      return NULL_TREE;
+    }
+
   if (TYPE_P (*tp) && typedef_variant_p (*tp))
     /* Remember local typedefs (85214).  */
     tp = &TYPE_NAME (*tp);
@@ -13117,6 +13126,14 @@ static tree
 extract_local_specs (tree pattern, tsubst_flags_t complain)
 {
   el_data data (complain);
+  /* Walk the pattern twice, ignoring unevaluated operands the first time
+     around, so that if a local specialization appears in both an
+     evaluated and unevaluated context we prefer to process it in the
+     former context (since e.g. process_outer_var_ref is a no-op inside
+     an unevaluated context).  */
+  data.skip_unevaluated_operands = true;
+  cp_walk_tree (&pattern, extract_locals_r, &data, &data.visited);
+  data.skip_unevaluated_operands = false;
   cp_walk_tree (&pattern, extract_locals_r, &data, &data.visited);
   return data.extra;
 }
diff --git a/gcc/testsuite/g++.dg/cpp1z/constexpr-if-lambda5.C b/gcc/testsuite/g++.dg/cpp1z/constexpr-if-lambda5.C
new file mode 100644
index 00000000000..d2bf0221743
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/constexpr-if-lambda5.C
@@ -0,0 +1,15 @@
+// PR c++/100295
+// { dg-do compile { target c++17 } }
+
+template<typename... Ts>
+void f(Ts... ts) {
+  auto lambda = [=](auto x) {
+    if constexpr (sizeof((ts+x) + ...) != 0)
+      (..., ts);
+  };
+  lambda(0);
+}
+
+int main() {
+  f(0, 'a');
+}
-- 
2.39.0.rc2

Re: [PATCH] c++: extract_local_specs and unevaluated contexts [PR100295]

[Jason Merrill]

On 12/9/22 16:57, Patrick Palka wrote:
> Here during partial instantiation of the constexpr if, extra_local_specs
> walks the statement looking for local specializations within to save and
> possibly capture.  However, we're thwarted by the fact that 'ts' first
> appears inside an unevaluated context, and so the calls to
> process_outer_var_ref for its local specializations are a no-op.  And
> since we walk each tree exactly once, we end up not capturing them
> despite it later occuring in an evaluated context.
> 
> This patch fixes this by making extract_local_specs walk evaluated
> contexts first before walking unevaluated contexts.  We could probably
> get away with not walking unevaluated contexts at all, but this approach
> seems safer.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk/12?

OK.

> 	PR c++/100295
> 	PR c++/107579
> 
> gcc/cp/ChangeLog:
> 
> 	* pt.cc (el_data::skip_unevaluated_operands): New data member.
> 	(extract_locals_r): If skip_unevaluated_operands is true,
> 	don't walk into unevaluated contexts.
> 	(extract_local_specs): Walk the pattern twice, first with
> 	skip_unevaluated_operands true followed by it set to false.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp1z/constexpr-if-lambda5.C: New test.
> ---
>   gcc/cp/pt.cc                                  | 19 ++++++++++++++++++-
>   .../g++.dg/cpp1z/constexpr-if-lambda5.C       | 15 +++++++++++++++
>   2 files changed, 33 insertions(+), 1 deletion(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp1z/constexpr-if-lambda5.C
> 
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index d05a49b1c11..2b22bf14c53 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -13015,17 +13015,26 @@ public:
>     /* List of local_specializations used within the pattern.  */
>     tree extra;
>     tsubst_flags_t complain;
> +  /* True iff we don't want to walk into unevaluated contexts.  */
> +  bool skip_unevaluated_operands = false;
>   
>     el_data (tsubst_flags_t c)
>       : extra (NULL_TREE), complain (c) {}
>   };
>   static tree
> -extract_locals_r (tree *tp, int */*walk_subtrees*/, void *data_)
> +extract_locals_r (tree *tp, int *walk_subtrees, void *data_)
>   {
>     el_data &data = *reinterpret_cast<el_data*>(data_);
>     tree *extra = &data.extra;
>     tsubst_flags_t complain = data.complain;
>   
> +  if (data.skip_unevaluated_operands
> +      && unevaluated_p (TREE_CODE (*tp)))
> +    {
> +      *walk_subtrees = 0;
> +      return NULL_TREE;
> +    }
> +
>     if (TYPE_P (*tp) && typedef_variant_p (*tp))
>       /* Remember local typedefs (85214).  */
>       tp = &TYPE_NAME (*tp);
> @@ -13117,6 +13126,14 @@ static tree
>   extract_local_specs (tree pattern, tsubst_flags_t complain)
>   {
>     el_data data (complain);
> +  /* Walk the pattern twice, ignoring unevaluated operands the first time
> +     around, so that if a local specialization appears in both an
> +     evaluated and unevaluated context we prefer to process it in the
> +     former context (since e.g. process_outer_var_ref is a no-op inside
> +     an unevaluated context).  */
> +  data.skip_unevaluated_operands = true;
> +  cp_walk_tree (&pattern, extract_locals_r, &data, &data.visited);
> +  data.skip_unevaluated_operands = false;
>     cp_walk_tree (&pattern, extract_locals_r, &data, &data.visited);
>     return data.extra;
>   }
> diff --git a/gcc/testsuite/g++.dg/cpp1z/constexpr-if-lambda5.C b/gcc/testsuite/g++.dg/cpp1z/constexpr-if-lambda5.C
> new file mode 100644
> index 00000000000..d2bf0221743
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1z/constexpr-if-lambda5.C
> @@ -0,0 +1,15 @@
> +// PR c++/100295
> +// { dg-do compile { target c++17 } }
> +
> +template<typename... Ts>
> +void f(Ts... ts) {
> +  auto lambda = [=](auto x) {
> +    if constexpr (sizeof((ts+x) + ...) != 0)
> +      (..., ts);
> +  };
> +  lambda(0);
> +}
> +
> +int main() {
> +  f(0, 'a');
> +}