public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
From: Patrick Palka <ppalka@gcc.gnu.org>
To: gcc-cvs@gcc.gnu.org
Subject: [gcc r13-6452] c++: thinko in extract_local_specs [PR108998]
Date: Fri,  3 Mar 2023 16:37:12 +0000 (GMT)	[thread overview]
Message-ID: <20230303163712.07B2B3858D37@sourceware.org> (raw)

https://gcc.gnu.org/g:341e6cd8d603a334fd34657a6b454176be1c6437

commit r13-6452-g341e6cd8d603a334fd34657a6b454176be1c6437
Author: Patrick Palka <ppalka@redhat.com>
Date:   Fri Mar 3 11:37:02 2023 -0500

    c++: thinko in extract_local_specs [PR108998]
    
    In order to fix PR100295, r13-4730-g18499b9f848707 attempted to make
    extract_local_specs walk the given pattern twice, ignoring unevaluated
    operands the first time around so that we prefer to process a local
    specialization in an evaluated context if it appears in one (we process
    each local specialization once even if it appears multiple times in the
    pattern).
    
    But there's a thinko in the patch, namely that we don't actually walk
    the pattern twice since we don't clear the visited set for the second
    walk (to avoid processing a local specialization twice) and so the root
    node (and any node leading up to an unevaluated operand) is considered
    visited already.  So the patch effectively made extract_local_specs
    ignore unevaluated operands altogether, which this testcase demonstrates
    isn't quite safe (extract_local_specs never sees 'aa' and we don't record
    its local specialization, so later we try to specialize 'aa' on the spot
    with the args {{int},{17}} which causes us to nonsensically substitute
    its auto with 17.)
    
    This patch fixes this by refining the second walk to start from the
    trees we skipped over during the first walk.
    
            PR c++/108998
    
    gcc/cp/ChangeLog:
    
            * pt.cc (el_data::skipped_trees): New data member.
            (extract_locals_r): Push to skipped_trees any unevaluated
            contexts that we skipped over.
            (extract_local_specs): For the second walk, start from each
            tree in skipped_trees.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/cpp2a/lambda-generic11.C: New test.

Diff:
---
 gcc/cp/pt.cc                                  | 10 +++++++++-
 gcc/testsuite/g++.dg/cpp2a/lambda-generic11.C | 13 +++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index ba1b3027513..85136df1730 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -13052,6 +13052,8 @@ public:
   tsubst_flags_t complain;
   /* True iff we don't want to walk into unevaluated contexts.  */
   bool skip_unevaluated_operands = false;
+  /* The unevaluated contexts that we avoided walking.  */
+  auto_vec<tree> skipped_trees;
 
   el_data (tsubst_flags_t c)
     : extra (NULL_TREE), complain (c) {}
@@ -13066,6 +13068,7 @@ extract_locals_r (tree *tp, int *walk_subtrees, void *data_)
   if (data.skip_unevaluated_operands
       && unevaluated_p (TREE_CODE (*tp)))
     {
+      data.skipped_trees.safe_push (*tp);
       *walk_subtrees = 0;
       return NULL_TREE;
     }
@@ -13168,8 +13171,13 @@ extract_local_specs (tree pattern, tsubst_flags_t complain)
      context).  */
   data.skip_unevaluated_operands = true;
   cp_walk_tree (&pattern, extract_locals_r, &data, &data.visited);
+  /* Now walk the unevaluated contexts we skipped the first time around.  */
   data.skip_unevaluated_operands = false;
-  cp_walk_tree (&pattern, extract_locals_r, &data, &data.visited);
+  for (tree t : data.skipped_trees)
+    {
+      data.visited.remove (t);
+      cp_walk_tree (&t, extract_locals_r, &data, &data.visited);
+    }
   return data.extra;
 }
 
diff --git a/gcc/testsuite/g++.dg/cpp2a/lambda-generic11.C b/gcc/testsuite/g++.dg/cpp2a/lambda-generic11.C
new file mode 100644
index 00000000000..62affac542e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/lambda-generic11.C
@@ -0,0 +1,13 @@
+// PR c++/108999
+// { dg-do compile { target c++20 } }
+
+template<typename T>
+void ice(T a) {
+  auto aa = a;
+  auto lambda = []<int I>() {
+    if constexpr (sizeof(aa) + I != 42) { }
+  };
+  lambda.template operator()<17>();
+}
+
+template void ice(int);

                 reply	other threads:[~2023-03-03 16:37 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20230303163712.07B2B3858D37@sourceware.org \
    --to=ppalka@gcc.gnu.org \
    --cc=gcc-cvs@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).