On 4/10/24 17:39, Patrick Palka wrote:
On Wed, 10 Apr 2024, Jason Merrill wrote:

On 3/12/24 10:51, Patrick Palka wrote:
On Tue, 12 Mar 2024, Patrick Palka wrote:
On Tue, 12 Mar 2024, Jason Merrill wrote:
On 3/11/24 12:53, Patrick Palka wrote:

r13-6452-g341e6cd8d603a3 made build_extra_args walk evaluated contexts
first so that we prefer processing a local specialization in an
evaluated
context even if its first use is in an unevaluated context.  But this
means we need to avoid walking a tree that already has extra
args/specs
saved because the list of saved specs appears to be an evaluated
context.  It seems then that we should be calculating the saved specs
from scratch each time, rather than potentially walking the saved
specs
list from an earlier partial instantiation when calling
build_extra_args
a second time around.

Makes sense, but I wonder if we want to approach that by avoiding
walking into
*_EXTRA_ARGS in extract_locals_r?  Or do we still want to walk into any
nested
extra args?  And if so, will we run into this same problem then?

I'm not sure totally but I'd expect a nested extra-args tree to always
have empty *_EXTRA_ARGS since the outer extra-args tree should intercept
any substitution before the inner extra-args tree can see it?

... and so in extract_locals_r I think we can assume *_EXTRA_ARGS is
empty, and not have to explicitly avoid walking it.

It seems more robust to me to handle _EXTRA_ARGS appropriately in
build_extra_args rather than expect callers to know that they shouldn't pass
in a tree with _EXTRA_ARGS set.  At least check and abort in that case?

Sounds good.  That IMHO seems simpler than actually avoiding walking
into *_EXTRA_ARGS from extract_locals_r because we'd have to repeat
the walking logic from cp_walk_subtree modulo the *_EXTRA_ARGS walk.

How does the following look? Bootstraped and regtested on
x86_64-pc-linux-gnu.

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index c38594cd862..6cc9b95fc06 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -13310,6 +13310,19 @@ extract_locals_r (tree *tp, int *walk_subtrees, void 
*data_)
      /* Remember local typedefs (85214).  */
      tp = &TYPE_NAME (*tp);

Please add a comment explaining why it needs to be null.

Also, how about a generic _EXTRA_ARGS accessor so places like this don't need to check each code themselves?

+  if (has_extra_args_mechanism_p (*tp))
+    {
+      if (PACK_EXPANSION_P (*tp))
+       gcc_checking_assert (!PACK_EXPANSION_EXTRA_ARGS (*tp));
+      else if (TREE_CODE (*tp) == REQUIRES_EXPR)
+       gcc_checking_assert (!REQUIRES_EXPR_EXTRA_ARGS (*tp));
+      else if (TREE_CODE (*tp) == IF_STMT
+              && IF_STMT_CONSTEXPR_P (*tp))
+       gcc_checking_assert (!IF_STMT_EXTRA_ARGS (*tp));
+      else
+       gcc_unreachable ();
+    }
+
    if (TREE_CODE (*tp) == DECL_EXPR)
      {
        tree decl = DECL_EXPR_DECL (*tp);
@@ -18738,7 +18751,8 @@ tsubst_stmt (tree t, tree args, tsubst_flags_t 
complain, tree in_decl)
          IF_COND (stmt) = IF_COND (t);
          THEN_CLAUSE (stmt) = THEN_CLAUSE (t);
          ELSE_CLAUSE (stmt) = ELSE_CLAUSE (t);
-         IF_STMT_EXTRA_ARGS (stmt) = build_extra_args (t, args, complain);
+         IF_SCOPE (stmt) = NULL_TREE;

What does IF_SCOPE have to do with this?

Jason

Reply via email to