| Message ID | 20260505012506.748365-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers |
Return-Path: <gcc-patches-bounces~patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id A4C614B9DB7C for <patchwork@sourceware.org>; Tue, 5 May 2026 01:26:05 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A4C614B9DB7C Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=GC+1ALfI X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id ABE464B9DB64 for <gcc-patches@gcc.gnu.org>; Tue, 5 May 2026 01:25:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org ABE464B9DB64 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org ABE464B9DB64 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1777944314; cv=none; b=xYjbZjmlguHQHxte9fI0jrClNPfA3kYqB6c/8bv7N8BgwoqgG+4ib2BScR16peB237veuXBE2zNIVJjCzrdlkVYbgnthNRVVELh8LH6EYN1OTmQTAMdAQU3Mf6XmiCRakF59Qs8iPGq32Sam8gzr9754P/+PbAyfWOxHUXbKKCo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1777944314; c=relaxed/simple; bh=avSuZD9WbJSwLIbCIuGukeCsl33uEdE7VjQ11ug2lgg=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=kyfIlzPkPV2uMkavzw5LvgSlb5KLE63EqfKfOxf0i75UwNc6lx2CEwFvZiZNybKbZRONOAVdmY8O4xrznIqBssXvZZh9jrgl9h0CrtAUVa3+BPctZ642IvlCRetWU5RzzaTKcFK5pCS8b3ussxmKp0qSuhxcs1kFMgTJO4QBFy8= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org ABE464B9DB64 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1777944314; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=xEpf7z2mH/txvoGqeGNBD2HGjbVpRjeBZx2KL0hTw6I=; b=GC+1ALfI5D0ZU9GKQZnYWoYTlCrP2F3UvgMZ4/g2m4nE4pvuXdoDPV28auXdbqdyE5m/Fo cHZxODr8nktxD8MRwTbgSfwAhZgdGdPl3mWyQfBNhlDHC4tZnivjZTce/vYBiKvtQGeRw4 jEWutnWeTDqlxEFIjiOPtfUa8aiEMyU= Received: from mail-qk1-f197.google.com (mail-qk1-f197.google.com [209.85.222.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-124-IKRFE9riN-OKllab10UeGA-1; Mon, 04 May 2026 21:25:12 -0400 X-MC-Unique: IKRFE9riN-OKllab10UeGA-1 X-Mimecast-MFC-AGG-ID: IKRFE9riN-OKllab10UeGA_1777944312 Received: by mail-qk1-f197.google.com with SMTP id af79cd13be357-8d59968444aso108233085a.3 for <gcc-patches@gcc.gnu.org>; Mon, 04 May 2026 18:25:12 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1777944312; x=1778549112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=xEpf7z2mH/txvoGqeGNBD2HGjbVpRjeBZx2KL0hTw6I=; b=LU/et6SfsZKJQ0t5ruJtq7kQ5Xt0kwGAg/g4eLSSvUAmZd6ROSA5WNIWOwpwt6TG2s I3hhFkmWgI/CVJ+gf+krWaNmz/cT6Ms03+/wrcRUm/JqPAmxn4x74zlMAQPp9R6lO6ch 5p7c7NfgLQF7LpgKD8IUK3Qkt3/tIn81u3iRLtz9Zzi1IAq+hz+AUnR0O8pDS5IceD4Q VNpGHRq7Qd5/ZJ7eAtnBqGCc8arCT8Z4E0gwOkQ1MmxypXXLxjD6l9+kTAFRm20No9Bm SMADbx/rh9yPqqLzXHaCEFFv0CbMAq55f5XVfJfTBwstID1+0AhJ4wU3Mb9na3rFuRWK NkWQ== X-Gm-Message-State: AOJu0Yw7zXC+C4p3HRLyrK9/tVZ26fSYjg6IKkoW7YHG3awWNE6cf3Vm ODVWEpKSSyxT1gmDZtjR2+bJmvAqlmSX33SfD99GHcNGoUV1NB9wW3ucjG1adh7SXofdg516eOr v5/DYKRPKebOvWLBMmSfe1vrUv8btduljYbOJNgoWTc8cLKEc0IV+Hl8vB0Ly/0NZOqWkS3qMxj g9CG1/7R+OVsWhmVyD5g96mRKwr2xzHhCl3ZbvYqrg X-Gm-Gg: AeBDieuVTLFCueTRY1nj7mqvVo8qHMUVolTeBz26J+lZjZr65y9BJGXM92fPiCzXDl7 wgE9fM/2cPh/I0jQQd8fJeOcBrv7/59wg4LlMl/NF2tOPhOm2r32A2NwyJiWVRv8v4FMyMN3sg1 WeUdcBiUF+rohkDMQZjtDspjerSyDxD9VkYI7LLW2bpIuEIDQBpO63KpfM4+1u0hwAM4lWkR9Cd jdJOgqf6RO5zerWXQL9ygxEL2EEbe8Dd3LbWISIpadqU7swgpgbXkrL3+Y7PzuMz+OairvfBQbL baZQEIiRlcxqWsXSJuzu+X6R5+CWNil7SBI/4rmfjeYJEsdBKd1wwp1AaKRQWWTMnTmvZXOL2w+ llHotXPxAoJqtz1kKC3rCTqo= X-Received: by 2002:a05:620a:f11:b0:8f0:10b0:9e34 with SMTP id af79cd13be357-8fd18e26004mr1241694885a.8.1777944311485; Mon, 04 May 2026 18:25:11 -0700 (PDT) X-Received: by 2002:a05:620a:f11:b0:8f0:10b0:9e34 with SMTP id af79cd13be357-8fd18e26004mr1241691685a.8.1777944310854; Mon, 04 May 2026 18:25:10 -0700 (PDT) Received: from idea ([2600:4040:aa66:bf00:9e8e:99ff:fed1:71f]) by smtp.gmail.com with ESMTPSA id af79cd13be357-8fc29a80069sm1352688485a.15.2026.05.04.18.25.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 04 May 2026 18:25:10 -0700 (PDT) From: Patrick Palka <ppalka@redhat.com> To: gcc-patches@gcc.gnu.org Cc: jason@redhat.com, Patrick Palka <ppalka@redhat.com> Subject: [PATCH] c++/reflection: rewrite and memoize consteval_only_p [PR125179] Date: Mon, 4 May 2026 21:25:06 -0400 Message-ID: <20260505012506.748365-1-ppalka@redhat.com> X-Mailer: git-send-email 2.54.0.rc1.54.g60f07c4f5c MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: Dz8w0FdApvBIhCHw1djRKv88hm7tI5dU5qtcUe-vKz0_1777944312 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit content-type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-13.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> Errors-To: gcc-patches-bounces~patchwork=sourceware.org@gcc.gnu.org |
| Series |
c++/reflection: rewrite and memoize consteval_only_p [PR125179]
|
|
Commit Message
Patrick Palka
May 5, 2026, 1:25 a.m. UTC
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for trunk/16? -- >8 -- The TU in this PR exhibits a 40x compile-time slowdown with -freflection vs without, all due to consteval_only_p which happens to be quite slow on large intertwined classes due to its recursive walking of TYPE_FIELDS. This patch firstly rewrites the predicate to use direct recursion instead of cp_walk_tree so that it's easier to reason about and more closely mirrors the standard definition ([basic.types.general]/2). This patch also makes the predicate partially tristate, where the unknown state tracks whether we've seen an incomplete class type and therefore don't know if it's consteval-only. Finally this patch caches the result of the predicate for class types when the result is known (and the class type is complete). When the result is unknown then we must not cache so that a subsequent call to the predicate can try again. We also need to be able to cope with a class input that's not been laid out yet. With this patch compile time for the TU is now 1.15x with -freflection instead of 40x. PR c++/125179 gcc/cp/ChangeLog: * reflect.cc (consteval_only_type_r): Remove this cp_walk_tree callback and replace with ... (consteval_only_class_cache, consteval_only_p_1): ... this recursive memoized implementation. (consteval_only_p): Define in terms of consteval_only_p_1. --- gcc/cp/reflect.cc | 122 +++++++++++++++++++++++++++++----------------- 1 file changed, 78 insertions(+), 44 deletions(-)
Comments
On Mon, May 04, 2026 at 09:25:06PM -0400, Patrick Palka wrote: > + else if (RECORD_OR_UNION_TYPE_P (t)) > + { > + if (tree *slot = hash_map_safe_get (consteval_only_class_cache, t)) > + return *slot == boolean_true_node; > + > + if (class_set.add (t)) > + /* Handle struct A { A* p; } etc. */ > + return false; > + > + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown (); > + for (tree member = TYPE_FIELDS (t); member; member = DECL_CHAIN (member)) > + if (TREE_CODE (member) == FIELD_DECL) > + { > + r = r || consteval_only_p_1 (TREE_TYPE (member), class_set); > + if (r.is_true ()) > + break; > + } > + > + if (COMPLETE_TYPE_P (t)) > + { > + if (r.is_true ()) > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > + t, boolean_true_node); > + else if (r.is_false ()) > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > + t, boolean_false_node); > + } > + > + class_set.remove (t); Doesn't the above stmt result in really bad compile time complexity? If I have say struct S1; struct S2; struct S3 { S2 *a; }; struct S4 { S3 *a; }; struct S5 { S4 *a; }; ... struct S9999 { S9998 *a; }; struct S2 { S9999 *a; S1 *b; }; then when asking if S2 is consteval-only, this will walk ~10000^2 types rather than just ~10000 types. Wouldn't it be better to class_set.add (t) and never remove, but if we trigger that return false; in there, remember that we shouldn't cache r.is_false () results (perhaps with the exception when this was the non-recursive call)? Jakub
On Tue, 5 May 2026, Jakub Jelinek wrote: > On Mon, May 04, 2026 at 09:25:06PM -0400, Patrick Palka wrote: > > + else if (RECORD_OR_UNION_TYPE_P (t)) > > + { > > + if (tree *slot = hash_map_safe_get (consteval_only_class_cache, t)) > > + return *slot == boolean_true_node; > > + > > + if (class_set.add (t)) > > + /* Handle struct A { A* p; } etc. */ > > + return false; > > + > > + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown (); > > + for (tree member = TYPE_FIELDS (t); member; member = DECL_CHAIN (member)) > > + if (TREE_CODE (member) == FIELD_DECL) > > + { > > + r = r || consteval_only_p_1 (TREE_TYPE (member), class_set); > > + if (r.is_true ()) > > + break; > > + } > > + > > + if (COMPLETE_TYPE_P (t)) > > + { > > + if (r.is_true ()) > > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > > + t, boolean_true_node); > > + else if (r.is_false ()) > > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > > + t, boolean_false_node); > > + } > > + > > + class_set.remove (t); > > Doesn't the above stmt result in really bad compile time complexity? > If I have say > struct S1; > struct S2; > struct S3 { S2 *a; }; > struct S4 { S3 *a; }; > struct S5 { S4 *a; }; > ... > struct S9999 { S9998 *a; }; > struct S2 { S9999 *a; S1 *b; }; > then when asking if S2 is consteval-only, this will walk ~10000^2 types > rather than just ~10000 types. Wouldn't it be better to > class_set.add (t) and never remove, but if we trigger that return false; > in there, remember that we shouldn't cache r.is_false () results (perhaps > with the exception when this was the non-recursive call)? Yes, and the current patch is buggy for: struct B { A* p; }; struct A { B m; meta::info n; }; We'd incorrectly deem B not consteval-only if we first walk it from A. It's apparently important for the testcase from this PR to return false rather than unknown when detecting mutual recursion, otherwise we still get a 4x slowdown. But if we must return false in the case of mutual recursion then indeed it's not safe to cache r.is_false () results except for the outermost class because of scenarios like B above. To fix this efficiently I think we need to maintian both a class_seen set and a class_stack. What do you think of the following? -- >8 -- PR c++/125179 gcc/cp/ChangeLog: * reflect.cc: Include <algorithm>. (consteval_only_type_r): Remove this cp_walk_tree callback and replace with ... (consteval_only_p_state, consteval_only_class_cache) (consteval_only_p_1): ... this recursive memoized implementation. (consteval_only_p): Define in terms of consteval_only_p_1. --- gcc/cp/reflect.cc | 138 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 97 insertions(+), 41 deletions(-) diff --git a/gcc/cp/reflect.cc b/gcc/cp/reflect.cc index 3b9f56ea5484..67a1e1b85895 100644 --- a/gcc/cp/reflect.cc +++ b/gcc/cp/reflect.cc @@ -20,6 +20,7 @@ along with GCC; see the file COPYING3. If not see <http://www.gnu.org/licenses/>. */ #include "config.h" +#define INCLUDE_ALGORITHM #include "system.h" #include "coretypes.h" #include "target.h" @@ -8561,47 +8562,22 @@ splice (tree refl) return refl; } -/* A walker for consteval_only_p. It cannot be a lambda, because we - have to call this recursively, sigh. */ +/* State maintained during consteval_only_p_1 recursion. */ -static tree -consteval_only_type_r (tree *tp, int *walk_subtrees, void *data) +struct consteval_only_p_state { - tree t = *tp; - /* Types can contain themselves recursively, hence this. */ - auto visited = static_cast<hash_set<tree> *>(data); - - if (!TYPE_P (t)) - return NULL_TREE; - - if (REFLECTION_TYPE_P (t)) - return t; - - if (typedef_variant_p (t)) - /* Tell cp_walk_subtrees to look through typedefs. */ - *walk_subtrees = 2; - - if (RECORD_OR_UNION_TYPE_P (t)) - { - /* Don't walk template arguments; A<info>::type isn't a consteval-only - type. */ - *walk_subtrees = 0; - /* So we have to walk the fields manually. */ - for (tree member = TYPE_FIELDS (t); - member; member = DECL_CHAIN (member)) - if (TREE_CODE (member) == FIELD_DECL) - if (tree r = cp_walk_tree (&TREE_TYPE (member), - consteval_only_type_r, visited, visited)) - return r; - } + /* The stack of class types we're recursively inside. */ + auto_vec<tree> class_stack; + /* The set of class types we've seen. */ + hash_set<tree> class_seen; + /* Whether we've seen mutual class recursion. */ + bool seen_mutual_recursion = false; +}; - return NULL_TREE; -} +static tristate consteval_only_p_1 (tree, consteval_only_p_state&); -/* True if T is a consteval-only type as per [basic.types.general]: - "A type is consteval-only if it is either std::meta::info or a type - compounded from a consteval-only type", or something that has - a consteval-only type. */ +/* True if T is a consteval-only type as per [basic.types.general], or + is a declaration with such a type, or a TREE_VEC thereof. */ bool consteval_only_p (tree t) @@ -8612,7 +8588,7 @@ consteval_only_p (tree t) if (!TYPE_P (t)) t = TREE_TYPE (t); - if (!t) + if (!t || t == error_mark_node) return false; if (TREE_CODE (t) == TREE_VEC) @@ -8634,9 +8610,89 @@ consteval_only_p (tree t) which could be consteval-only, depending on T. */ t = complete_type (t); - /* Classes with std::meta::info members are also consteval-only. */ - hash_set<tree> visited; - return !!cp_walk_tree (&t, consteval_only_type_r, &visited, &visited); + consteval_only_p_state state; + return consteval_only_p_1 (t, state).is_true (); +} + +/* A cache of the boolean result of consteval_only_p_1 for class types, when + the result is known. */ + +static GTY((cache)) type_tree_cache_map *consteval_only_class_cache; + +/* Recursive workhorse of consteval_only_p. Returns true if T is definitely + consteval-only, false if it's definitely not, and unknown if we saw an + incomplete type and therefore don't know. */ + +static tristate +consteval_only_p_1 (tree t, consteval_only_p_state& state) +{ + t = TYPE_MAIN_VARIANT (t); + + if (REFLECTION_TYPE_P (t)) + return true; + else if (INDIRECT_TYPE_P (t)) + return consteval_only_p_1 (TREE_TYPE (t), state); + else if (TREE_CODE (t) == ARRAY_TYPE) + return consteval_only_p_1 (TREE_TYPE (t), state); + else if (FUNC_OR_METHOD_TYPE_P (t)) + { + tristate r = consteval_only_p_1 (TREE_TYPE (t), state); + for (tree parm = TYPE_ARG_TYPES (t); + parm != NULL_TREE && parm != void_list_node; + parm = TREE_CHAIN (parm)) + { + r = r || consteval_only_p_1 (TREE_VALUE (parm), state); + if (r.is_true ()) + break; + } + return r; + } + else if (RECORD_OR_UNION_TYPE_P (t)) + { + if (tree *slot = hash_map_safe_get (consteval_only_class_cache, t)) + return *slot == boolean_true_node; + + if (state.class_seen.add (t)) + { + auto begin = state.class_stack.begin (); + auto end = state.class_stack.end (); + auto it = std::find (begin, end, t); + if (it != end && it != &state.class_stack.last ()) + state.seen_mutual_recursion = true; + + return false; + } + state.class_stack.safe_push (t); + + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown (); + for (tree member = TYPE_FIELDS (t); member; member = DECL_CHAIN (member)) + if (TREE_CODE (member) == FIELD_DECL) + { + r = r || consteval_only_p_1 (TREE_TYPE (member), state); + if (r.is_true ()) + break; + } + + if (!COMPLETE_TYPE_P (t)) + /* Until the type is laid out, we can't trust that its TYPE_FIELDS + won't change. */; + else if (r.is_true ()) + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, + t, boolean_true_node); + else if (r.is_false () + && (state.class_stack.length () == 1 + || !state.seen_mutual_recursion)) + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, + t, boolean_false_node); + + state.class_stack.pop (); + return r; + } + else if (TYPE_PTRMEM_P (t)) + return (consteval_only_p_1 (TYPE_PTRMEM_CLASS_TYPE (t), state) + || consteval_only_p_1 (TYPE_PTRMEM_POINTED_TO_TYPE (t), state)); + else + return false; } /* A walker for check_out_of_consteval_use_r. It cannot be a lambda, because
On Tue, 5 May 2026, Patrick Palka wrote: > On Tue, 5 May 2026, Jakub Jelinek wrote: > > > On Mon, May 04, 2026 at 09:25:06PM -0400, Patrick Palka wrote: > > > + else if (RECORD_OR_UNION_TYPE_P (t)) > > > + { > > > + if (tree *slot = hash_map_safe_get (consteval_only_class_cache, t)) > > > + return *slot == boolean_true_node; > > > + > > > + if (class_set.add (t)) > > > + /* Handle struct A { A* p; } etc. */ > > > + return false; > > > + > > > + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown (); > > > + for (tree member = TYPE_FIELDS (t); member; member = DECL_CHAIN (member)) > > > + if (TREE_CODE (member) == FIELD_DECL) > > > + { > > > + r = r || consteval_only_p_1 (TREE_TYPE (member), class_set); > > > + if (r.is_true ()) > > > + break; > > > + } > > > + > > > + if (COMPLETE_TYPE_P (t)) > > > + { > > > + if (r.is_true ()) > > > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > > > + t, boolean_true_node); > > > + else if (r.is_false ()) > > > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > > > + t, boolean_false_node); > > > + } > > > + > > > + class_set.remove (t); > > > > Doesn't the above stmt result in really bad compile time complexity? > > If I have say > > struct S1; > > struct S2; > > struct S3 { S2 *a; }; > > struct S4 { S3 *a; }; > > struct S5 { S4 *a; }; > > ... > > struct S9999 { S9998 *a; }; > > struct S2 { S9999 *a; S1 *b; }; > > then when asking if S2 is consteval-only, this will walk ~10000^2 types > > rather than just ~10000 types. Wouldn't it be better to > > class_set.add (t) and never remove, but if we trigger that return false; > > in there, remember that we shouldn't cache r.is_false () results (perhaps > > with the exception when this was the non-recursive call)? > > Yes, and the current patch is buggy for: > > struct B { A* p; }; > struct A { > B m; > meta::info n; > }; > > We'd incorrectly deem B not consteval-only if we first walk it from A. > > It's apparently important for the testcase from this PR to return false > rather than unknown when detecting mutual recursion, otherwise we still > get a 4x slowdown. The reason it's important to optimistically return false upon hitting mutual recursion is so that we get a chance to cache the result. Otherwise we'll never cache the result for some mutually recursive types; consteval_only_p_1 will always return unknown. > But if we must return false in the case of mutual > recursion then indeed it's not safe to cache r.is_false () results > except for the outermost class because of scenarios like B above. > > To fix this efficiently I think we need to maintian both a class_seen > set and a class_stack. What do you think of the following? > > -- >8 -- > > PR c++/125179 > > gcc/cp/ChangeLog: > > * reflect.cc: Include <algorithm>. > (consteval_only_type_r): Remove this cp_walk_tree callback and > replace with ... > (consteval_only_p_state, consteval_only_class_cache) > (consteval_only_p_1): ... this recursive memoized implementation. > (consteval_only_p): Define in terms of consteval_only_p_1. > --- > gcc/cp/reflect.cc | 138 ++++++++++++++++++++++++++++++++-------------- > 1 file changed, 97 insertions(+), 41 deletions(-) > > diff --git a/gcc/cp/reflect.cc b/gcc/cp/reflect.cc > index 3b9f56ea5484..67a1e1b85895 100644 > --- a/gcc/cp/reflect.cc > +++ b/gcc/cp/reflect.cc > @@ -20,6 +20,7 @@ along with GCC; see the file COPYING3. If not see > <http://www.gnu.org/licenses/>. */ > > #include "config.h" > +#define INCLUDE_ALGORITHM > #include "system.h" > #include "coretypes.h" > #include "target.h" > @@ -8561,47 +8562,22 @@ splice (tree refl) > return refl; > } > > -/* A walker for consteval_only_p. It cannot be a lambda, because we > - have to call this recursively, sigh. */ > +/* State maintained during consteval_only_p_1 recursion. */ > > -static tree > -consteval_only_type_r (tree *tp, int *walk_subtrees, void *data) > +struct consteval_only_p_state > { > - tree t = *tp; > - /* Types can contain themselves recursively, hence this. */ > - auto visited = static_cast<hash_set<tree> *>(data); > - > - if (!TYPE_P (t)) > - return NULL_TREE; > - > - if (REFLECTION_TYPE_P (t)) > - return t; > - > - if (typedef_variant_p (t)) > - /* Tell cp_walk_subtrees to look through typedefs. */ > - *walk_subtrees = 2; > - > - if (RECORD_OR_UNION_TYPE_P (t)) > - { > - /* Don't walk template arguments; A<info>::type isn't a consteval-only > - type. */ > - *walk_subtrees = 0; > - /* So we have to walk the fields manually. */ > - for (tree member = TYPE_FIELDS (t); > - member; member = DECL_CHAIN (member)) > - if (TREE_CODE (member) == FIELD_DECL) > - if (tree r = cp_walk_tree (&TREE_TYPE (member), > - consteval_only_type_r, visited, visited)) > - return r; > - } > + /* The stack of class types we're recursively inside. */ > + auto_vec<tree> class_stack; > + /* The set of class types we've seen. */ > + hash_set<tree> class_seen; > + /* Whether we've seen mutual class recursion. */ > + bool seen_mutual_recursion = false; > +}; > > - return NULL_TREE; > -} > +static tristate consteval_only_p_1 (tree, consteval_only_p_state&); > > -/* True if T is a consteval-only type as per [basic.types.general]: > - "A type is consteval-only if it is either std::meta::info or a type > - compounded from a consteval-only type", or something that has > - a consteval-only type. */ > +/* True if T is a consteval-only type as per [basic.types.general], or > + is a declaration with such a type, or a TREE_VEC thereof. */ > > bool > consteval_only_p (tree t) > @@ -8612,7 +8588,7 @@ consteval_only_p (tree t) > if (!TYPE_P (t)) > t = TREE_TYPE (t); > > - if (!t) > + if (!t || t == error_mark_node) > return false; > > if (TREE_CODE (t) == TREE_VEC) > @@ -8634,9 +8610,89 @@ consteval_only_p (tree t) > which could be consteval-only, depending on T. */ > t = complete_type (t); > > - /* Classes with std::meta::info members are also consteval-only. */ > - hash_set<tree> visited; > - return !!cp_walk_tree (&t, consteval_only_type_r, &visited, &visited); > + consteval_only_p_state state; > + return consteval_only_p_1 (t, state).is_true (); > +} > + > +/* A cache of the boolean result of consteval_only_p_1 for class types, when > + the result is known. */ > + > +static GTY((cache)) type_tree_cache_map *consteval_only_class_cache; > + > +/* Recursive workhorse of consteval_only_p. Returns true if T is definitely > + consteval-only, false if it's definitely not, and unknown if we saw an > + incomplete type and therefore don't know. */ > + > +static tristate > +consteval_only_p_1 (tree t, consteval_only_p_state& state) > +{ > + t = TYPE_MAIN_VARIANT (t); > + > + if (REFLECTION_TYPE_P (t)) > + return true; > + else if (INDIRECT_TYPE_P (t)) > + return consteval_only_p_1 (TREE_TYPE (t), state); > + else if (TREE_CODE (t) == ARRAY_TYPE) > + return consteval_only_p_1 (TREE_TYPE (t), state); > + else if (FUNC_OR_METHOD_TYPE_P (t)) > + { > + tristate r = consteval_only_p_1 (TREE_TYPE (t), state); > + for (tree parm = TYPE_ARG_TYPES (t); > + parm != NULL_TREE && parm != void_list_node; > + parm = TREE_CHAIN (parm)) > + { > + r = r || consteval_only_p_1 (TREE_VALUE (parm), state); > + if (r.is_true ()) > + break; > + } > + return r; > + } > + else if (RECORD_OR_UNION_TYPE_P (t)) > + { > + if (tree *slot = hash_map_safe_get (consteval_only_class_cache, t)) > + return *slot == boolean_true_node; > + > + if (state.class_seen.add (t)) > + { > + auto begin = state.class_stack.begin (); > + auto end = state.class_stack.end (); > + auto it = std::find (begin, end, t); > + if (it != end && it != &state.class_stack.last ()) > + state.seen_mutual_recursion = true; > + > + return false; > + } > + state.class_stack.safe_push (t); > + > + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown (); > + for (tree member = TYPE_FIELDS (t); member; member = DECL_CHAIN (member)) > + if (TREE_CODE (member) == FIELD_DECL) > + { > + r = r || consteval_only_p_1 (TREE_TYPE (member), state); > + if (r.is_true ()) > + break; > + } > + > + if (!COMPLETE_TYPE_P (t)) > + /* Until the type is laid out, we can't trust that its TYPE_FIELDS > + won't change. */; > + else if (r.is_true ()) > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > + t, boolean_true_node); > + else if (r.is_false () > + && (state.class_stack.length () == 1 > + || !state.seen_mutual_recursion)) D'oh, this !state.seen_mutual_recursion exception for caching nested types is wrong. struct A { struct B { struct C *c; } b; struct C { B b; } c; }; Here if we start walking from A, which is not mutually recursively, we first correctly deem B as unknown-consteval-only, but then go on to deem the nested C as not consteval only _and_ cache it (since there's no mutual recursion). So being optimistic about mutually recursive types (the 'if (class_seen.add (t)) return false' early exit) is at odds with caching the result for nested class types in general, unless we significantly complicate things which is probably not worth it. Here is v3 which removes the seen_mutual_recursion logic and adds some more comments: -- >8 -- PR c++/125179 gcc/cp/ChangeLog: * reflect.cc: Include <algorithm>. (consteval_only_type_r): Remove this cp_walk_tree callback and replace with ... (consteval_only_p_state, consteval_only_class_cache) (consteval_only_p_1): ... this recursive memoized implementation. (consteval_only_p): Define in terms of consteval_only_p_1. --- gcc/cp/reflect.cc | 131 +++++++++++++++++++++++++++++++--------------- 1 file changed, 90 insertions(+), 41 deletions(-) diff --git a/gcc/cp/reflect.cc b/gcc/cp/reflect.cc index 3b9f56ea5484..051f80902972 100644 --- a/gcc/cp/reflect.cc +++ b/gcc/cp/reflect.cc @@ -20,6 +20,7 @@ along with GCC; see the file COPYING3. If not see <http://www.gnu.org/licenses/>. */ #include "config.h" +#define INCLUDE_ALGORITHM #include "system.h" #include "coretypes.h" #include "target.h" @@ -8561,47 +8562,20 @@ splice (tree refl) return refl; } -/* A walker for consteval_only_p. It cannot be a lambda, because we - have to call this recursively, sigh. */ +/* State maintained during consteval_only_p_1 recursion. */ -static tree -consteval_only_type_r (tree *tp, int *walk_subtrees, void *data) +struct consteval_only_p_state { - tree t = *tp; - /* Types can contain themselves recursively, hence this. */ - auto visited = static_cast<hash_set<tree> *>(data); - - if (!TYPE_P (t)) - return NULL_TREE; - - if (REFLECTION_TYPE_P (t)) - return t; - - if (typedef_variant_p (t)) - /* Tell cp_walk_subtrees to look through typedefs. */ - *walk_subtrees = 2; - - if (RECORD_OR_UNION_TYPE_P (t)) - { - /* Don't walk template arguments; A<info>::type isn't a consteval-only - type. */ - *walk_subtrees = 0; - /* So we have to walk the fields manually. */ - for (tree member = TYPE_FIELDS (t); - member; member = DECL_CHAIN (member)) - if (TREE_CODE (member) == FIELD_DECL) - if (tree r = cp_walk_tree (&TREE_TYPE (member), - consteval_only_type_r, visited, visited)) - return r; - } + /* The stack of class types we're recursively inside. */ + auto_vec<tree> class_stack; + /* The set of class types we've seen. */ + hash_set<tree> class_seen; +}; - return NULL_TREE; -} +static tristate consteval_only_p_1 (tree, consteval_only_p_state&); -/* True if T is a consteval-only type as per [basic.types.general]: - "A type is consteval-only if it is either std::meta::info or a type - compounded from a consteval-only type", or something that has - a consteval-only type. */ +/* True if T is a consteval-only type as per [basic.types.general], or + is a declaration with such a type, or a TREE_VEC thereof. */ bool consteval_only_p (tree t) @@ -8612,7 +8586,7 @@ consteval_only_p (tree t) if (!TYPE_P (t)) t = TREE_TYPE (t); - if (!t) + if (!t || t == error_mark_node) return false; if (TREE_CODE (t) == TREE_VEC) @@ -8634,9 +8608,84 @@ consteval_only_p (tree t) which could be consteval-only, depending on T. */ t = complete_type (t); - /* Classes with std::meta::info members are also consteval-only. */ - hash_set<tree> visited; - return !!cp_walk_tree (&t, consteval_only_type_r, &visited, &visited); + consteval_only_p_state state; + return consteval_only_p_1 (t, state).is_true (); +} + +/* A cache of the boolean result of consteval_only_p_1 for class types, when + the result is known. */ + +static GTY((cache)) type_tree_cache_map *consteval_only_class_cache; + +/* Recursive workhorse of consteval_only_p. Returns true if T is definitely + consteval-only, false if it's definitely not, and unknown if we saw an + incomplete type and therefore don't know. */ + +static tristate +consteval_only_p_1 (tree t, consteval_only_p_state& state) +{ + t = TYPE_MAIN_VARIANT (t); + + if (REFLECTION_TYPE_P (t)) + return true; + else if (INDIRECT_TYPE_P (t)) + return consteval_only_p_1 (TREE_TYPE (t), state); + else if (TREE_CODE (t) == ARRAY_TYPE) + return consteval_only_p_1 (TREE_TYPE (t), state); + else if (FUNC_OR_METHOD_TYPE_P (t)) + { + tristate r = consteval_only_p_1 (TREE_TYPE (t), state); + for (tree parm = TYPE_ARG_TYPES (t); + parm != NULL_TREE && parm != void_list_node; + parm = TREE_CHAIN (parm)) + { + r = r || consteval_only_p_1 (TREE_VALUE (parm), state); + if (r.is_true ()) + break; + } + return r; + } + else if (RECORD_OR_UNION_TYPE_P (t)) + { + if (tree *slot = hash_map_safe_get (consteval_only_class_cache, t)) + return *slot == boolean_true_node; + + if (state.class_seen.add (t)) + /* Optimistically assume this consteval-unknown type is not consteval + only, for sake of mutually recursive class types. */ + return false; + state.class_stack.safe_push (t); + + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown (); + for (tree member = TYPE_FIELDS (t); member; member = DECL_CHAIN (member)) + if (TREE_CODE (member) == FIELD_DECL) + { + r = r || consteval_only_p_1 (TREE_TYPE (member), state); + if (r.is_true ()) + break; + } + + if (!COMPLETE_TYPE_P (t)) + /* Until the type is laid out, we can't trust that its TYPE_FIELDS + won't change. */; + else if (r.is_true ()) + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, + t, boolean_true_node); + else if (r.is_false () + /* The optimistic assumption above is at odds with caching + 'false' results for a nested class type. */ + && state.class_stack.length () == 1) + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, + t, boolean_false_node); + + state.class_stack.pop (); + return r; + } + else if (TYPE_PTRMEM_P (t)) + return (consteval_only_p_1 (TYPE_PTRMEM_CLASS_TYPE (t), state) + || consteval_only_p_1 (TYPE_PTRMEM_POINTED_TO_TYPE (t), state)); + else + return false; } /* A walker for check_out_of_consteval_use_r. It cannot be a lambda, because
On Tue, 5 May 2026, Patrick Palka wrote: > On Tue, 5 May 2026, Patrick Palka wrote: > > > On Tue, 5 May 2026, Jakub Jelinek wrote: > > > > > On Mon, May 04, 2026 at 09:25:06PM -0400, Patrick Palka wrote: > > > > + else if (RECORD_OR_UNION_TYPE_P (t)) > > > > + { > > > > + if (tree *slot = hash_map_safe_get (consteval_only_class_cache, t)) > > > > + return *slot == boolean_true_node; > > > > + > > > > + if (class_set.add (t)) > > > > + /* Handle struct A { A* p; } etc. */ > > > > + return false; > > > > + > > > > + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown (); > > > > + for (tree member = TYPE_FIELDS (t); member; member = DECL_CHAIN (member)) > > > > + if (TREE_CODE (member) == FIELD_DECL) > > > > + { > > > > + r = r || consteval_only_p_1 (TREE_TYPE (member), class_set); > > > > + if (r.is_true ()) > > > > + break; > > > > + } > > > > + > > > > + if (COMPLETE_TYPE_P (t)) > > > > + { > > > > + if (r.is_true ()) > > > > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > > > > + t, boolean_true_node); > > > > + else if (r.is_false ()) > > > > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > > > > + t, boolean_false_node); > > > > + } > > > > + > > > > + class_set.remove (t); > > > > > > Doesn't the above stmt result in really bad compile time complexity? > > > If I have say > > > struct S1; > > > struct S2; > > > struct S3 { S2 *a; }; > > > struct S4 { S3 *a; }; > > > struct S5 { S4 *a; }; > > > ... > > > struct S9999 { S9998 *a; }; > > > struct S2 { S9999 *a; S1 *b; }; > > > then when asking if S2 is consteval-only, this will walk ~10000^2 types > > > rather than just ~10000 types. Wouldn't it be better to > > > class_set.add (t) and never remove, but if we trigger that return false; > > > in there, remember that we shouldn't cache r.is_false () results (perhaps > > > with the exception when this was the non-recursive call)? > > > > Yes, and the current patch is buggy for: > > > > struct B { A* p; }; > > struct A { > > B m; > > meta::info n; > > }; > > > > We'd incorrectly deem B not consteval-only if we first walk it from A. > > > > It's apparently important for the testcase from this PR to return false > > rather than unknown when detecting mutual recursion, otherwise we still > > get a 4x slowdown. > > The reason it's important to optimistically return false upon hitting > mutual recursion is so that we get a chance to cache the result. > Otherwise we'll never cache the result for some mutually recursive > types; consteval_only_p_1 will always return unknown. > > > But if we must return false in the case of mutual > > recursion then indeed it's not safe to cache r.is_false () results > > except for the outermost class because of scenarios like B above. > > > > To fix this efficiently I think we need to maintian both a class_seen > > set and a class_stack. What do you think of the following? > > > > -- >8 -- > > > > PR c++/125179 > > > > gcc/cp/ChangeLog: > > > > * reflect.cc: Include <algorithm>. > > (consteval_only_type_r): Remove this cp_walk_tree callback and > > replace with ... > > (consteval_only_p_state, consteval_only_class_cache) > > (consteval_only_p_1): ... this recursive memoized implementation. > > (consteval_only_p): Define in terms of consteval_only_p_1. > > --- > > gcc/cp/reflect.cc | 138 ++++++++++++++++++++++++++++++++-------------- > > 1 file changed, 97 insertions(+), 41 deletions(-) > > > > diff --git a/gcc/cp/reflect.cc b/gcc/cp/reflect.cc > > index 3b9f56ea5484..67a1e1b85895 100644 > > --- a/gcc/cp/reflect.cc > > +++ b/gcc/cp/reflect.cc > > @@ -20,6 +20,7 @@ along with GCC; see the file COPYING3. If not see > > <http://www.gnu.org/licenses/>. */ > > > > #include "config.h" > > +#define INCLUDE_ALGORITHM > > #include "system.h" > > #include "coretypes.h" > > #include "target.h" > > @@ -8561,47 +8562,22 @@ splice (tree refl) > > return refl; > > } > > > > -/* A walker for consteval_only_p. It cannot be a lambda, because we > > - have to call this recursively, sigh. */ > > +/* State maintained during consteval_only_p_1 recursion. */ > > > > -static tree > > -consteval_only_type_r (tree *tp, int *walk_subtrees, void *data) > > +struct consteval_only_p_state > > { > > - tree t = *tp; > > - /* Types can contain themselves recursively, hence this. */ > > - auto visited = static_cast<hash_set<tree> *>(data); > > - > > - if (!TYPE_P (t)) > > - return NULL_TREE; > > - > > - if (REFLECTION_TYPE_P (t)) > > - return t; > > - > > - if (typedef_variant_p (t)) > > - /* Tell cp_walk_subtrees to look through typedefs. */ > > - *walk_subtrees = 2; > > - > > - if (RECORD_OR_UNION_TYPE_P (t)) > > - { > > - /* Don't walk template arguments; A<info>::type isn't a consteval-only > > - type. */ > > - *walk_subtrees = 0; > > - /* So we have to walk the fields manually. */ > > - for (tree member = TYPE_FIELDS (t); > > - member; member = DECL_CHAIN (member)) > > - if (TREE_CODE (member) == FIELD_DECL) > > - if (tree r = cp_walk_tree (&TREE_TYPE (member), > > - consteval_only_type_r, visited, visited)) > > - return r; > > - } > > + /* The stack of class types we're recursively inside. */ > > + auto_vec<tree> class_stack; > > + /* The set of class types we've seen. */ > > + hash_set<tree> class_seen; > > + /* Whether we've seen mutual class recursion. */ > > + bool seen_mutual_recursion = false; > > > +}; > > > > - return NULL_TREE; > > -} > > +static tristate consteval_only_p_1 (tree, consteval_only_p_state&); > > > > -/* True if T is a consteval-only type as per [basic.types.general]: > > - "A type is consteval-only if it is either std::meta::info or a type > > - compounded from a consteval-only type", or something that has > > - a consteval-only type. */ > > +/* True if T is a consteval-only type as per [basic.types.general], or > > + is a declaration with such a type, or a TREE_VEC thereof. */ > > > > bool > > consteval_only_p (tree t) > > @@ -8612,7 +8588,7 @@ consteval_only_p (tree t) > > if (!TYPE_P (t)) > > t = TREE_TYPE (t); > > > > - if (!t) > > + if (!t || t == error_mark_node) > > return false; > > > > if (TREE_CODE (t) == TREE_VEC) > > @@ -8634,9 +8610,89 @@ consteval_only_p (tree t) > > which could be consteval-only, depending on T. */ > > t = complete_type (t); > > > > - /* Classes with std::meta::info members are also consteval-only. */ > > - hash_set<tree> visited; > > - return !!cp_walk_tree (&t, consteval_only_type_r, &visited, &visited); > > + consteval_only_p_state state; > > + return consteval_only_p_1 (t, state).is_true (); > > +} > > + > > +/* A cache of the boolean result of consteval_only_p_1 for class types, when > > + the result is known. */ > > + > > +static GTY((cache)) type_tree_cache_map *consteval_only_class_cache; > > + > > +/* Recursive workhorse of consteval_only_p. Returns true if T is definitely > > + consteval-only, false if it's definitely not, and unknown if we saw an > > + incomplete type and therefore don't know. */ > > + > > +static tristate > > +consteval_only_p_1 (tree t, consteval_only_p_state& state) > > +{ > > + t = TYPE_MAIN_VARIANT (t); > > + > > + if (REFLECTION_TYPE_P (t)) > > + return true; > > + else if (INDIRECT_TYPE_P (t)) > > + return consteval_only_p_1 (TREE_TYPE (t), state); > > + else if (TREE_CODE (t) == ARRAY_TYPE) > > + return consteval_only_p_1 (TREE_TYPE (t), state); > > + else if (FUNC_OR_METHOD_TYPE_P (t)) > > + { > > + tristate r = consteval_only_p_1 (TREE_TYPE (t), state); > > + for (tree parm = TYPE_ARG_TYPES (t); > > + parm != NULL_TREE && parm != void_list_node; > > + parm = TREE_CHAIN (parm)) > > + { > > + r = r || consteval_only_p_1 (TREE_VALUE (parm), state); > > + if (r.is_true ()) > > + break; > > + } > > + return r; > > + } > > + else if (RECORD_OR_UNION_TYPE_P (t)) > > + { > > + if (tree *slot = hash_map_safe_get (consteval_only_class_cache, t)) > > + return *slot == boolean_true_node; > > + > > + if (state.class_seen.add (t)) > > + { > > + auto begin = state.class_stack.begin (); > > + auto end = state.class_stack.end (); > > + auto it = std::find (begin, end, t); > > + if (it != end && it != &state.class_stack.last ()) > > + state.seen_mutual_recursion = true; > > + > > + return false; > > + } > > + state.class_stack.safe_push (t); > > + > > + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown (); > > + for (tree member = TYPE_FIELDS (t); member; member = DECL_CHAIN (member)) > > + if (TREE_CODE (member) == FIELD_DECL) > > + { > > + r = r || consteval_only_p_1 (TREE_TYPE (member), state); > > + if (r.is_true ()) > > + break; > > + } > > + > > + if (!COMPLETE_TYPE_P (t)) > > + /* Until the type is laid out, we can't trust that its TYPE_FIELDS > > + won't change. */; > > + else if (r.is_true ()) > > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > > + t, boolean_true_node); > > + else if (r.is_false () > > + && (state.class_stack.length () == 1 > > + || !state.seen_mutual_recursion)) > > D'oh, this !state.seen_mutual_recursion exception for caching nested > types is wrong. > > struct A { > struct B { struct C *c; } b; > struct C { B b; } c; > }; > > Here if we start walking from A, which is not mutually recursively, we > first correctly deem B as unknown-consteval-only, but then go on to > deem the nested C as not consteval only _and_ cache it (since there's > no mutual recursion). > > So being optimistic about mutually recursive types (the > 'if (class_seen.add (t)) return false' early exit) is at odds > with caching the result for nested class types in general, unless > we significantly complicate things which is probably not worth it. Oh, we can just remember if we ever took the early exit and only disable false result caching for nested classes in that case. That should be safe, I think (famous last words), and allow us to cache nested classes in more cases. So for struct A { struct B { int n; } b; }; the false result for B will be cached, but for struct A { struct B { A* a; } b; struct C { int n; } c; }; neither the false result for B or for C will be cached. Here is v4 that introduces an 'optimistic_p' flag to that effect. This reduces the -freflection slowdown for the TU to 1.1x from 1.2x compared to v3. -- >8 -- PR c++/125179 gcc/cp/ChangeLog: * reflect.cc: (consteval_only_type_r): Remove this cp_walk_tree callback and replace with ... (consteval_only_p_state, consteval_only_class_cache) (consteval_only_p_1): ... this recursive memoized implementation. (consteval_only_p): Define in terms of consteval_only_p_1. --- gcc/cp/reflect.cc | 136 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 95 insertions(+), 41 deletions(-) diff --git a/gcc/cp/reflect.cc b/gcc/cp/reflect.cc index 3b9f56ea5484..e9bd301fb84b 100644 --- a/gcc/cp/reflect.cc +++ b/gcc/cp/reflect.cc @@ -8561,47 +8561,23 @@ splice (tree refl) return refl; } -/* A walker for consteval_only_p. It cannot be a lambda, because we - have to call this recursively, sigh. */ +/* State maintained during consteval_only_p_1 recursion. */ -static tree -consteval_only_type_r (tree *tp, int *walk_subtrees, void *data) +struct consteval_only_p_state { - tree t = *tp; - /* Types can contain themselves recursively, hence this. */ - auto visited = static_cast<hash_set<tree> *>(data); - - if (!TYPE_P (t)) - return NULL_TREE; - - if (REFLECTION_TYPE_P (t)) - return t; - - if (typedef_variant_p (t)) - /* Tell cp_walk_subtrees to look through typedefs. */ - *walk_subtrees = 2; - - if (RECORD_OR_UNION_TYPE_P (t)) - { - /* Don't walk template arguments; A<info>::type isn't a consteval-only - type. */ - *walk_subtrees = 0; - /* So we have to walk the fields manually. */ - for (tree member = TYPE_FIELDS (t); - member; member = DECL_CHAIN (member)) - if (TREE_CODE (member) == FIELD_DECL) - if (tree r = cp_walk_tree (&TREE_TYPE (member), - consteval_only_type_r, visited, visited)) - return r; - } + /* The stack of class types we're recursively inside. */ + auto_vec<tree> class_stack; + /* The set of class types we've seen. */ + hash_set<tree> class_seen; + /* True if we've optimistically assumed an already-seen + consteval-unknown class type is not consteval. */ + bool optimistic_p = false; +}; - return NULL_TREE; -} +static tristate consteval_only_p_1 (tree, consteval_only_p_state&); -/* True if T is a consteval-only type as per [basic.types.general]: - "A type is consteval-only if it is either std::meta::info or a type - compounded from a consteval-only type", or something that has - a consteval-only type. */ +/* True if T is a consteval-only type as per [basic.types.general], or + is a declaration with such a type, or a TREE_VEC thereof. */ bool consteval_only_p (tree t) @@ -8612,7 +8588,7 @@ consteval_only_p (tree t) if (!TYPE_P (t)) t = TREE_TYPE (t); - if (!t) + if (!t || t == error_mark_node) return false; if (TREE_CODE (t) == TREE_VEC) @@ -8634,9 +8610,87 @@ consteval_only_p (tree t) which could be consteval-only, depending on T. */ t = complete_type (t); - /* Classes with std::meta::info members are also consteval-only. */ - hash_set<tree> visited; - return !!cp_walk_tree (&t, consteval_only_type_r, &visited, &visited); + consteval_only_p_state state; + return consteval_only_p_1 (t, state).is_true (); +} + +/* A cache of the boolean result of consteval_only_p_1 for class types, when + the result is known. */ + +static GTY((cache)) type_tree_cache_map *consteval_only_class_cache; + +/* Recursive workhorse of consteval_only_p. Returns true if T is definitely + consteval-only, false if it's definitely not, and unknown if we saw an + incomplete type and therefore don't know. */ + +static tristate +consteval_only_p_1 (tree t, consteval_only_p_state& state) +{ + t = TYPE_MAIN_VARIANT (t); + + if (REFLECTION_TYPE_P (t)) + return true; + else if (INDIRECT_TYPE_P (t)) + return consteval_only_p_1 (TREE_TYPE (t), state); + else if (TREE_CODE (t) == ARRAY_TYPE) + return consteval_only_p_1 (TREE_TYPE (t), state); + else if (FUNC_OR_METHOD_TYPE_P (t)) + { + tristate r = consteval_only_p_1 (TREE_TYPE (t), state); + for (tree parm = TYPE_ARG_TYPES (t); + parm != NULL_TREE && parm != void_list_node; + parm = TREE_CHAIN (parm)) + { + r = r || consteval_only_p_1 (TREE_VALUE (parm), state); + if (r.is_true ()) + break; + } + return r; + } + else if (RECORD_OR_UNION_TYPE_P (t)) + { + if (tree *slot = hash_map_safe_get (consteval_only_class_cache, t)) + return *slot == boolean_true_node; + + if (state.class_seen.add (t)) + { + /* Optimistically assume this already seen consteval-unknown class is + not consteval only, for sake of mutually recursive classes. */ + state.optimistic_p = true; + return false; + } + state.class_stack.safe_push (t); + + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown (); + for (tree member = TYPE_FIELDS (t); member; member = DECL_CHAIN (member)) + if (TREE_CODE (member) == FIELD_DECL) + { + r = r || consteval_only_p_1 (TREE_TYPE (member), state); + if (r.is_true ()) + break; + } + + if (!COMPLETE_TYPE_P (t)) + /* Until the type is laid out, we can't trust that its TYPE_FIELDS + won't change. */; + else if (r.is_true ()) + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, + t, boolean_true_node); + else if (r.is_false () + /* The optimistic assumption above is at odds with caching + 'false' results for a nested class type. */ + && (state.class_stack.length () == 1 || !state.optimistic_p)) + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, + t, boolean_false_node); + + state.class_stack.pop (); + return r; + } + else if (TYPE_PTRMEM_P (t)) + return (consteval_only_p_1 (TYPE_PTRMEM_CLASS_TYPE (t), state) + || consteval_only_p_1 (TYPE_PTRMEM_POINTED_TO_TYPE (t), state)); + else + return false; } /* A walker for check_out_of_consteval_use_r. It cannot be a lambda, because
On 5/5/26 11:21 AM, Patrick Palka wrote: > On Tue, 5 May 2026, Patrick Palka wrote: > >> On Tue, 5 May 2026, Patrick Palka wrote: >> >>> On Tue, 5 May 2026, Jakub Jelinek wrote: >>> >>>> On Mon, May 04, 2026 at 09:25:06PM -0400, Patrick Palka wrote: >>>>> + else if (RECORD_OR_UNION_TYPE_P (t)) >>>>> + { >>>>> + if (tree *slot = hash_map_safe_get (consteval_only_class_cache, t)) >>>>> + return *slot == boolean_true_node; >>>>> + >>>>> + if (class_set.add (t)) >>>>> + /* Handle struct A { A* p; } etc. */ >>>>> + return false; >>>>> + >>>>> + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown (); >>>>> + for (tree member = TYPE_FIELDS (t); member; member = DECL_CHAIN (member)) >>>>> + if (TREE_CODE (member) == FIELD_DECL) >>>>> + { >>>>> + r = r || consteval_only_p_1 (TREE_TYPE (member), class_set); >>>>> + if (r.is_true ()) >>>>> + break; >>>>> + } >>>>> + >>>>> + if (COMPLETE_TYPE_P (t)) >>>>> + { >>>>> + if (r.is_true ()) >>>>> + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, >>>>> + t, boolean_true_node); >>>>> + else if (r.is_false ()) >>>>> + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, >>>>> + t, boolean_false_node); >>>>> + } >>>>> + >>>>> + class_set.remove (t); >>>> >>>> Doesn't the above stmt result in really bad compile time complexity? >>>> If I have say >>>> struct S1; >>>> struct S2; >>>> struct S3 { S2 *a; }; >>>> struct S4 { S3 *a; }; >>>> struct S5 { S4 *a; }; >>>> ... >>>> struct S9999 { S9998 *a; }; >>>> struct S2 { S9999 *a; S1 *b; }; >>>> then when asking if S2 is consteval-only, this will walk ~10000^2 types >>>> rather than just ~10000 types. Wouldn't it be better to >>>> class_set.add (t) and never remove, but if we trigger that return false; >>>> in there, remember that we shouldn't cache r.is_false () results (perhaps >>>> with the exception when this was the non-recursive call)? >>> >>> Yes, and the current patch is buggy for: >>> >>> struct B { A* p; }; >>> struct A { >>> B m; >>> meta::info n; >>> }; >>> >>> We'd incorrectly deem B not consteval-only if we first walk it from A. >>> >>> It's apparently important for the testcase from this PR to return false >>> rather than unknown when detecting mutual recursion, otherwise we still >>> get a 4x slowdown. >> >> The reason it's important to optimistically return false upon hitting >> mutual recursion is so that we get a chance to cache the result. >> Otherwise we'll never cache the result for some mutually recursive >> types; consteval_only_p_1 will always return unknown. >> >>> But if we must return false in the case of mutual >>> recursion then indeed it's not safe to cache r.is_false () results >>> except for the outermost class because of scenarios like B above. >>> >>> To fix this efficiently I think we need to maintian both a class_seen >>> set and a class_stack. What do you think of the following? >>> >>> -- >8 -- >>> >>> PR c++/125179 >>> >>> gcc/cp/ChangeLog: >>> >>> * reflect.cc: Include <algorithm>. >>> (consteval_only_type_r): Remove this cp_walk_tree callback and >>> replace with ... >>> (consteval_only_p_state, consteval_only_class_cache) >>> (consteval_only_p_1): ... this recursive memoized implementation. >>> (consteval_only_p): Define in terms of consteval_only_p_1. >>> --- >>> gcc/cp/reflect.cc | 138 ++++++++++++++++++++++++++++++++-------------- >>> 1 file changed, 97 insertions(+), 41 deletions(-) >>> >>> diff --git a/gcc/cp/reflect.cc b/gcc/cp/reflect.cc >>> index 3b9f56ea5484..67a1e1b85895 100644 >>> --- a/gcc/cp/reflect.cc >>> +++ b/gcc/cp/reflect.cc >>> @@ -20,6 +20,7 @@ along with GCC; see the file COPYING3. If not see >>> <http://www.gnu.org/licenses/>. */ >>> >>> #include "config.h" >>> +#define INCLUDE_ALGORITHM >>> #include "system.h" >>> #include "coretypes.h" >>> #include "target.h" >>> @@ -8561,47 +8562,22 @@ splice (tree refl) >>> return refl; >>> } >>> >>> -/* A walker for consteval_only_p. It cannot be a lambda, because we >>> - have to call this recursively, sigh. */ >>> +/* State maintained during consteval_only_p_1 recursion. */ >>> >>> -static tree >>> -consteval_only_type_r (tree *tp, int *walk_subtrees, void *data) >>> +struct consteval_only_p_state >>> { >>> - tree t = *tp; >>> - /* Types can contain themselves recursively, hence this. */ >>> - auto visited = static_cast<hash_set<tree> *>(data); >>> - >>> - if (!TYPE_P (t)) >>> - return NULL_TREE; >>> - >>> - if (REFLECTION_TYPE_P (t)) >>> - return t; >>> - >>> - if (typedef_variant_p (t)) >>> - /* Tell cp_walk_subtrees to look through typedefs. */ >>> - *walk_subtrees = 2; >>> - >>> - if (RECORD_OR_UNION_TYPE_P (t)) >>> - { >>> - /* Don't walk template arguments; A<info>::type isn't a consteval-only >>> - type. */ >>> - *walk_subtrees = 0; >>> - /* So we have to walk the fields manually. */ >>> - for (tree member = TYPE_FIELDS (t); >>> - member; member = DECL_CHAIN (member)) >>> - if (TREE_CODE (member) == FIELD_DECL) >>> - if (tree r = cp_walk_tree (&TREE_TYPE (member), >>> - consteval_only_type_r, visited, visited)) >>> - return r; >>> - } >>> + /* The stack of class types we're recursively inside. */ >>> + auto_vec<tree> class_stack; >>> + /* The set of class types we've seen. */ >>> + hash_set<tree> class_seen; >>> + /* Whether we've seen mutual class recursion. */ >>> + bool seen_mutual_recursion = false; >> >>> +}; >>> >>> - return NULL_TREE; >>> -} >>> +static tristate consteval_only_p_1 (tree, consteval_only_p_state&); >>> >>> -/* True if T is a consteval-only type as per [basic.types.general]: >>> - "A type is consteval-only if it is either std::meta::info or a type >>> - compounded from a consteval-only type", or something that has >>> - a consteval-only type. */ >>> +/* True if T is a consteval-only type as per [basic.types.general], or >>> + is a declaration with such a type, or a TREE_VEC thereof. */ >>> >>> bool >>> consteval_only_p (tree t) >>> @@ -8612,7 +8588,7 @@ consteval_only_p (tree t) >>> if (!TYPE_P (t)) >>> t = TREE_TYPE (t); >>> >>> - if (!t) >>> + if (!t || t == error_mark_node) >>> return false; >>> >>> if (TREE_CODE (t) == TREE_VEC) >>> @@ -8634,9 +8610,89 @@ consteval_only_p (tree t) >>> which could be consteval-only, depending on T. */ >>> t = complete_type (t); >>> >>> - /* Classes with std::meta::info members are also consteval-only. */ >>> - hash_set<tree> visited; >>> - return !!cp_walk_tree (&t, consteval_only_type_r, &visited, &visited); >>> + consteval_only_p_state state; >>> + return consteval_only_p_1 (t, state).is_true (); >>> +} >>> + >>> +/* A cache of the boolean result of consteval_only_p_1 for class types, when >>> + the result is known. */ >>> + >>> +static GTY((cache)) type_tree_cache_map *consteval_only_class_cache; >>> + >>> +/* Recursive workhorse of consteval_only_p. Returns true if T is definitely >>> + consteval-only, false if it's definitely not, and unknown if we saw an >>> + incomplete type and therefore don't know. */ >>> + >>> +static tristate >>> +consteval_only_p_1 (tree t, consteval_only_p_state& state) >>> +{ >>> + t = TYPE_MAIN_VARIANT (t); >>> + >>> + if (REFLECTION_TYPE_P (t)) >>> + return true; >>> + else if (INDIRECT_TYPE_P (t)) >>> + return consteval_only_p_1 (TREE_TYPE (t), state); >>> + else if (TREE_CODE (t) == ARRAY_TYPE) >>> + return consteval_only_p_1 (TREE_TYPE (t), state); >>> + else if (FUNC_OR_METHOD_TYPE_P (t)) >>> + { >>> + tristate r = consteval_only_p_1 (TREE_TYPE (t), state); >>> + for (tree parm = TYPE_ARG_TYPES (t); >>> + parm != NULL_TREE && parm != void_list_node; >>> + parm = TREE_CHAIN (parm)) >>> + { >>> + r = r || consteval_only_p_1 (TREE_VALUE (parm), state); >>> + if (r.is_true ()) >>> + break; >>> + } >>> + return r; >>> + } >>> + else if (RECORD_OR_UNION_TYPE_P (t)) >>> + { >>> + if (tree *slot = hash_map_safe_get (consteval_only_class_cache, t)) >>> + return *slot == boolean_true_node; >>> + >>> + if (state.class_seen.add (t)) >>> + { >>> + auto begin = state.class_stack.begin (); >>> + auto end = state.class_stack.end (); >>> + auto it = std::find (begin, end, t); >>> + if (it != end && it != &state.class_stack.last ()) >>> + state.seen_mutual_recursion = true; >>> + >>> + return false; >>> + } >>> + state.class_stack.safe_push (t); >>> + >>> + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown (); >>> + for (tree member = TYPE_FIELDS (t); member; member = DECL_CHAIN (member)) >>> + if (TREE_CODE (member) == FIELD_DECL) >>> + { >>> + r = r || consteval_only_p_1 (TREE_TYPE (member), state); >>> + if (r.is_true ()) >>> + break; >>> + } >>> + >>> + if (!COMPLETE_TYPE_P (t)) >>> + /* Until the type is laid out, we can't trust that its TYPE_FIELDS >>> + won't change. */; >>> + else if (r.is_true ()) >>> + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, >>> + t, boolean_true_node); >>> + else if (r.is_false () >>> + && (state.class_stack.length () == 1 >>> + || !state.seen_mutual_recursion)) >> >> D'oh, this !state.seen_mutual_recursion exception for caching nested >> types is wrong. >> >> struct A { >> struct B { struct C *c; } b; >> struct C { B b; } c; >> }; >> >> Here if we start walking from A, which is not mutually recursively, we >> first correctly deem B as unknown-consteval-only, but then go on to >> deem the nested C as not consteval only _and_ cache it (since there's >> no mutual recursion). >> >> So being optimistic about mutually recursive types (the >> 'if (class_seen.add (t)) return false' early exit) is at odds >> with caching the result for nested class types in general, unless >> we significantly complicate things which is probably not worth it. > > Oh, we can just remember if we ever took the early exit and only disable > false result caching for nested classes in that case. That should be > safe, I think (famous last words), and allow us to cache nested classes > in more cases. So for > > struct A { > struct B { int n; } b; > }; > > the false result for B will be cached, but for > > struct A { > struct B { A* a; } b; > struct C { int n; } c; > }; > > neither the false result for B or for C will be cached. > > Here is v4 that introduces an 'optimistic_p' flag to that effect. This > reduces the -freflection slowdown for the TU to 1.1x from 1.2x compared > to v3. > > -- >8 -- > > PR c++/125179 > > gcc/cp/ChangeLog: > > * reflect.cc: (consteval_only_type_r): Remove this cp_walk_tree > callback and replace with ... > (consteval_only_p_state, consteval_only_class_cache) > (consteval_only_p_1): ... this recursive memoized implementation. > (consteval_only_p): Define in terms of consteval_only_p_1. > --- > gcc/cp/reflect.cc | 136 ++++++++++++++++++++++++++++++++-------------- > 1 file changed, 95 insertions(+), 41 deletions(-) > > diff --git a/gcc/cp/reflect.cc b/gcc/cp/reflect.cc > index 3b9f56ea5484..e9bd301fb84b 100644 > --- a/gcc/cp/reflect.cc > +++ b/gcc/cp/reflect.cc > @@ -8561,47 +8561,23 @@ splice (tree refl) > return refl; > } > > -/* A walker for consteval_only_p. It cannot be a lambda, because we > - have to call this recursively, sigh. */ > +/* State maintained during consteval_only_p_1 recursion. */ > > -static tree > -consteval_only_type_r (tree *tp, int *walk_subtrees, void *data) > +struct consteval_only_p_state > { > - tree t = *tp; > - /* Types can contain themselves recursively, hence this. */ > - auto visited = static_cast<hash_set<tree> *>(data); > - > - if (!TYPE_P (t)) > - return NULL_TREE; > - > - if (REFLECTION_TYPE_P (t)) > - return t; > - > - if (typedef_variant_p (t)) > - /* Tell cp_walk_subtrees to look through typedefs. */ > - *walk_subtrees = 2; > - > - if (RECORD_OR_UNION_TYPE_P (t)) > - { > - /* Don't walk template arguments; A<info>::type isn't a consteval-only > - type. */ > - *walk_subtrees = 0; > - /* So we have to walk the fields manually. */ > - for (tree member = TYPE_FIELDS (t); > - member; member = DECL_CHAIN (member)) > - if (TREE_CODE (member) == FIELD_DECL) > - if (tree r = cp_walk_tree (&TREE_TYPE (member), > - consteval_only_type_r, visited, visited)) > - return r; > - } > + /* The stack of class types we're recursively inside. */ > + auto_vec<tree> class_stack; > + /* The set of class types we've seen. */ > + hash_set<tree> class_seen; > + /* True if we've optimistically assumed an already-seen > + consteval-unknown class type is not consteval. */ > + bool optimistic_p = false; > +}; > > - return NULL_TREE; > -} > +static tristate consteval_only_p_1 (tree, consteval_only_p_state&); How about making this a member function of consteval_only_p_state instead of a free function with a reference parameter? > -/* True if T is a consteval-only type as per [basic.types.general]: > - "A type is consteval-only if it is either std::meta::info or a type > - compounded from a consteval-only type", or something that has > - a consteval-only type. */ > +/* True if T is a consteval-only type as per [basic.types.general], or > + is a declaration with such a type, or a TREE_VEC thereof. */ > > bool > consteval_only_p (tree t) > @@ -8612,7 +8588,7 @@ consteval_only_p (tree t) > if (!TYPE_P (t)) > t = TREE_TYPE (t); > > - if (!t) > + if (!t || t == error_mark_node) > return false; > > if (TREE_CODE (t) == TREE_VEC) > @@ -8634,9 +8610,87 @@ consteval_only_p (tree t) > which could be consteval-only, depending on T. */ > t = complete_type (t); > > - /* Classes with std::meta::info members are also consteval-only. */ > - hash_set<tree> visited; > - return !!cp_walk_tree (&t, consteval_only_type_r, &visited, &visited); > + consteval_only_p_state state; > + return consteval_only_p_1 (t, state).is_true (); > +} > + > +/* A cache of the boolean result of consteval_only_p_1 for class types, when > + the result is known. */ > + > +static GTY((cache)) type_tree_cache_map *consteval_only_class_cache; Since you aren't actually returning the tree value, I guess you're using type_tree_cache_map mostly because it's an existing type and there isn't one for type to bool? Also, from gty.texi: > Note that caches should generally use @code{deletable} instead; > @code{cache} is only preferable if the value is impractical to > recompute from the key when needed. This seems like a ((deletable)) case to me, so we could just use hash_map<tree,bool>. > + > +/* Recursive workhorse of consteval_only_p. Returns true if T is definitely > + consteval-only, false if it's definitely not, and unknown if we saw an > + incomplete type and therefore don't know. */ > + > +static tristate > +consteval_only_p_1 (tree t, consteval_only_p_state& state) > +{ > + t = TYPE_MAIN_VARIANT (t); > + > + if (REFLECTION_TYPE_P (t)) > + return true; > + else if (INDIRECT_TYPE_P (t)) > + return consteval_only_p_1 (TREE_TYPE (t), state); > + else if (TREE_CODE (t) == ARRAY_TYPE) > + return consteval_only_p_1 (TREE_TYPE (t), state); > + else if (FUNC_OR_METHOD_TYPE_P (t)) > + { > + tristate r = consteval_only_p_1 (TREE_TYPE (t), state); > + for (tree parm = TYPE_ARG_TYPES (t); > + parm != NULL_TREE && parm != void_list_node; > + parm = TREE_CHAIN (parm)) > + { > + r = r || consteval_only_p_1 (TREE_VALUE (parm), state); > + if (r.is_true ()) Let's move the is_true check before the || so that we don't look at the first parm when the return type is consteval-only. > + break; > + } > + return r; > + } > + else if (RECORD_OR_UNION_TYPE_P (t)) > + { > + if (tree *slot = hash_map_safe_get (consteval_only_class_cache, t)) > + return *slot == boolean_true_node; > + > + if (state.class_seen.add (t)) > + { > + /* Optimistically assume this already seen consteval-unknown class is > + not consteval only, for sake of mutually recursive classes. */ > + state.optimistic_p = true; > + return false; > + } > + state.class_stack.safe_push (t); > + > + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown (); > + for (tree member = TYPE_FIELDS (t); member; member = DECL_CHAIN (member)) > + if (TREE_CODE (member) == FIELD_DECL) > + { > + r = r || consteval_only_p_1 (TREE_TYPE (member), state); > + if (r.is_true ()) > + break; > + } > + > + if (!COMPLETE_TYPE_P (t)) > + /* Until the type is laid out, we can't trust that its TYPE_FIELDS > + won't change. */; > + else if (r.is_true ()) > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > + t, boolean_true_node); > + else if (r.is_false () > + /* The optimistic assumption above is at odds with caching > + 'false' results for a nested class type. */ > + && (state.class_stack.length () == 1 || !state.optimistic_p)) > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > + t, boolean_false_node); > + > + state.class_stack.pop (); > + return r; > + } > + else if (TYPE_PTRMEM_P (t)) > + return (consteval_only_p_1 (TYPE_PTRMEM_CLASS_TYPE (t), state) > + || consteval_only_p_1 (TYPE_PTRMEM_POINTED_TO_TYPE (t), state)); > + else > + return false; > } > > /* A walker for check_out_of_consteval_use_r. It cannot be a lambda, because
On Tue, 5 May 2026, Jason Merrill wrote: > On 5/5/26 11:21 AM, Patrick Palka wrote: > > On Tue, 5 May 2026, Patrick Palka wrote: > > > > > On Tue, 5 May 2026, Patrick Palka wrote: > > > > > > > On Tue, 5 May 2026, Jakub Jelinek wrote: > > > > > > > > > On Mon, May 04, 2026 at 09:25:06PM -0400, Patrick Palka wrote: > > > > > > + else if (RECORD_OR_UNION_TYPE_P (t)) > > > > > > + { > > > > > > + if (tree *slot = hash_map_safe_get > > > > > > (consteval_only_class_cache, t)) > > > > > > + return *slot == boolean_true_node; > > > > > > + > > > > > > + if (class_set.add (t)) > > > > > > + /* Handle struct A { A* p; } etc. */ > > > > > > + return false; > > > > > > + > > > > > > + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown > > > > > > (); > > > > > > + for (tree member = TYPE_FIELDS (t); member; member = > > > > > > DECL_CHAIN (member)) > > > > > > + if (TREE_CODE (member) == FIELD_DECL) > > > > > > + { > > > > > > + r = r || consteval_only_p_1 (TREE_TYPE (member), > > > > > > class_set); > > > > > > + if (r.is_true ()) > > > > > > + break; > > > > > > + } > > > > > > + > > > > > > + if (COMPLETE_TYPE_P (t)) > > > > > > + { > > > > > > + if (r.is_true ()) > > > > > > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > > > > > > + t, boolean_true_node); > > > > > > + else if (r.is_false ()) > > > > > > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > > > > > > + t, boolean_false_node); > > > > > > + } > > > > > > + > > > > > > + class_set.remove (t); > > > > > > > > > > Doesn't the above stmt result in really bad compile time complexity? > > > > > If I have say > > > > > struct S1; > > > > > struct S2; > > > > > struct S3 { S2 *a; }; > > > > > struct S4 { S3 *a; }; > > > > > struct S5 { S4 *a; }; > > > > > ... > > > > > struct S9999 { S9998 *a; }; > > > > > struct S2 { S9999 *a; S1 *b; }; > > > > > then when asking if S2 is consteval-only, this will walk ~10000^2 > > > > > types > > > > > rather than just ~10000 types. Wouldn't it be better to > > > > > class_set.add (t) and never remove, but if we trigger that return > > > > > false; > > > > > in there, remember that we shouldn't cache r.is_false () results > > > > > (perhaps > > > > > with the exception when this was the non-recursive call)? > > > > > > > > Yes, and the current patch is buggy for: > > > > > > > > struct B { A* p; }; > > > > struct A { > > > > B m; > > > > meta::info n; > > > > }; > > > > > > > > We'd incorrectly deem B not consteval-only if we first walk it from A. > > > > > > > > It's apparently important for the testcase from this PR to return false > > > > rather than unknown when detecting mutual recursion, otherwise we still > > > > get a 4x slowdown. > > > > > > The reason it's important to optimistically return false upon hitting > > > mutual recursion is so that we get a chance to cache the result. > > > Otherwise we'll never cache the result for some mutually recursive > > > types; consteval_only_p_1 will always return unknown. > > > > > > > But if we must return false in the case of mutual > > > > recursion then indeed it's not safe to cache r.is_false () results > > > > except for the outermost class because of scenarios like B above. > > > > > > > > To fix this efficiently I think we need to maintian both a class_seen > > > > set and a class_stack. What do you think of the following? > > > > > > > > -- >8 -- > > > > > > > > PR c++/125179 > > > > > > > > gcc/cp/ChangeLog: > > > > > > > > * reflect.cc: Include <algorithm>. > > > > (consteval_only_type_r): Remove this cp_walk_tree callback and > > > > replace with ... > > > > (consteval_only_p_state, consteval_only_class_cache) > > > > (consteval_only_p_1): ... this recursive memoized implementation. > > > > (consteval_only_p): Define in terms of consteval_only_p_1. > > > > --- > > > > gcc/cp/reflect.cc | 138 ++++++++++++++++++++++++++++++++-------------- > > > > 1 file changed, 97 insertions(+), 41 deletions(-) > > > > > > > > diff --git a/gcc/cp/reflect.cc b/gcc/cp/reflect.cc > > > > index 3b9f56ea5484..67a1e1b85895 100644 > > > > --- a/gcc/cp/reflect.cc > > > > +++ b/gcc/cp/reflect.cc > > > > @@ -20,6 +20,7 @@ along with GCC; see the file COPYING3. If not see > > > > <http://www.gnu.org/licenses/>. */ > > > > #include "config.h" > > > > +#define INCLUDE_ALGORITHM > > > > #include "system.h" > > > > #include "coretypes.h" > > > > #include "target.h" > > > > @@ -8561,47 +8562,22 @@ splice (tree refl) > > > > return refl; > > > > } > > > > -/* A walker for consteval_only_p. It cannot be a lambda, because we > > > > - have to call this recursively, sigh. */ > > > > +/* State maintained during consteval_only_p_1 recursion. */ > > > > -static tree > > > > -consteval_only_type_r (tree *tp, int *walk_subtrees, void *data) > > > > +struct consteval_only_p_state > > > > { > > > > - tree t = *tp; > > > > - /* Types can contain themselves recursively, hence this. */ > > > > - auto visited = static_cast<hash_set<tree> *>(data); > > > > - > > > > - if (!TYPE_P (t)) > > > > - return NULL_TREE; > > > > - > > > > - if (REFLECTION_TYPE_P (t)) > > > > - return t; > > > > - > > > > - if (typedef_variant_p (t)) > > > > - /* Tell cp_walk_subtrees to look through typedefs. */ > > > > - *walk_subtrees = 2; > > > > - > > > > - if (RECORD_OR_UNION_TYPE_P (t)) > > > > - { > > > > - /* Don't walk template arguments; A<info>::type isn't a > > > > consteval-only > > > > - type. */ > > > > - *walk_subtrees = 0; > > > > - /* So we have to walk the fields manually. */ > > > > - for (tree member = TYPE_FIELDS (t); > > > > - member; member = DECL_CHAIN (member)) > > > > - if (TREE_CODE (member) == FIELD_DECL) > > > > - if (tree r = cp_walk_tree (&TREE_TYPE (member), > > > > - consteval_only_type_r, visited, visited)) > > > > - return r; > > > > - } > > > > + /* The stack of class types we're recursively inside. */ > > > > + auto_vec<tree> class_stack; > > > > + /* The set of class types we've seen. */ > > > > + hash_set<tree> class_seen; > > > > + /* Whether we've seen mutual class recursion. */ > > > > + bool seen_mutual_recursion = false; > > > > > > > +}; > > > > - return NULL_TREE; > > > > -} > > > > +static tristate consteval_only_p_1 (tree, consteval_only_p_state&); > > > > -/* True if T is a consteval-only type as per [basic.types.general]: > > > > - "A type is consteval-only if it is either std::meta::info or a type > > > > - compounded from a consteval-only type", or something that has > > > > - a consteval-only type. */ > > > > +/* True if T is a consteval-only type as per [basic.types.general], or > > > > + is a declaration with such a type, or a TREE_VEC thereof. */ > > > > bool > > > > consteval_only_p (tree t) > > > > @@ -8612,7 +8588,7 @@ consteval_only_p (tree t) > > > > if (!TYPE_P (t)) > > > > t = TREE_TYPE (t); > > > > - if (!t) > > > > + if (!t || t == error_mark_node) > > > > return false; > > > > if (TREE_CODE (t) == TREE_VEC) > > > > @@ -8634,9 +8610,89 @@ consteval_only_p (tree t) > > > > which could be consteval-only, depending on T. */ > > > > t = complete_type (t); > > > > - /* Classes with std::meta::info members are also consteval-only. > > > > */ > > > > - hash_set<tree> visited; > > > > - return !!cp_walk_tree (&t, consteval_only_type_r, &visited, > > > > &visited); > > > > + consteval_only_p_state state; > > > > + return consteval_only_p_1 (t, state).is_true (); > > > > +} > > > > + > > > > +/* A cache of the boolean result of consteval_only_p_1 for class types, > > > > when > > > > + the result is known. */ > > > > + > > > > +static GTY((cache)) type_tree_cache_map *consteval_only_class_cache; > > > > + > > > > +/* Recursive workhorse of consteval_only_p. Returns true if T is > > > > definitely > > > > + consteval-only, false if it's definitely not, and unknown if we saw > > > > an > > > > + incomplete type and therefore don't know. */ > > > > + > > > > +static tristate > > > > +consteval_only_p_1 (tree t, consteval_only_p_state& state) > > > > +{ > > > > + t = TYPE_MAIN_VARIANT (t); > > > > + > > > > + if (REFLECTION_TYPE_P (t)) > > > > + return true; > > > > + else if (INDIRECT_TYPE_P (t)) > > > > + return consteval_only_p_1 (TREE_TYPE (t), state); > > > > + else if (TREE_CODE (t) == ARRAY_TYPE) > > > > + return consteval_only_p_1 (TREE_TYPE (t), state); > > > > + else if (FUNC_OR_METHOD_TYPE_P (t)) > > > > + { > > > > + tristate r = consteval_only_p_1 (TREE_TYPE (t), state); > > > > + for (tree parm = TYPE_ARG_TYPES (t); > > > > + parm != NULL_TREE && parm != void_list_node; > > > > + parm = TREE_CHAIN (parm)) > > > > + { > > > > + r = r || consteval_only_p_1 (TREE_VALUE (parm), state); > > > > + if (r.is_true ()) > > > > + break; > > > > + } > > > > + return r; > > > > + } > > > > + else if (RECORD_OR_UNION_TYPE_P (t)) > > > > + { > > > > + if (tree *slot = hash_map_safe_get (consteval_only_class_cache, > > > > t)) > > > > + return *slot == boolean_true_node; > > > > + > > > > + if (state.class_seen.add (t)) > > > > + { > > > > + auto begin = state.class_stack.begin (); > > > > + auto end = state.class_stack.end (); > > > > + auto it = std::find (begin, end, t); > > > > + if (it != end && it != &state.class_stack.last ()) > > > > + state.seen_mutual_recursion = true; > > > > + > > > > + return false; > > > > + } > > > > + state.class_stack.safe_push (t); > > > > + > > > > + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown (); > > > > + for (tree member = TYPE_FIELDS (t); member; member = DECL_CHAIN > > > > (member)) > > > > + if (TREE_CODE (member) == FIELD_DECL) > > > > + { > > > > + r = r || consteval_only_p_1 (TREE_TYPE (member), state); > > > > + if (r.is_true ()) > > > > + break; > > > > + } > > > > + > > > > + if (!COMPLETE_TYPE_P (t)) > > > > + /* Until the type is laid out, we can't trust that its TYPE_FIELDS > > > > + won't change. */; > > > > + else if (r.is_true ()) > > > > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > > > > + t, boolean_true_node); > > > > + else if (r.is_false () > > > > + && (state.class_stack.length () == 1 > > > > + || !state.seen_mutual_recursion)) > > > > > > D'oh, this !state.seen_mutual_recursion exception for caching nested > > > types is wrong. > > > > > > struct A { > > > struct B { struct C *c; } b; > > > struct C { B b; } c; > > > }; > > > > > > Here if we start walking from A, which is not mutually recursively, we > > > first correctly deem B as unknown-consteval-only, but then go on to > > > deem the nested C as not consteval only _and_ cache it (since there's > > > no mutual recursion). > > > > > > So being optimistic about mutually recursive types (the > > > 'if (class_seen.add (t)) return false' early exit) is at odds > > > with caching the result for nested class types in general, unless > > > we significantly complicate things which is probably not worth it. > > > > Oh, we can just remember if we ever took the early exit and only disable > > false result caching for nested classes in that case. That should be > > safe, I think (famous last words), and allow us to cache nested classes > > in more cases. So for > > > > struct A { > > struct B { int n; } b; > > }; > > > > the false result for B will be cached, but for > > > > struct A { > > struct B { A* a; } b; > > struct C { int n; } c; > > }; > > > > neither the false result for B or for C will be cached. > > > > Here is v4 that introduces an 'optimistic_p' flag to that effect. This > > reduces the -freflection slowdown for the TU to 1.1x from 1.2x compared > > to v3. > > > > -- >8 -- > > > > PR c++/125179 > > > > gcc/cp/ChangeLog: > > > > * reflect.cc: (consteval_only_type_r): Remove this cp_walk_tree > > callback and replace with ... > > (consteval_only_p_state, consteval_only_class_cache) > > (consteval_only_p_1): ... this recursive memoized implementation. > > (consteval_only_p): Define in terms of consteval_only_p_1. > > --- > > gcc/cp/reflect.cc | 136 ++++++++++++++++++++++++++++++++-------------- > > 1 file changed, 95 insertions(+), 41 deletions(-) > > > > diff --git a/gcc/cp/reflect.cc b/gcc/cp/reflect.cc > > index 3b9f56ea5484..e9bd301fb84b 100644 > > --- a/gcc/cp/reflect.cc > > +++ b/gcc/cp/reflect.cc > > @@ -8561,47 +8561,23 @@ splice (tree refl) > > return refl; > > } > > -/* A walker for consteval_only_p. It cannot be a lambda, because we > > - have to call this recursively, sigh. */ > > +/* State maintained during consteval_only_p_1 recursion. */ > > -static tree > > -consteval_only_type_r (tree *tp, int *walk_subtrees, void *data) > > +struct consteval_only_p_state > > { > > - tree t = *tp; > > - /* Types can contain themselves recursively, hence this. */ > > - auto visited = static_cast<hash_set<tree> *>(data); > > - > > - if (!TYPE_P (t)) > > - return NULL_TREE; > > - > > - if (REFLECTION_TYPE_P (t)) > > - return t; > > - > > - if (typedef_variant_p (t)) > > - /* Tell cp_walk_subtrees to look through typedefs. */ > > - *walk_subtrees = 2; > > - > > - if (RECORD_OR_UNION_TYPE_P (t)) > > - { > > - /* Don't walk template arguments; A<info>::type isn't a > > consteval-only > > - type. */ > > - *walk_subtrees = 0; > > - /* So we have to walk the fields manually. */ > > - for (tree member = TYPE_FIELDS (t); > > - member; member = DECL_CHAIN (member)) > > - if (TREE_CODE (member) == FIELD_DECL) > > - if (tree r = cp_walk_tree (&TREE_TYPE (member), > > - consteval_only_type_r, visited, visited)) > > - return r; > > - } > > + /* The stack of class types we're recursively inside. */ > > + auto_vec<tree> class_stack; > > + /* The set of class types we've seen. */ > > + hash_set<tree> class_seen; > > + /* True if we've optimistically assumed an already-seen > > + consteval-unknown class type is not consteval. */ > > + bool optimistic_p = false; > > +}; > > - return NULL_TREE; > > -} > > +static tristate consteval_only_p_1 (tree, consteval_only_p_state&); > > How about making this a member function of consteval_only_p_state instead of a > free function with a reference parameter? Good idea, done. > > > -/* True if T is a consteval-only type as per [basic.types.general]: > > - "A type is consteval-only if it is either std::meta::info or a type > > - compounded from a consteval-only type", or something that has > > - a consteval-only type. */ > > +/* True if T is a consteval-only type as per [basic.types.general], or > > + is a declaration with such a type, or a TREE_VEC thereof. */ > > bool > > consteval_only_p (tree t) > > @@ -8612,7 +8588,7 @@ consteval_only_p (tree t) > > if (!TYPE_P (t)) > > t = TREE_TYPE (t); > > - if (!t) > > + if (!t || t == error_mark_node) > > return false; > > if (TREE_CODE (t) == TREE_VEC) > > @@ -8634,9 +8610,87 @@ consteval_only_p (tree t) > > which could be consteval-only, depending on T. */ > > t = complete_type (t); > > - /* Classes with std::meta::info members are also consteval-only. */ > > - hash_set<tree> visited; > > - return !!cp_walk_tree (&t, consteval_only_type_r, &visited, &visited); > > + consteval_only_p_state state; > > + return consteval_only_p_1 (t, state).is_true (); > > +} > > + > > +/* A cache of the boolean result of consteval_only_p_1 for class types, > > when > > + the result is known. */ > > + > > +static GTY((cache)) type_tree_cache_map *consteval_only_class_cache; > > Since you aren't actually returning the tree value, I guess you're using > type_tree_cache_map mostly because it's an existing type and there isn't one > for type to bool? Yeah, though in an earlier version of the patch the mapped values were tri-state so using bool wasn't an option. I'm not keen on creating a new map type just for this case, at least as part of this patch, the memory savings should be negligible due to padding and type_tree_cache_map is heavily used and well tested. > > Also, from gty.texi: > > > Note that caches should generally use @code{deletable} instead; > > @code{cache} is only preferable if the value is impractical to > > recompute from the key when needed. > > This seems like a ((deletable)) case to me, so we could just use > hash_map<tree,bool>. Using ((deletable)) results in a 10x -freflection slowdown for the testcase from the PR, vs 1.1x with ((cache)), I suspect because it gets cleared every time we complete a class. > > > + > > +/* Recursive workhorse of consteval_only_p. Returns true if T is > > definitely > > + consteval-only, false if it's definitely not, and unknown if we saw an > > + incomplete type and therefore don't know. */ > > + > > +static tristate > > +consteval_only_p_1 (tree t, consteval_only_p_state& state) > > +{ > > + t = TYPE_MAIN_VARIANT (t); > > + > > + if (REFLECTION_TYPE_P (t)) > > + return true; > > + else if (INDIRECT_TYPE_P (t)) > > + return consteval_only_p_1 (TREE_TYPE (t), state); > > + else if (TREE_CODE (t) == ARRAY_TYPE) > > + return consteval_only_p_1 (TREE_TYPE (t), state); > > + else if (FUNC_OR_METHOD_TYPE_P (t)) > > + { > > + tristate r = consteval_only_p_1 (TREE_TYPE (t), state); > > + for (tree parm = TYPE_ARG_TYPES (t); > > + parm != NULL_TREE && parm != void_list_node; > > + parm = TREE_CHAIN (parm)) > > + { > > + r = r || consteval_only_p_1 (TREE_VALUE (parm), state); > > + if (r.is_true ()) > > Let's move the is_true check before the || so that we don't look at the first > parm when the return type is consteval-only. Fixed. Here's v5 which uses a member function and fixes the above. The cache is still marked as ((cache)) instead of ((deletable)). -- >8 -- PR c++/125179 gcc/cp/ChangeLog: * reflect.cc: (consteval_only_type_r): Remove this cp_walk_tree callback and replace with ... (consteval_only_walker): ... this recursive memoized implementation. (consteval_only_p): Define in terms of consteval_only_walker. --- gcc/cp/reflect.cc | 134 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 93 insertions(+), 41 deletions(-) diff --git a/gcc/cp/reflect.cc b/gcc/cp/reflect.cc index 3b9f56ea5484..6e7000f7de81 100644 --- a/gcc/cp/reflect.cc +++ b/gcc/cp/reflect.cc @@ -8561,47 +8561,26 @@ splice (tree refl) return refl; } -/* A walker for consteval_only_p. It cannot be a lambda, because we - have to call this recursively, sigh. */ +/* A cache of the known boolean result of consteval_only_p_walker::walk for + class types. */ -static tree -consteval_only_type_r (tree *tp, int *walk_subtrees, void *data) -{ - tree t = *tp; - /* Types can contain themselves recursively, hence this. */ - auto visited = static_cast<hash_set<tree> *>(data); - - if (!TYPE_P (t)) - return NULL_TREE; +static GTY((cache)) type_tree_cache_map *consteval_only_class_cache; - if (REFLECTION_TYPE_P (t)) - return t; - - if (typedef_variant_p (t)) - /* Tell cp_walk_subtrees to look through typedefs. */ - *walk_subtrees = 2; - - if (RECORD_OR_UNION_TYPE_P (t)) - { - /* Don't walk template arguments; A<info>::type isn't a consteval-only - type. */ - *walk_subtrees = 0; - /* So we have to walk the fields manually. */ - for (tree member = TYPE_FIELDS (t); - member; member = DECL_CHAIN (member)) - if (TREE_CODE (member) == FIELD_DECL) - if (tree r = cp_walk_tree (&TREE_TYPE (member), - consteval_only_type_r, visited, visited)) - return r; - } +struct consteval_only_p_walker +{ + /* The stack of class types we're recursively inside. */ + auto_vec<tree> class_stack; + /* The set of class types we've seen. */ + hash_set<tree> class_seen; + /* True if we've optimistically assumed an already-seen + consteval-unknown class type is not consteval. */ + bool optimistic_p = false; - return NULL_TREE; -} + tristate walk (tree); +}; -/* True if T is a consteval-only type as per [basic.types.general]: - "A type is consteval-only if it is either std::meta::info or a type - compounded from a consteval-only type", or something that has - a consteval-only type. */ +/* True if T is a consteval-only type as per [basic.types.general], or + is a declaration with such a type, or a TREE_VEC thereof. */ bool consteval_only_p (tree t) @@ -8612,7 +8591,7 @@ consteval_only_p (tree t) if (!TYPE_P (t)) t = TREE_TYPE (t); - if (!t) + if (!t || t == error_mark_node) return false; if (TREE_CODE (t) == TREE_VEC) @@ -8634,9 +8613,82 @@ consteval_only_p (tree t) which could be consteval-only, depending on T. */ t = complete_type (t); - /* Classes with std::meta::info members are also consteval-only. */ - hash_set<tree> visited; - return !!cp_walk_tree (&t, consteval_only_type_r, &visited, &visited); + consteval_only_p_walker walker; + return walker.walk (t).is_true (); +} + +/* Recursive workhorse of consteval_only_p. Returns true if T is definitely + consteval-only, false if it's definitely not, and unknown if we saw an + incomplete type and therefore don't know. */ + +tristate +consteval_only_p_walker::walk (tree t) +{ + t = TYPE_MAIN_VARIANT (t); + + if (REFLECTION_TYPE_P (t)) + return true; + else if (INDIRECT_TYPE_P (t)) + return walk (TREE_TYPE (t)); + else if (TREE_CODE (t) == ARRAY_TYPE) + return walk (TREE_TYPE (t)); + else if (FUNC_OR_METHOD_TYPE_P (t)) + { + tristate r = walk (TREE_TYPE (t)); + for (tree parm = TYPE_ARG_TYPES (t); + parm != NULL_TREE && parm != void_list_node; + parm = TREE_CHAIN (parm)) + { + if (r.is_true ()) + break; + r = r || walk (TREE_VALUE (parm)); + } + return r; + } + else if (RECORD_OR_UNION_TYPE_P (t)) + { + if (tree *slot = hash_map_safe_get (consteval_only_class_cache, t)) + return *slot == boolean_true_node; + + if (class_seen.add (t)) + { + /* Optimistically assume this already seen consteval-unknown class is + not consteval only, for sake of mutually recursive classes. */ + optimistic_p = true; + return false; + } + class_stack.safe_push (t); + + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown (); + for (tree member = TYPE_FIELDS (t); member; member = DECL_CHAIN (member)) + if (TREE_CODE (member) == FIELD_DECL) + { + r = r || walk (TREE_TYPE (member)); + if (r.is_true ()) + break; + } + + if (!COMPLETE_TYPE_P (t)) + /* Until the type is laid out, we can't trust that its TYPE_FIELDS + won't change. */; + else if (r.is_true ()) + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, + t, boolean_true_node); + else if (r.is_false () + /* The optimistic assumption above is at odds with caching + 'false' results for a nested class type. */ + && (class_stack.length () == 1 || !optimistic_p)) + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, + t, boolean_false_node); + + class_stack.pop (); + return r; + } + else if (TYPE_PTRMEM_P (t)) + return (walk (TYPE_PTRMEM_CLASS_TYPE (t)) + || walk (TYPE_PTRMEM_POINTED_TO_TYPE (t))); + else + return false; } /* A walker for check_out_of_consteval_use_r. It cannot be a lambda, because
On 5/5/26 1:20 PM, Patrick Palka wrote: > On Tue, 5 May 2026, Jason Merrill wrote: > >> On 5/5/26 11:21 AM, Patrick Palka wrote: >>> On Tue, 5 May 2026, Patrick Palka wrote: >>> >>>> On Tue, 5 May 2026, Patrick Palka wrote: >>>> >>>>> On Tue, 5 May 2026, Jakub Jelinek wrote: >>>>> >>>>>> On Mon, May 04, 2026 at 09:25:06PM -0400, Patrick Palka wrote: >>>>>>> + else if (RECORD_OR_UNION_TYPE_P (t)) >>>>>>> + { >>>>>>> + if (tree *slot = hash_map_safe_get >>>>>>> (consteval_only_class_cache, t)) >>>>>>> + return *slot == boolean_true_node; >>>>>>> + >>>>>>> + if (class_set.add (t)) >>>>>>> + /* Handle struct A { A* p; } etc. */ >>>>>>> + return false; >>>>>>> + >>>>>>> + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown >>>>>>> (); >>>>>>> + for (tree member = TYPE_FIELDS (t); member; member = >>>>>>> DECL_CHAIN (member)) >>>>>>> + if (TREE_CODE (member) == FIELD_DECL) >>>>>>> + { >>>>>>> + r = r || consteval_only_p_1 (TREE_TYPE (member), >>>>>>> class_set); >>>>>>> + if (r.is_true ()) >>>>>>> + break; >>>>>>> + } >>>>>>> + >>>>>>> + if (COMPLETE_TYPE_P (t)) >>>>>>> + { >>>>>>> + if (r.is_true ()) >>>>>>> + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, >>>>>>> + t, boolean_true_node); >>>>>>> + else if (r.is_false ()) >>>>>>> + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, >>>>>>> + t, boolean_false_node); >>>>>>> + } >>>>>>> + >>>>>>> + class_set.remove (t); >>>>>> >>>>>> Doesn't the above stmt result in really bad compile time complexity? >>>>>> If I have say >>>>>> struct S1; >>>>>> struct S2; >>>>>> struct S3 { S2 *a; }; >>>>>> struct S4 { S3 *a; }; >>>>>> struct S5 { S4 *a; }; >>>>>> ... >>>>>> struct S9999 { S9998 *a; }; >>>>>> struct S2 { S9999 *a; S1 *b; }; >>>>>> then when asking if S2 is consteval-only, this will walk ~10000^2 >>>>>> types >>>>>> rather than just ~10000 types. Wouldn't it be better to >>>>>> class_set.add (t) and never remove, but if we trigger that return >>>>>> false; >>>>>> in there, remember that we shouldn't cache r.is_false () results >>>>>> (perhaps >>>>>> with the exception when this was the non-recursive call)? >>>>> >>>>> Yes, and the current patch is buggy for: >>>>> >>>>> struct B { A* p; }; >>>>> struct A { >>>>> B m; >>>>> meta::info n; >>>>> }; >>>>> >>>>> We'd incorrectly deem B not consteval-only if we first walk it from A. >>>>> >>>>> It's apparently important for the testcase from this PR to return false >>>>> rather than unknown when detecting mutual recursion, otherwise we still >>>>> get a 4x slowdown. >>>> >>>> The reason it's important to optimistically return false upon hitting >>>> mutual recursion is so that we get a chance to cache the result. >>>> Otherwise we'll never cache the result for some mutually recursive >>>> types; consteval_only_p_1 will always return unknown. >>>> >>>>> But if we must return false in the case of mutual >>>>> recursion then indeed it's not safe to cache r.is_false () results >>>>> except for the outermost class because of scenarios like B above. >>>>> >>>>> To fix this efficiently I think we need to maintian both a class_seen >>>>> set and a class_stack. What do you think of the following? >>>>> >>>>> -- >8 -- >>>>> >>>>> PR c++/125179 >>>>> >>>>> gcc/cp/ChangeLog: >>>>> >>>>> * reflect.cc: Include <algorithm>. >>>>> (consteval_only_type_r): Remove this cp_walk_tree callback and >>>>> replace with ... >>>>> (consteval_only_p_state, consteval_only_class_cache) >>>>> (consteval_only_p_1): ... this recursive memoized implementation. >>>>> (consteval_only_p): Define in terms of consteval_only_p_1. >>>>> --- >>>>> gcc/cp/reflect.cc | 138 ++++++++++++++++++++++++++++++++-------------- >>>>> 1 file changed, 97 insertions(+), 41 deletions(-) >>>>> >>>>> diff --git a/gcc/cp/reflect.cc b/gcc/cp/reflect.cc >>>>> index 3b9f56ea5484..67a1e1b85895 100644 >>>>> --- a/gcc/cp/reflect.cc >>>>> +++ b/gcc/cp/reflect.cc >>>>> @@ -20,6 +20,7 @@ along with GCC; see the file COPYING3. If not see >>>>> <http://www.gnu.org/licenses/>. */ >>>>> #include "config.h" >>>>> +#define INCLUDE_ALGORITHM >>>>> #include "system.h" >>>>> #include "coretypes.h" >>>>> #include "target.h" >>>>> @@ -8561,47 +8562,22 @@ splice (tree refl) >>>>> return refl; >>>>> } >>>>> -/* A walker for consteval_only_p. It cannot be a lambda, because we >>>>> - have to call this recursively, sigh. */ >>>>> +/* State maintained during consteval_only_p_1 recursion. */ >>>>> -static tree >>>>> -consteval_only_type_r (tree *tp, int *walk_subtrees, void *data) >>>>> +struct consteval_only_p_state >>>>> { >>>>> - tree t = *tp; >>>>> - /* Types can contain themselves recursively, hence this. */ >>>>> - auto visited = static_cast<hash_set<tree> *>(data); >>>>> - >>>>> - if (!TYPE_P (t)) >>>>> - return NULL_TREE; >>>>> - >>>>> - if (REFLECTION_TYPE_P (t)) >>>>> - return t; >>>>> - >>>>> - if (typedef_variant_p (t)) >>>>> - /* Tell cp_walk_subtrees to look through typedefs. */ >>>>> - *walk_subtrees = 2; >>>>> - >>>>> - if (RECORD_OR_UNION_TYPE_P (t)) >>>>> - { >>>>> - /* Don't walk template arguments; A<info>::type isn't a >>>>> consteval-only >>>>> - type. */ >>>>> - *walk_subtrees = 0; >>>>> - /* So we have to walk the fields manually. */ >>>>> - for (tree member = TYPE_FIELDS (t); >>>>> - member; member = DECL_CHAIN (member)) >>>>> - if (TREE_CODE (member) == FIELD_DECL) >>>>> - if (tree r = cp_walk_tree (&TREE_TYPE (member), >>>>> - consteval_only_type_r, visited, visited)) >>>>> - return r; >>>>> - } >>>>> + /* The stack of class types we're recursively inside. */ >>>>> + auto_vec<tree> class_stack; >>>>> + /* The set of class types we've seen. */ >>>>> + hash_set<tree> class_seen; >>>>> + /* Whether we've seen mutual class recursion. */ >>>>> + bool seen_mutual_recursion = false; >>>> >>>>> +}; >>>>> - return NULL_TREE; >>>>> -} >>>>> +static tristate consteval_only_p_1 (tree, consteval_only_p_state&); >>>>> -/* True if T is a consteval-only type as per [basic.types.general]: >>>>> - "A type is consteval-only if it is either std::meta::info or a type >>>>> - compounded from a consteval-only type", or something that has >>>>> - a consteval-only type. */ >>>>> +/* True if T is a consteval-only type as per [basic.types.general], or >>>>> + is a declaration with such a type, or a TREE_VEC thereof. */ >>>>> bool >>>>> consteval_only_p (tree t) >>>>> @@ -8612,7 +8588,7 @@ consteval_only_p (tree t) >>>>> if (!TYPE_P (t)) >>>>> t = TREE_TYPE (t); >>>>> - if (!t) >>>>> + if (!t || t == error_mark_node) >>>>> return false; >>>>> if (TREE_CODE (t) == TREE_VEC) >>>>> @@ -8634,9 +8610,89 @@ consteval_only_p (tree t) >>>>> which could be consteval-only, depending on T. */ >>>>> t = complete_type (t); >>>>> - /* Classes with std::meta::info members are also consteval-only. >>>>> */ >>>>> - hash_set<tree> visited; >>>>> - return !!cp_walk_tree (&t, consteval_only_type_r, &visited, >>>>> &visited); >>>>> + consteval_only_p_state state; >>>>> + return consteval_only_p_1 (t, state).is_true (); >>>>> +} >>>>> + >>>>> +/* A cache of the boolean result of consteval_only_p_1 for class types, >>>>> when >>>>> + the result is known. */ >>>>> + >>>>> +static GTY((cache)) type_tree_cache_map *consteval_only_class_cache; >>>>> + >>>>> +/* Recursive workhorse of consteval_only_p. Returns true if T is >>>>> definitely >>>>> + consteval-only, false if it's definitely not, and unknown if we saw >>>>> an >>>>> + incomplete type and therefore don't know. */ >>>>> + >>>>> +static tristate >>>>> +consteval_only_p_1 (tree t, consteval_only_p_state& state) >>>>> +{ >>>>> + t = TYPE_MAIN_VARIANT (t); >>>>> + >>>>> + if (REFLECTION_TYPE_P (t)) >>>>> + return true; >>>>> + else if (INDIRECT_TYPE_P (t)) >>>>> + return consteval_only_p_1 (TREE_TYPE (t), state); >>>>> + else if (TREE_CODE (t) == ARRAY_TYPE) >>>>> + return consteval_only_p_1 (TREE_TYPE (t), state); >>>>> + else if (FUNC_OR_METHOD_TYPE_P (t)) >>>>> + { >>>>> + tristate r = consteval_only_p_1 (TREE_TYPE (t), state); >>>>> + for (tree parm = TYPE_ARG_TYPES (t); >>>>> + parm != NULL_TREE && parm != void_list_node; >>>>> + parm = TREE_CHAIN (parm)) >>>>> + { >>>>> + r = r || consteval_only_p_1 (TREE_VALUE (parm), state); >>>>> + if (r.is_true ()) >>>>> + break; >>>>> + } >>>>> + return r; >>>>> + } >>>>> + else if (RECORD_OR_UNION_TYPE_P (t)) >>>>> + { >>>>> + if (tree *slot = hash_map_safe_get (consteval_only_class_cache, >>>>> t)) >>>>> + return *slot == boolean_true_node; >>>>> + >>>>> + if (state.class_seen.add (t)) >>>>> + { >>>>> + auto begin = state.class_stack.begin (); >>>>> + auto end = state.class_stack.end (); >>>>> + auto it = std::find (begin, end, t); >>>>> + if (it != end && it != &state.class_stack.last ()) >>>>> + state.seen_mutual_recursion = true; >>>>> + >>>>> + return false; >>>>> + } >>>>> + state.class_stack.safe_push (t); >>>>> + >>>>> + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown (); >>>>> + for (tree member = TYPE_FIELDS (t); member; member = DECL_CHAIN >>>>> (member)) >>>>> + if (TREE_CODE (member) == FIELD_DECL) >>>>> + { >>>>> + r = r || consteval_only_p_1 (TREE_TYPE (member), state); >>>>> + if (r.is_true ()) >>>>> + break; >>>>> + } >>>>> + >>>>> + if (!COMPLETE_TYPE_P (t)) >>>>> + /* Until the type is laid out, we can't trust that its TYPE_FIELDS >>>>> + won't change. */; >>>>> + else if (r.is_true ()) >>>>> + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, >>>>> + t, boolean_true_node); >>>>> + else if (r.is_false () >>>>> + && (state.class_stack.length () == 1 >>>>> + || !state.seen_mutual_recursion)) >>>> >>>> D'oh, this !state.seen_mutual_recursion exception for caching nested >>>> types is wrong. >>>> >>>> struct A { >>>> struct B { struct C *c; } b; >>>> struct C { B b; } c; >>>> }; >>>> >>>> Here if we start walking from A, which is not mutually recursively, we >>>> first correctly deem B as unknown-consteval-only, but then go on to >>>> deem the nested C as not consteval only _and_ cache it (since there's >>>> no mutual recursion). >>>> >>>> So being optimistic about mutually recursive types (the >>>> 'if (class_seen.add (t)) return false' early exit) is at odds >>>> with caching the result for nested class types in general, unless >>>> we significantly complicate things which is probably not worth it. >>> >>> Oh, we can just remember if we ever took the early exit and only disable >>> false result caching for nested classes in that case. That should be >>> safe, I think (famous last words), and allow us to cache nested classes >>> in more cases. So for >>> >>> struct A { >>> struct B { int n; } b; >>> }; >>> >>> the false result for B will be cached, but for >>> >>> struct A { >>> struct B { A* a; } b; >>> struct C { int n; } c; >>> }; >>> >>> neither the false result for B or for C will be cached. >>> >>> Here is v4 that introduces an 'optimistic_p' flag to that effect. This >>> reduces the -freflection slowdown for the TU to 1.1x from 1.2x compared >>> to v3. >>> >>> -- >8 -- >>> >>> PR c++/125179 >>> >>> gcc/cp/ChangeLog: >>> >>> * reflect.cc: (consteval_only_type_r): Remove this cp_walk_tree >>> callback and replace with ... >>> (consteval_only_p_state, consteval_only_class_cache) >>> (consteval_only_p_1): ... this recursive memoized implementation. >>> (consteval_only_p): Define in terms of consteval_only_p_1. >>> --- >>> gcc/cp/reflect.cc | 136 ++++++++++++++++++++++++++++++++-------------- >>> 1 file changed, 95 insertions(+), 41 deletions(-) >>> >>> diff --git a/gcc/cp/reflect.cc b/gcc/cp/reflect.cc >>> index 3b9f56ea5484..e9bd301fb84b 100644 >>> --- a/gcc/cp/reflect.cc >>> +++ b/gcc/cp/reflect.cc >>> @@ -8561,47 +8561,23 @@ splice (tree refl) >>> return refl; >>> } >>> -/* A walker for consteval_only_p. It cannot be a lambda, because we >>> - have to call this recursively, sigh. */ >>> +/* State maintained during consteval_only_p_1 recursion. */ >>> -static tree >>> -consteval_only_type_r (tree *tp, int *walk_subtrees, void *data) >>> +struct consteval_only_p_state >>> { >>> - tree t = *tp; >>> - /* Types can contain themselves recursively, hence this. */ >>> - auto visited = static_cast<hash_set<tree> *>(data); >>> - >>> - if (!TYPE_P (t)) >>> - return NULL_TREE; >>> - >>> - if (REFLECTION_TYPE_P (t)) >>> - return t; >>> - >>> - if (typedef_variant_p (t)) >>> - /* Tell cp_walk_subtrees to look through typedefs. */ >>> - *walk_subtrees = 2; >>> - >>> - if (RECORD_OR_UNION_TYPE_P (t)) >>> - { >>> - /* Don't walk template arguments; A<info>::type isn't a >>> consteval-only >>> - type. */ >>> - *walk_subtrees = 0; >>> - /* So we have to walk the fields manually. */ >>> - for (tree member = TYPE_FIELDS (t); >>> - member; member = DECL_CHAIN (member)) >>> - if (TREE_CODE (member) == FIELD_DECL) >>> - if (tree r = cp_walk_tree (&TREE_TYPE (member), >>> - consteval_only_type_r, visited, visited)) >>> - return r; >>> - } >>> + /* The stack of class types we're recursively inside. */ >>> + auto_vec<tree> class_stack; >>> + /* The set of class types we've seen. */ >>> + hash_set<tree> class_seen; >>> + /* True if we've optimistically assumed an already-seen >>> + consteval-unknown class type is not consteval. */ >>> + bool optimistic_p = false; >>> +}; >>> - return NULL_TREE; >>> -} >>> +static tristate consteval_only_p_1 (tree, consteval_only_p_state&); >> >> How about making this a member function of consteval_only_p_state instead of a >> free function with a reference parameter? > > Good idea, done. > >> >>> -/* True if T is a consteval-only type as per [basic.types.general]: >>> - "A type is consteval-only if it is either std::meta::info or a type >>> - compounded from a consteval-only type", or something that has >>> - a consteval-only type. */ >>> +/* True if T is a consteval-only type as per [basic.types.general], or >>> + is a declaration with such a type, or a TREE_VEC thereof. */ >>> bool >>> consteval_only_p (tree t) >>> @@ -8612,7 +8588,7 @@ consteval_only_p (tree t) >>> if (!TYPE_P (t)) >>> t = TREE_TYPE (t); >>> - if (!t) >>> + if (!t || t == error_mark_node) >>> return false; >>> if (TREE_CODE (t) == TREE_VEC) >>> @@ -8634,9 +8610,87 @@ consteval_only_p (tree t) >>> which could be consteval-only, depending on T. */ >>> t = complete_type (t); >>> - /* Classes with std::meta::info members are also consteval-only. */ >>> - hash_set<tree> visited; >>> - return !!cp_walk_tree (&t, consteval_only_type_r, &visited, &visited); >>> + consteval_only_p_state state; >>> + return consteval_only_p_1 (t, state).is_true (); >>> +} >>> + >>> +/* A cache of the boolean result of consteval_only_p_1 for class types, >>> when >>> + the result is known. */ >>> + >>> +static GTY((cache)) type_tree_cache_map *consteval_only_class_cache; >> >> Since you aren't actually returning the tree value, I guess you're using >> type_tree_cache_map mostly because it's an existing type and there isn't one >> for type to bool? > > Yeah, though in an earlier version of the patch the mapped values were > tri-state so using bool wasn't an option. I'm not keen on creating > a new map type just for this case, at least as part of this patch, the > memory savings should be negligible due to padding and type_tree_cache_map > is heavily used and well tested. Agreed. >> Also, from gty.texi: >> >>> Note that caches should generally use @code{deletable} instead; >>> @code{cache} is only preferable if the value is impractical to >>> recompute from the key when needed. >> >> This seems like a ((deletable)) case to me, so we could just use >> hash_map<tree,bool>. > > Using ((deletable)) results in a 10x -freflection slowdown for the > testcase from the PR, vs 1.1x with ((cache)), I suspect because it gets > cleared every time we complete a class. Hunh, I'm surprised GC would happen often enough for that. Never mind, then, thanks for checking. >>> +/* Recursive workhorse of consteval_only_p. Returns true if T is >>> definitely >>> + consteval-only, false if it's definitely not, and unknown if we saw an >>> + incomplete type and therefore don't know. */ >>> + >>> +static tristate >>> +consteval_only_p_1 (tree t, consteval_only_p_state& state) >>> +{ >>> + t = TYPE_MAIN_VARIANT (t); >>> + >>> + if (REFLECTION_TYPE_P (t)) >>> + return true; >>> + else if (INDIRECT_TYPE_P (t)) >>> + return consteval_only_p_1 (TREE_TYPE (t), state); >>> + else if (TREE_CODE (t) == ARRAY_TYPE) >>> + return consteval_only_p_1 (TREE_TYPE (t), state); >>> + else if (FUNC_OR_METHOD_TYPE_P (t)) >>> + { >>> + tristate r = consteval_only_p_1 (TREE_TYPE (t), state); >>> + for (tree parm = TYPE_ARG_TYPES (t); >>> + parm != NULL_TREE && parm != void_list_node; >>> + parm = TREE_CHAIN (parm)) >>> + { >>> + r = r || consteval_only_p_1 (TREE_VALUE (parm), state); >>> + if (r.is_true ()) >> >> Let's move the is_true check before the || so that we don't look at the first >> parm when the return type is consteval-only. > > Fixed. > > Here's v5 which uses a member function and fixes the above. The cache > is still marked as ((cache)) instead of ((deletable)). > > -- >8 -- > > PR c++/125179 > > gcc/cp/ChangeLog: > > * reflect.cc: (consteval_only_type_r): Remove this cp_walk_tree > callback and replace with ... > (consteval_only_walker): ... this recursive memoized > implementation. > (consteval_only_p): Define in terms of consteval_only_walker. > --- > gcc/cp/reflect.cc | 134 ++++++++++++++++++++++++++++++++-------------- > 1 file changed, 93 insertions(+), 41 deletions(-) > > diff --git a/gcc/cp/reflect.cc b/gcc/cp/reflect.cc > index 3b9f56ea5484..6e7000f7de81 100644 > --- a/gcc/cp/reflect.cc > +++ b/gcc/cp/reflect.cc > @@ -8561,47 +8561,26 @@ splice (tree refl) > return refl; > } > > -/* A walker for consteval_only_p. It cannot be a lambda, because we > - have to call this recursively, sigh. */ > +/* A cache of the known boolean result of consteval_only_p_walker::walk for > + class types. */ > > -static tree > -consteval_only_type_r (tree *tp, int *walk_subtrees, void *data) > -{ > - tree t = *tp; > - /* Types can contain themselves recursively, hence this. */ > - auto visited = static_cast<hash_set<tree> *>(data); > - > - if (!TYPE_P (t)) > - return NULL_TREE; > +static GTY((cache)) type_tree_cache_map *consteval_only_class_cache; > > - if (REFLECTION_TYPE_P (t)) > - return t; > - > - if (typedef_variant_p (t)) > - /* Tell cp_walk_subtrees to look through typedefs. */ > - *walk_subtrees = 2; > - > - if (RECORD_OR_UNION_TYPE_P (t)) > - { > - /* Don't walk template arguments; A<info>::type isn't a consteval-only > - type. */ > - *walk_subtrees = 0; > - /* So we have to walk the fields manually. */ > - for (tree member = TYPE_FIELDS (t); > - member; member = DECL_CHAIN (member)) > - if (TREE_CODE (member) == FIELD_DECL) > - if (tree r = cp_walk_tree (&TREE_TYPE (member), > - consteval_only_type_r, visited, visited)) > - return r; > - } > +struct consteval_only_p_walker > +{ > + /* The stack of class types we're recursively inside. */ > + auto_vec<tree> class_stack; > + /* The set of class types we've seen. */ > + hash_set<tree> class_seen; > + /* True if we've optimistically assumed an already-seen > + consteval-unknown class type is not consteval. */ > + bool optimistic_p = false; > > - return NULL_TREE; > -} > + tristate walk (tree); > +}; > > -/* True if T is a consteval-only type as per [basic.types.general]: > - "A type is consteval-only if it is either std::meta::info or a type > - compounded from a consteval-only type", or something that has > - a consteval-only type. */ > +/* True if T is a consteval-only type as per [basic.types.general], or > + is a declaration with such a type, or a TREE_VEC thereof. */ > > bool > consteval_only_p (tree t) > @@ -8612,7 +8591,7 @@ consteval_only_p (tree t) > if (!TYPE_P (t)) > t = TREE_TYPE (t); > > - if (!t) > + if (!t || t == error_mark_node) > return false; > > if (TREE_CODE (t) == TREE_VEC) > @@ -8634,9 +8613,82 @@ consteval_only_p (tree t) > which could be consteval-only, depending on T. */ > t = complete_type (t); > > - /* Classes with std::meta::info members are also consteval-only. */ > - hash_set<tree> visited; > - return !!cp_walk_tree (&t, consteval_only_type_r, &visited, &visited); > + consteval_only_p_walker walker; > + return walker.walk (t).is_true (); > +} > + > +/* Recursive workhorse of consteval_only_p. Returns true if T is definitely > + consteval-only, false if it's definitely not, and unknown if we saw an > + incomplete type and therefore don't know. */ > + > +tristate > +consteval_only_p_walker::walk (tree t) > +{ > + t = TYPE_MAIN_VARIANT (t); > + > + if (REFLECTION_TYPE_P (t)) > + return true; > + else if (INDIRECT_TYPE_P (t)) > + return walk (TREE_TYPE (t)); > + else if (TREE_CODE (t) == ARRAY_TYPE) > + return walk (TREE_TYPE (t)); > + else if (FUNC_OR_METHOD_TYPE_P (t)) > + { > + tristate r = walk (TREE_TYPE (t)); > + for (tree parm = TYPE_ARG_TYPES (t); > + parm != NULL_TREE && parm != void_list_node; > + parm = TREE_CHAIN (parm)) > + { > + if (r.is_true ()) > + break; > + r = r || walk (TREE_VALUE (parm)); > + } > + return r; > + } > + else if (RECORD_OR_UNION_TYPE_P (t)) > + { > + if (tree *slot = hash_map_safe_get (consteval_only_class_cache, t)) > + return *slot == boolean_true_node; > + > + if (class_seen.add (t)) > + { > + /* Optimistically assume this already seen consteval-unknown class is > + not consteval only, for sake of mutually recursive classes. */ > + optimistic_p = true; > + return false; > + } > + class_stack.safe_push (t); > + > + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown (); > + for (tree member = TYPE_FIELDS (t); member; member = DECL_CHAIN (member)) > + if (TREE_CODE (member) == FIELD_DECL) > + { > + r = r || walk (TREE_TYPE (member)); > + if (r.is_true ()) > + break; > + } > + > + if (!COMPLETE_TYPE_P (t)) > + /* Until the type is laid out, we can't trust that its TYPE_FIELDS > + won't change. */; We might move the COMPLETE_TYPE check... > + else if (r.is_true ()) > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > + t, boolean_true_node); > + else if (r.is_false () ...into another exception for the is_false case, since I don't think the TYPE_FIELDS can change in a way that would invalidate is_true(). Actually, checking COMPLETE_TYPE_P here seems redundant with checking it in the initializer of r above; can we just drop it (and move the comment up)? > + /* The optimistic assumption above is at odds with caching > + 'false' results for a nested class type. */ > + && (class_stack.length () == 1 || !optimistic_p)) > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > + t, boolean_false_node); > + > + class_stack.pop (); > + return r; > + } > + else if (TYPE_PTRMEM_P (t)) > + return (walk (TYPE_PTRMEM_CLASS_TYPE (t)) > + || walk (TYPE_PTRMEM_POINTED_TO_TYPE (t))); > + else > + return false; > } > > /* A walker for check_out_of_consteval_use_r. It cannot be a lambda, because
On Tue, 5 May 2026, Jason Merrill wrote: > On 5/5/26 1:20 PM, Patrick Palka wrote: > > On Tue, 5 May 2026, Jason Merrill wrote: > > > > > On 5/5/26 11:21 AM, Patrick Palka wrote: > > > > On Tue, 5 May 2026, Patrick Palka wrote: > > > > > > > > > On Tue, 5 May 2026, Patrick Palka wrote: > > > > > > > > > > > On Tue, 5 May 2026, Jakub Jelinek wrote: > > > > > > > > > > > > > On Mon, May 04, 2026 at 09:25:06PM -0400, Patrick Palka wrote: > > > > > > > > + else if (RECORD_OR_UNION_TYPE_P (t)) > > > > > > > > + { > > > > > > > > + if (tree *slot = hash_map_safe_get > > > > > > > > (consteval_only_class_cache, t)) > > > > > > > > + return *slot == boolean_true_node; > > > > > > > > + > > > > > > > > + if (class_set.add (t)) > > > > > > > > + /* Handle struct A { A* p; } etc. */ > > > > > > > > + return false; > > > > > > > > + > > > > > > > > + tristate r = COMPLETE_TYPE_P (t) ? false : > > > > > > > > tristate::unknown > > > > > > > > (); > > > > > > > > + for (tree member = TYPE_FIELDS (t); member; member = > > > > > > > > DECL_CHAIN (member)) > > > > > > > > + if (TREE_CODE (member) == FIELD_DECL) > > > > > > > > + { > > > > > > > > + r = r || consteval_only_p_1 (TREE_TYPE (member), > > > > > > > > class_set); > > > > > > > > + if (r.is_true ()) > > > > > > > > + break; > > > > > > > > + } > > > > > > > > + > > > > > > > > + if (COMPLETE_TYPE_P (t)) > > > > > > > > + { > > > > > > > > + if (r.is_true ()) > > > > > > > > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > > > > > > > > + t, boolean_true_node); > > > > > > > > + else if (r.is_false ()) > > > > > > > > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > > > > > > > > + t, boolean_false_node); > > > > > > > > + } > > > > > > > > + > > > > > > > > + class_set.remove (t); > > > > > > > > > > > > > > Doesn't the above stmt result in really bad compile time > > > > > > > complexity? > > > > > > > If I have say > > > > > > > struct S1; > > > > > > > struct S2; > > > > > > > struct S3 { S2 *a; }; > > > > > > > struct S4 { S3 *a; }; > > > > > > > struct S5 { S4 *a; }; > > > > > > > ... > > > > > > > struct S9999 { S9998 *a; }; > > > > > > > struct S2 { S9999 *a; S1 *b; }; > > > > > > > then when asking if S2 is consteval-only, this will walk ~10000^2 > > > > > > > types > > > > > > > rather than just ~10000 types. Wouldn't it be better to > > > > > > > class_set.add (t) and never remove, but if we trigger that return > > > > > > > false; > > > > > > > in there, remember that we shouldn't cache r.is_false () results > > > > > > > (perhaps > > > > > > > with the exception when this was the non-recursive call)? > > > > > > > > > > > > Yes, and the current patch is buggy for: > > > > > > > > > > > > struct B { A* p; }; > > > > > > struct A { > > > > > > B m; > > > > > > meta::info n; > > > > > > }; > > > > > > > > > > > > We'd incorrectly deem B not consteval-only if we first walk it from > > > > > > A. > > > > > > > > > > > > It's apparently important for the testcase from this PR to return > > > > > > false > > > > > > rather than unknown when detecting mutual recursion, otherwise we > > > > > > still > > > > > > get a 4x slowdown. > > > > > > > > > > The reason it's important to optimistically return false upon hitting > > > > > mutual recursion is so that we get a chance to cache the result. > > > > > Otherwise we'll never cache the result for some mutually recursive > > > > > types; consteval_only_p_1 will always return unknown. > > > > > > > > > > > But if we must return false in the case of mutual > > > > > > recursion then indeed it's not safe to cache r.is_false () results > > > > > > except for the outermost class because of scenarios like B above. > > > > > > > > > > > > To fix this efficiently I think we need to maintian both a > > > > > > class_seen > > > > > > set and a class_stack. What do you think of the following? > > > > > > > > > > > > -- >8 -- > > > > > > > > > > > > PR c++/125179 > > > > > > > > > > > > gcc/cp/ChangeLog: > > > > > > > > > > > > * reflect.cc: Include <algorithm>. > > > > > > (consteval_only_type_r): Remove this cp_walk_tree callback and > > > > > > replace with ... > > > > > > (consteval_only_p_state, consteval_only_class_cache) > > > > > > (consteval_only_p_1): ... this recursive memoized > > > > > > implementation. > > > > > > (consteval_only_p): Define in terms of consteval_only_p_1. > > > > > > --- > > > > > > gcc/cp/reflect.cc | 138 > > > > > > ++++++++++++++++++++++++++++++++-------------- > > > > > > 1 file changed, 97 insertions(+), 41 deletions(-) > > > > > > > > > > > > diff --git a/gcc/cp/reflect.cc b/gcc/cp/reflect.cc > > > > > > index 3b9f56ea5484..67a1e1b85895 100644 > > > > > > --- a/gcc/cp/reflect.cc > > > > > > +++ b/gcc/cp/reflect.cc > > > > > > @@ -20,6 +20,7 @@ along with GCC; see the file COPYING3. If not see > > > > > > <http://www.gnu.org/licenses/>. */ > > > > > > #include "config.h" > > > > > > +#define INCLUDE_ALGORITHM > > > > > > #include "system.h" > > > > > > #include "coretypes.h" > > > > > > #include "target.h" > > > > > > @@ -8561,47 +8562,22 @@ splice (tree refl) > > > > > > return refl; > > > > > > } > > > > > > -/* A walker for consteval_only_p. It cannot be a lambda, > > > > > > because we > > > > > > - have to call this recursively, sigh. */ > > > > > > +/* State maintained during consteval_only_p_1 recursion. */ > > > > > > -static tree > > > > > > -consteval_only_type_r (tree *tp, int *walk_subtrees, void *data) > > > > > > +struct consteval_only_p_state > > > > > > { > > > > > > - tree t = *tp; > > > > > > - /* Types can contain themselves recursively, hence this. */ > > > > > > - auto visited = static_cast<hash_set<tree> *>(data); > > > > > > - > > > > > > - if (!TYPE_P (t)) > > > > > > - return NULL_TREE; > > > > > > - > > > > > > - if (REFLECTION_TYPE_P (t)) > > > > > > - return t; > > > > > > - > > > > > > - if (typedef_variant_p (t)) > > > > > > - /* Tell cp_walk_subtrees to look through typedefs. */ > > > > > > - *walk_subtrees = 2; > > > > > > - > > > > > > - if (RECORD_OR_UNION_TYPE_P (t)) > > > > > > - { > > > > > > - /* Don't walk template arguments; A<info>::type isn't a > > > > > > consteval-only > > > > > > - type. */ > > > > > > - *walk_subtrees = 0; > > > > > > - /* So we have to walk the fields manually. */ > > > > > > - for (tree member = TYPE_FIELDS (t); > > > > > > - member; member = DECL_CHAIN (member)) > > > > > > - if (TREE_CODE (member) == FIELD_DECL) > > > > > > - if (tree r = cp_walk_tree (&TREE_TYPE (member), > > > > > > - consteval_only_type_r, visited, > > > > > > visited)) > > > > > > - return r; > > > > > > - } > > > > > > + /* The stack of class types we're recursively inside. */ > > > > > > + auto_vec<tree> class_stack; > > > > > > + /* The set of class types we've seen. */ > > > > > > + hash_set<tree> class_seen; > > > > > > + /* Whether we've seen mutual class recursion. */ > > > > > > + bool seen_mutual_recursion = false; > > > > > > > > > > > +}; > > > > > > - return NULL_TREE; > > > > > > -} > > > > > > +static tristate consteval_only_p_1 (tree, consteval_only_p_state&); > > > > > > -/* True if T is a consteval-only type as per > > > > > > [basic.types.general]: > > > > > > - "A type is consteval-only if it is either std::meta::info or a > > > > > > type > > > > > > - compounded from a consteval-only type", or something that has > > > > > > - a consteval-only type. */ > > > > > > +/* True if T is a consteval-only type as per [basic.types.general], > > > > > > or > > > > > > + is a declaration with such a type, or a TREE_VEC thereof. */ > > > > > > bool > > > > > > consteval_only_p (tree t) > > > > > > @@ -8612,7 +8588,7 @@ consteval_only_p (tree t) > > > > > > if (!TYPE_P (t)) > > > > > > t = TREE_TYPE (t); > > > > > > - if (!t) > > > > > > + if (!t || t == error_mark_node) > > > > > > return false; > > > > > > if (TREE_CODE (t) == TREE_VEC) > > > > > > @@ -8634,9 +8610,89 @@ consteval_only_p (tree t) > > > > > > which could be consteval-only, depending on T. */ > > > > > > t = complete_type (t); > > > > > > - /* Classes with std::meta::info members are also > > > > > > consteval-only. > > > > > > */ > > > > > > - hash_set<tree> visited; > > > > > > - return !!cp_walk_tree (&t, consteval_only_type_r, &visited, > > > > > > &visited); > > > > > > + consteval_only_p_state state; > > > > > > + return consteval_only_p_1 (t, state).is_true (); > > > > > > +} > > > > > > + > > > > > > +/* A cache of the boolean result of consteval_only_p_1 for class > > > > > > types, > > > > > > when > > > > > > + the result is known. */ > > > > > > + > > > > > > +static GTY((cache)) type_tree_cache_map > > > > > > *consteval_only_class_cache; > > > > > > + > > > > > > +/* Recursive workhorse of consteval_only_p. Returns true if T is > > > > > > definitely > > > > > > + consteval-only, false if it's definitely not, and unknown if we > > > > > > saw > > > > > > an > > > > > > + incomplete type and therefore don't know. */ > > > > > > + > > > > > > +static tristate > > > > > > +consteval_only_p_1 (tree t, consteval_only_p_state& state) > > > > > > +{ > > > > > > + t = TYPE_MAIN_VARIANT (t); > > > > > > + > > > > > > + if (REFLECTION_TYPE_P (t)) > > > > > > + return true; > > > > > > + else if (INDIRECT_TYPE_P (t)) > > > > > > + return consteval_only_p_1 (TREE_TYPE (t), state); > > > > > > + else if (TREE_CODE (t) == ARRAY_TYPE) > > > > > > + return consteval_only_p_1 (TREE_TYPE (t), state); > > > > > > + else if (FUNC_OR_METHOD_TYPE_P (t)) > > > > > > + { > > > > > > + tristate r = consteval_only_p_1 (TREE_TYPE (t), state); > > > > > > + for (tree parm = TYPE_ARG_TYPES (t); > > > > > > + parm != NULL_TREE && parm != void_list_node; > > > > > > + parm = TREE_CHAIN (parm)) > > > > > > + { > > > > > > + r = r || consteval_only_p_1 (TREE_VALUE (parm), state); > > > > > > + if (r.is_true ()) > > > > > > + break; > > > > > > + } > > > > > > + return r; > > > > > > + } > > > > > > + else if (RECORD_OR_UNION_TYPE_P (t)) > > > > > > + { > > > > > > + if (tree *slot = hash_map_safe_get > > > > > > (consteval_only_class_cache, > > > > > > t)) > > > > > > + return *slot == boolean_true_node; > > > > > > + > > > > > > + if (state.class_seen.add (t)) > > > > > > + { > > > > > > + auto begin = state.class_stack.begin (); > > > > > > + auto end = state.class_stack.end (); > > > > > > + auto it = std::find (begin, end, t); > > > > > > + if (it != end && it != &state.class_stack.last ()) > > > > > > + state.seen_mutual_recursion = true; > > > > > > + > > > > > > + return false; > > > > > > + } > > > > > > + state.class_stack.safe_push (t); > > > > > > + > > > > > > + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown > > > > > > (); > > > > > > + for (tree member = TYPE_FIELDS (t); member; member = > > > > > > DECL_CHAIN > > > > > > (member)) > > > > > > + if (TREE_CODE (member) == FIELD_DECL) > > > > > > + { > > > > > > + r = r || consteval_only_p_1 (TREE_TYPE (member), state); > > > > > > + if (r.is_true ()) > > > > > > + break; > > > > > > + } > > > > > > + > > > > > > + if (!COMPLETE_TYPE_P (t)) > > > > > > + /* Until the type is laid out, we can't trust that its > > > > > > TYPE_FIELDS > > > > > > + won't change. */; > > > > > > + else if (r.is_true ()) > > > > > > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > > > > > > + t, boolean_true_node); > > > > > > + else if (r.is_false () > > > > > > + && (state.class_stack.length () == 1 > > > > > > + || !state.seen_mutual_recursion)) > > > > > > > > > > D'oh, this !state.seen_mutual_recursion exception for caching nested > > > > > types is wrong. > > > > > > > > > > struct A { > > > > > struct B { struct C *c; } b; > > > > > struct C { B b; } c; > > > > > }; > > > > > > > > > > Here if we start walking from A, which is not mutually recursively, we > > > > > first correctly deem B as unknown-consteval-only, but then go on to > > > > > deem the nested C as not consteval only _and_ cache it (since there's > > > > > no mutual recursion). > > > > > > > > > > So being optimistic about mutually recursive types (the > > > > > 'if (class_seen.add (t)) return false' early exit) is at odds > > > > > with caching the result for nested class types in general, unless > > > > > we significantly complicate things which is probably not worth it. > > > > > > > > Oh, we can just remember if we ever took the early exit and only disable > > > > false result caching for nested classes in that case. That should be > > > > safe, I think (famous last words), and allow us to cache nested classes > > > > in more cases. So for > > > > > > > > struct A { > > > > struct B { int n; } b; > > > > }; > > > > > > > > the false result for B will be cached, but for > > > > > > > > struct A { > > > > struct B { A* a; } b; > > > > struct C { int n; } c; > > > > }; > > > > > > > > neither the false result for B or for C will be cached. > > > > > > > > Here is v4 that introduces an 'optimistic_p' flag to that effect. This > > > > reduces the -freflection slowdown for the TU to 1.1x from 1.2x compared > > > > to v3. > > > > > > > > -- >8 -- > > > > > > > > PR c++/125179 > > > > > > > > gcc/cp/ChangeLog: > > > > > > > > * reflect.cc: (consteval_only_type_r): Remove this cp_walk_tree > > > > callback and replace with ... > > > > (consteval_only_p_state, consteval_only_class_cache) > > > > (consteval_only_p_1): ... this recursive memoized implementation. > > > > (consteval_only_p): Define in terms of consteval_only_p_1. > > > > --- > > > > gcc/cp/reflect.cc | 136 > > > > ++++++++++++++++++++++++++++++++-------------- > > > > 1 file changed, 95 insertions(+), 41 deletions(-) > > > > > > > > diff --git a/gcc/cp/reflect.cc b/gcc/cp/reflect.cc > > > > index 3b9f56ea5484..e9bd301fb84b 100644 > > > > --- a/gcc/cp/reflect.cc > > > > +++ b/gcc/cp/reflect.cc > > > > @@ -8561,47 +8561,23 @@ splice (tree refl) > > > > return refl; > > > > } > > > > -/* A walker for consteval_only_p. It cannot be a lambda, because we > > > > - have to call this recursively, sigh. */ > > > > +/* State maintained during consteval_only_p_1 recursion. */ > > > > -static tree > > > > -consteval_only_type_r (tree *tp, int *walk_subtrees, void *data) > > > > +struct consteval_only_p_state > > > > { > > > > - tree t = *tp; > > > > - /* Types can contain themselves recursively, hence this. */ > > > > - auto visited = static_cast<hash_set<tree> *>(data); > > > > - > > > > - if (!TYPE_P (t)) > > > > - return NULL_TREE; > > > > - > > > > - if (REFLECTION_TYPE_P (t)) > > > > - return t; > > > > - > > > > - if (typedef_variant_p (t)) > > > > - /* Tell cp_walk_subtrees to look through typedefs. */ > > > > - *walk_subtrees = 2; > > > > - > > > > - if (RECORD_OR_UNION_TYPE_P (t)) > > > > - { > > > > - /* Don't walk template arguments; A<info>::type isn't a > > > > consteval-only > > > > - type. */ > > > > - *walk_subtrees = 0; > > > > - /* So we have to walk the fields manually. */ > > > > - for (tree member = TYPE_FIELDS (t); > > > > - member; member = DECL_CHAIN (member)) > > > > - if (TREE_CODE (member) == FIELD_DECL) > > > > - if (tree r = cp_walk_tree (&TREE_TYPE (member), > > > > - consteval_only_type_r, visited, visited)) > > > > - return r; > > > > - } > > > > + /* The stack of class types we're recursively inside. */ > > > > + auto_vec<tree> class_stack; > > > > + /* The set of class types we've seen. */ > > > > + hash_set<tree> class_seen; > > > > + /* True if we've optimistically assumed an already-seen > > > > + consteval-unknown class type is not consteval. */ > > > > + bool optimistic_p = false; > > > > +}; > > > > - return NULL_TREE; > > > > -} > > > > +static tristate consteval_only_p_1 (tree, consteval_only_p_state&); > > > > > > How about making this a member function of consteval_only_p_state instead > > > of a > > > free function with a reference parameter? > > > > Good idea, done. > > > > > > > > > -/* True if T is a consteval-only type as per [basic.types.general]: > > > > - "A type is consteval-only if it is either std::meta::info or a type > > > > - compounded from a consteval-only type", or something that has > > > > - a consteval-only type. */ > > > > +/* True if T is a consteval-only type as per [basic.types.general], or > > > > + is a declaration with such a type, or a TREE_VEC thereof. */ > > > > bool > > > > consteval_only_p (tree t) > > > > @@ -8612,7 +8588,7 @@ consteval_only_p (tree t) > > > > if (!TYPE_P (t)) > > > > t = TREE_TYPE (t); > > > > - if (!t) > > > > + if (!t || t == error_mark_node) > > > > return false; > > > > if (TREE_CODE (t) == TREE_VEC) > > > > @@ -8634,9 +8610,87 @@ consteval_only_p (tree t) > > > > which could be consteval-only, depending on T. */ > > > > t = complete_type (t); > > > > - /* Classes with std::meta::info members are also consteval-only. > > > > */ > > > > - hash_set<tree> visited; > > > > - return !!cp_walk_tree (&t, consteval_only_type_r, &visited, > > > > &visited); > > > > + consteval_only_p_state state; > > > > + return consteval_only_p_1 (t, state).is_true (); > > > > +} > > > > + > > > > +/* A cache of the boolean result of consteval_only_p_1 for class types, > > > > when > > > > + the result is known. */ > > > > + > > > > +static GTY((cache)) type_tree_cache_map *consteval_only_class_cache; > > > > > > Since you aren't actually returning the tree value, I guess you're using > > > type_tree_cache_map mostly because it's an existing type and there isn't > > > one > > > for type to bool? > > > > Yeah, though in an earlier version of the patch the mapped values were > > tri-state so using bool wasn't an option. I'm not keen on creating > > a new map type just for this case, at least as part of this patch, the > > memory savings should be negligible due to padding and type_tree_cache_map > > is heavily used and well tested. > > Agreed. > > > > Also, from gty.texi: > > > > > > > Note that caches should generally use @code{deletable} instead; > > > > @code{cache} is only preferable if the value is impractical to > > > > recompute from the key when needed. > > > > > > This seems like a ((deletable)) case to me, so we could just use > > > hash_map<tree,bool>. > > > > Using ((deletable)) results in a 10x -freflection slowdown for the > > testcase from the PR, vs 1.1x with ((cache)), I suspect because it gets > > cleared every time we complete a class. > > Hunh, I'm surprised GC would happen often enough for that. Never mind, then, > thanks for checking. Yeah. I chalk it up to this testcase being kind of an outlier (but apparently it's real-world code). > > > > > +/* Recursive workhorse of consteval_only_p. Returns true if T is > > > > definitely > > > > + consteval-only, false if it's definitely not, and unknown if we saw > > > > an > > > > + incomplete type and therefore don't know. */ > > > > + > > > > +static tristate > > > > +consteval_only_p_1 (tree t, consteval_only_p_state& state) > > > > +{ > > > > + t = TYPE_MAIN_VARIANT (t); > > > > + > > > > + if (REFLECTION_TYPE_P (t)) > > > > + return true; > > > > + else if (INDIRECT_TYPE_P (t)) > > > > + return consteval_only_p_1 (TREE_TYPE (t), state); > > > > + else if (TREE_CODE (t) == ARRAY_TYPE) > > > > + return consteval_only_p_1 (TREE_TYPE (t), state); > > > > + else if (FUNC_OR_METHOD_TYPE_P (t)) > > > > + { > > > > + tristate r = consteval_only_p_1 (TREE_TYPE (t), state); > > > > + for (tree parm = TYPE_ARG_TYPES (t); > > > > + parm != NULL_TREE && parm != void_list_node; > > > > + parm = TREE_CHAIN (parm)) > > > > + { > > > > + r = r || consteval_only_p_1 (TREE_VALUE (parm), state); > > > > + if (r.is_true ()) > > > > > > Let's move the is_true check before the || so that we don't look at the > > > first > > > parm when the return type is consteval-only. > > > > Fixed. > > > > Here's v5 which uses a member function and fixes the above. The cache > > is still marked as ((cache)) instead of ((deletable)). > > > > -- >8 -- > > > > PR c++/125179 > > > > gcc/cp/ChangeLog: > > > > * reflect.cc: (consteval_only_type_r): Remove this cp_walk_tree > > callback and replace with ... > > (consteval_only_walker): ... this recursive memoized > > implementation. > > (consteval_only_p): Define in terms of consteval_only_walker. > > --- > > gcc/cp/reflect.cc | 134 ++++++++++++++++++++++++++++++++-------------- > > 1 file changed, 93 insertions(+), 41 deletions(-) > > > > diff --git a/gcc/cp/reflect.cc b/gcc/cp/reflect.cc > > index 3b9f56ea5484..6e7000f7de81 100644 > > --- a/gcc/cp/reflect.cc > > +++ b/gcc/cp/reflect.cc > > @@ -8561,47 +8561,26 @@ splice (tree refl) > > return refl; > > } > > -/* A walker for consteval_only_p. It cannot be a lambda, because we > > - have to call this recursively, sigh. */ > > +/* A cache of the known boolean result of consteval_only_p_walker::walk for > > + class types. */ > > -static tree > > -consteval_only_type_r (tree *tp, int *walk_subtrees, void *data) > > -{ > > - tree t = *tp; > > - /* Types can contain themselves recursively, hence this. */ > > - auto visited = static_cast<hash_set<tree> *>(data); > > - > > - if (!TYPE_P (t)) > > - return NULL_TREE; > > +static GTY((cache)) type_tree_cache_map *consteval_only_class_cache; > > - if (REFLECTION_TYPE_P (t)) > > - return t; > > - > > - if (typedef_variant_p (t)) > > - /* Tell cp_walk_subtrees to look through typedefs. */ > > - *walk_subtrees = 2; > > - > > - if (RECORD_OR_UNION_TYPE_P (t)) > > - { > > - /* Don't walk template arguments; A<info>::type isn't a > > consteval-only > > - type. */ > > - *walk_subtrees = 0; > > - /* So we have to walk the fields manually. */ > > - for (tree member = TYPE_FIELDS (t); > > - member; member = DECL_CHAIN (member)) > > - if (TREE_CODE (member) == FIELD_DECL) > > - if (tree r = cp_walk_tree (&TREE_TYPE (member), > > - consteval_only_type_r, visited, visited)) > > - return r; > > - } > > +struct consteval_only_p_walker > > +{ > > + /* The stack of class types we're recursively inside. */ > > + auto_vec<tree> class_stack; > > + /* The set of class types we've seen. */ > > + hash_set<tree> class_seen; > > + /* True if we've optimistically assumed an already-seen > > + consteval-unknown class type is not consteval. */ > > + bool optimistic_p = false; > > - return NULL_TREE; > > -} > > + tristate walk (tree); > > +}; > > -/* True if T is a consteval-only type as per [basic.types.general]: > > - "A type is consteval-only if it is either std::meta::info or a type > > - compounded from a consteval-only type", or something that has > > - a consteval-only type. */ > > +/* True if T is a consteval-only type as per [basic.types.general], or > > + is a declaration with such a type, or a TREE_VEC thereof. */ > > bool > > consteval_only_p (tree t) > > @@ -8612,7 +8591,7 @@ consteval_only_p (tree t) > > if (!TYPE_P (t)) > > t = TREE_TYPE (t); > > - if (!t) > > + if (!t || t == error_mark_node) > > return false; > > if (TREE_CODE (t) == TREE_VEC) > > @@ -8634,9 +8613,82 @@ consteval_only_p (tree t) > > which could be consteval-only, depending on T. */ > > t = complete_type (t); > > - /* Classes with std::meta::info members are also consteval-only. */ > > - hash_set<tree> visited; > > - return !!cp_walk_tree (&t, consteval_only_type_r, &visited, &visited); > > + consteval_only_p_walker walker; > > + return walker.walk (t).is_true (); > > +} > > + > > +/* Recursive workhorse of consteval_only_p. Returns true if T is > > definitely > > + consteval-only, false if it's definitely not, and unknown if we saw an > > + incomplete type and therefore don't know. */ > > + > > +tristate > > +consteval_only_p_walker::walk (tree t) > > +{ > > + t = TYPE_MAIN_VARIANT (t); > > + > > + if (REFLECTION_TYPE_P (t)) > > + return true; > > + else if (INDIRECT_TYPE_P (t)) > > + return walk (TREE_TYPE (t)); > > + else if (TREE_CODE (t) == ARRAY_TYPE) > > + return walk (TREE_TYPE (t)); > > + else if (FUNC_OR_METHOD_TYPE_P (t)) > > + { > > + tristate r = walk (TREE_TYPE (t)); > > + for (tree parm = TYPE_ARG_TYPES (t); > > + parm != NULL_TREE && parm != void_list_node; > > + parm = TREE_CHAIN (parm)) > > + { > > + if (r.is_true ()) > > + break; > > + r = r || walk (TREE_VALUE (parm)); > > + } > > + return r; > > + } > > + else if (RECORD_OR_UNION_TYPE_P (t)) > > + { > > + if (tree *slot = hash_map_safe_get (consteval_only_class_cache, t)) > > + return *slot == boolean_true_node; > > + > > + if (class_seen.add (t)) > > + { > > + /* Optimistically assume this already seen consteval-unknown class > > is > > + not consteval only, for sake of mutually recursive classes. */ > > + optimistic_p = true; > > + return false; > > + } > > + class_stack.safe_push (t); > > + > > + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown (); > > + for (tree member = TYPE_FIELDS (t); member; member = DECL_CHAIN > > (member)) > > + if (TREE_CODE (member) == FIELD_DECL) > > + { > > + r = r || walk (TREE_TYPE (member)); > > + if (r.is_true ()) > > + break; > > + } > > + > > + if (!COMPLETE_TYPE_P (t)) > > + /* Until the type is laid out, we can't trust that its TYPE_FIELDS > > + won't change. */; > > We might move the COMPLETE_TYPE check... > > > + else if (r.is_true ()) > > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > > + t, boolean_true_node); > > + else if (r.is_false () > > ...into another exception for the is_false case, since I don't think the > TYPE_FIELDS can change in a way that would invalidate is_true(). Sadly, it can! prune_lambda_captures can remove consteval-only TYPE_FIELDS (corresponding to folded-away captures) before the closure type has been laid out, so if we'd cache the result now we'd get the wrong answer after pruning. Without this second COMPLETE_TYPE_P check we'd get a bogus error in reflect/reflect_constant_array6.C due to treating the lambda as consteval even after pruning. > > Actually, checking COMPLETE_TYPE_P here seems redundant with checking it in > the initializer of r above; can we just drop it (and move the comment up)? > > > + /* The optimistic assumption above is at odds with caching > > + 'false' results for a nested class type. */ > > + && (class_stack.length () == 1 || !optimistic_p)) > > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > > + t, boolean_false_node); > > + > > + class_stack.pop (); > > + return r; > > + } > > + else if (TYPE_PTRMEM_P (t)) > > + return (walk (TYPE_PTRMEM_CLASS_TYPE (t)) > > + || walk (TYPE_PTRMEM_POINTED_TO_TYPE (t))); > > + else > > + return false; > > } > > /* A walker for check_out_of_consteval_use_r. It cannot be a lambda, > > because > >
On 5/5/26 3:53 PM, Patrick Palka wrote: > On Tue, 5 May 2026, Jason Merrill wrote: >> On 5/5/26 1:20 PM, Patrick Palka wrote: >>> +/* Recursive workhorse of consteval_only_p. Returns true if T is >>> definitely >>> + consteval-only, false if it's definitely not, and unknown if we saw an >>> + incomplete type and therefore don't know. */ >>> + >>> +tristate >>> +consteval_only_p_walker::walk (tree t) >>> +{ >>> + t = TYPE_MAIN_VARIANT (t); >>> + >>> + if (REFLECTION_TYPE_P (t)) >>> + return true; >>> + else if (INDIRECT_TYPE_P (t)) >>> + return walk (TREE_TYPE (t)); >>> + else if (TREE_CODE (t) == ARRAY_TYPE) >>> + return walk (TREE_TYPE (t)); >>> + else if (FUNC_OR_METHOD_TYPE_P (t)) >>> + { >>> + tristate r = walk (TREE_TYPE (t)); >>> + for (tree parm = TYPE_ARG_TYPES (t); >>> + parm != NULL_TREE && parm != void_list_node; >>> + parm = TREE_CHAIN (parm)) >>> + { >>> + if (r.is_true ()) >>> + break; >>> + r = r || walk (TREE_VALUE (parm)); >>> + } >>> + return r; >>> + } >>> + else if (RECORD_OR_UNION_TYPE_P (t)) >>> + { >>> + if (tree *slot = hash_map_safe_get (consteval_only_class_cache, t)) >>> + return *slot == boolean_true_node; >>> + >>> + if (class_seen.add (t)) >>> + { >>> + /* Optimistically assume this already seen consteval-unknown class >>> is >>> + not consteval only, for sake of mutually recursive classes. */ >>> + optimistic_p = true; >>> + return false; >>> + } >>> + class_stack.safe_push (t); >>> + >>> + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown (); >>> + for (tree member = TYPE_FIELDS (t); member; member = DECL_CHAIN >>> (member)) >>> + if (TREE_CODE (member) == FIELD_DECL) >>> + { >>> + r = r || walk (TREE_TYPE (member)); >>> + if (r.is_true ()) >>> + break; >>> + } >>> + >>> + if (!COMPLETE_TYPE_P (t)) >>> + /* Until the type is laid out, we can't trust that its TYPE_FIELDS >>> + won't change. */; >> >> We might move the COMPLETE_TYPE check... >> >>> + else if (r.is_true ()) >>> + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, >>> + t, boolean_true_node); >>> + else if (r.is_false () >> >> ...into another exception for the is_false case, since I don't think the >> TYPE_FIELDS can change in a way that would invalidate is_true(). > > Sadly, it can! prune_lambda_captures can remove consteval-only > TYPE_FIELDS (corresponding to folded-away captures) before the closure > type has been laid out, so if we'd cache the result now we'd get the > wrong answer after pruning. > > Without this second COMPLETE_TYPE_P check we'd get a bogus error in > reflect/reflect_constant_array6.C due to treating the lambda as > consteval even after pruning. Ah, lambdas, always keeping us on our toes. But then we could skip walking the fields at all if we're going to return unknown no matter what we see. Jason
On Tue, 5 May 2026, Jason Merrill wrote: > On 5/5/26 3:53 PM, Patrick Palka wrote: > > On Tue, 5 May 2026, Jason Merrill wrote: > > > On 5/5/26 1:20 PM, Patrick Palka wrote: > > > > +/* Recursive workhorse of consteval_only_p. Returns true if T is > > > > definitely > > > > + consteval-only, false if it's definitely not, and unknown if we saw > > > > an > > > > + incomplete type and therefore don't know. */ > > > > + > > > > +tristate > > > > +consteval_only_p_walker::walk (tree t) > > > > +{ > > > > + t = TYPE_MAIN_VARIANT (t); > > > > + > > > > + if (REFLECTION_TYPE_P (t)) > > > > + return true; > > > > + else if (INDIRECT_TYPE_P (t)) > > > > + return walk (TREE_TYPE (t)); > > > > + else if (TREE_CODE (t) == ARRAY_TYPE) > > > > + return walk (TREE_TYPE (t)); > > > > + else if (FUNC_OR_METHOD_TYPE_P (t)) > > > > + { > > > > + tristate r = walk (TREE_TYPE (t)); > > > > + for (tree parm = TYPE_ARG_TYPES (t); > > > > + parm != NULL_TREE && parm != void_list_node; > > > > + parm = TREE_CHAIN (parm)) > > > > + { > > > > + if (r.is_true ()) > > > > + break; > > > > + r = r || walk (TREE_VALUE (parm)); > > > > + } > > > > + return r; > > > > + } > > > > + else if (RECORD_OR_UNION_TYPE_P (t)) > > > > + { > > > > + if (tree *slot = hash_map_safe_get (consteval_only_class_cache, > > > > t)) > > > > + return *slot == boolean_true_node; > > > > + > > > > + if (class_seen.add (t)) > > > > + { > > > > + /* Optimistically assume this already seen consteval-unknown class > > > > is > > > > + not consteval only, for sake of mutually recursive classes. */ > > > > + optimistic_p = true; > > > > + return false; > > > > + } > > > > + class_stack.safe_push (t); > > > > + > > > > + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown (); > > > > + for (tree member = TYPE_FIELDS (t); member; member = DECL_CHAIN > > > > (member)) > > > > + if (TREE_CODE (member) == FIELD_DECL) > > > > + { > > > > + r = r || walk (TREE_TYPE (member)); > > > > + if (r.is_true ()) > > > > + break; > > > > + } > > > > + > > > > + if (!COMPLETE_TYPE_P (t)) > > > > + /* Until the type is laid out, we can't trust that its TYPE_FIELDS > > > > + won't change. */; > > > > > > We might move the COMPLETE_TYPE check... > > > > > > > + else if (r.is_true ()) > > > > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > > > > + t, boolean_true_node); > > > > + else if (r.is_false () > > > > > > ...into another exception for the is_false case, since I don't think the > > > TYPE_FIELDS can change in a way that would invalidate is_true(). > > > > Sadly, it can! prune_lambda_captures can remove consteval-only > > TYPE_FIELDS (corresponding to folded-away captures) before the closure > > type has been laid out, so if we'd cache the result now we'd get the > > wrong answer after pruning. > > > > Without this second COMPLETE_TYPE_P check we'd get a bogus error in > > reflect/reflect_constant_array6.C due to treating the lambda as > > consteval even after pruning. > > Ah, lambdas, always keeping us on our toes. But then we could skip walking > the fields at all if we're going to return unknown no matter what we see. In that lambda case, where TYPE_FIELDS is set but TYPE_COMPLETE_P is false, we won't always return unknonwn, we could return true if one of its TYPE_FIELDS is consteval-only. > > Jason > >
On 5/5/26 4:34 PM, Patrick Palka wrote: > On Tue, 5 May 2026, Jason Merrill wrote: > >> On 5/5/26 3:53 PM, Patrick Palka wrote: >>> On Tue, 5 May 2026, Jason Merrill wrote: >>>> On 5/5/26 1:20 PM, Patrick Palka wrote: >>>>> +/* Recursive workhorse of consteval_only_p. Returns true if T is >>>>> definitely >>>>> + consteval-only, false if it's definitely not, and unknown if we saw >>>>> an >>>>> + incomplete type and therefore don't know. */ >>>>> + >>>>> +tristate >>>>> +consteval_only_p_walker::walk (tree t) >>>>> +{ >>>>> + t = TYPE_MAIN_VARIANT (t); >>>>> + >>>>> + if (REFLECTION_TYPE_P (t)) >>>>> + return true; >>>>> + else if (INDIRECT_TYPE_P (t)) >>>>> + return walk (TREE_TYPE (t)); >>>>> + else if (TREE_CODE (t) == ARRAY_TYPE) >>>>> + return walk (TREE_TYPE (t)); >>>>> + else if (FUNC_OR_METHOD_TYPE_P (t)) >>>>> + { >>>>> + tristate r = walk (TREE_TYPE (t)); >>>>> + for (tree parm = TYPE_ARG_TYPES (t); >>>>> + parm != NULL_TREE && parm != void_list_node; >>>>> + parm = TREE_CHAIN (parm)) >>>>> + { >>>>> + if (r.is_true ()) >>>>> + break; >>>>> + r = r || walk (TREE_VALUE (parm)); >>>>> + } >>>>> + return r; >>>>> + } >>>>> + else if (RECORD_OR_UNION_TYPE_P (t)) >>>>> + { >>>>> + if (tree *slot = hash_map_safe_get (consteval_only_class_cache, >>>>> t)) >>>>> + return *slot == boolean_true_node; >>>>> + >>>>> + if (class_seen.add (t)) >>>>> + { >>>>> + /* Optimistically assume this already seen consteval-unknown class >>>>> is >>>>> + not consteval only, for sake of mutually recursive classes. */ >>>>> + optimistic_p = true; >>>>> + return false; >>>>> + } >>>>> + class_stack.safe_push (t); >>>>> + >>>>> + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown (); >>>>> + for (tree member = TYPE_FIELDS (t); member; member = DECL_CHAIN >>>>> (member)) >>>>> + if (TREE_CODE (member) == FIELD_DECL) >>>>> + { >>>>> + r = r || walk (TREE_TYPE (member)); >>>>> + if (r.is_true ()) >>>>> + break; >>>>> + } >>>>> + >>>>> + if (!COMPLETE_TYPE_P (t)) >>>>> + /* Until the type is laid out, we can't trust that its TYPE_FIELDS >>>>> + won't change. */; >>>> >>>> We might move the COMPLETE_TYPE check... >>>> >>>>> + else if (r.is_true ()) >>>>> + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, >>>>> + t, boolean_true_node); >>>>> + else if (r.is_false () >>>> >>>> ...into another exception for the is_false case, since I don't think the >>>> TYPE_FIELDS can change in a way that would invalidate is_true(). >>> >>> Sadly, it can! prune_lambda_captures can remove consteval-only >>> TYPE_FIELDS (corresponding to folded-away captures) before the closure >>> type has been laid out, so if we'd cache the result now we'd get the >>> wrong answer after pruning. >>> >>> Without this second COMPLETE_TYPE_P check we'd get a bogus error in >>> reflect/reflect_constant_array6.C due to treating the lambda as >>> consteval even after pruning. >> >> Ah, lambdas, always keeping us on our toes. But then we could skip walking >> the fields at all if we're going to return unknown no matter what we see. > > In that lambda case, where TYPE_FIELDS is set but TYPE_COMPLETE_P is > false, we won't always return unknown, we could return true if one of > its TYPE_FIELDS is consteval-only. Hmm, we currently return true but don't cache it? So we could be returning consteval-only about a type that later turns out not to be. That doesn't seem better to me than just returning unknown. Jason
On Tue, 5 May 2026, Jason Merrill wrote: > On 5/5/26 4:34 PM, Patrick Palka wrote: > > On Tue, 5 May 2026, Jason Merrill wrote: > > > > > On 5/5/26 3:53 PM, Patrick Palka wrote: > > > > On Tue, 5 May 2026, Jason Merrill wrote: > > > > > On 5/5/26 1:20 PM, Patrick Palka wrote: > > > > > > +/* Recursive workhorse of consteval_only_p. Returns true if T is > > > > > > definitely > > > > > > + consteval-only, false if it's definitely not, and unknown if we > > > > > > saw > > > > > > an > > > > > > + incomplete type and therefore don't know. */ > > > > > > + > > > > > > +tristate > > > > > > +consteval_only_p_walker::walk (tree t) > > > > > > +{ > > > > > > + t = TYPE_MAIN_VARIANT (t); > > > > > > + > > > > > > + if (REFLECTION_TYPE_P (t)) > > > > > > + return true; > > > > > > + else if (INDIRECT_TYPE_P (t)) > > > > > > + return walk (TREE_TYPE (t)); > > > > > > + else if (TREE_CODE (t) == ARRAY_TYPE) > > > > > > + return walk (TREE_TYPE (t)); > > > > > > + else if (FUNC_OR_METHOD_TYPE_P (t)) > > > > > > + { > > > > > > + tristate r = walk (TREE_TYPE (t)); > > > > > > + for (tree parm = TYPE_ARG_TYPES (t); > > > > > > + parm != NULL_TREE && parm != void_list_node; > > > > > > + parm = TREE_CHAIN (parm)) > > > > > > + { > > > > > > + if (r.is_true ()) > > > > > > + break; > > > > > > + r = r || walk (TREE_VALUE (parm)); > > > > > > + } > > > > > > + return r; > > > > > > + } > > > > > > + else if (RECORD_OR_UNION_TYPE_P (t)) > > > > > > + { > > > > > > + if (tree *slot = hash_map_safe_get > > > > > > (consteval_only_class_cache, > > > > > > t)) > > > > > > + return *slot == boolean_true_node; > > > > > > + > > > > > > + if (class_seen.add (t)) > > > > > > + { > > > > > > + /* Optimistically assume this already seen consteval-unknown > > > > > > class > > > > > > is > > > > > > + not consteval only, for sake of mutually recursive > > > > > > classes. */ > > > > > > + optimistic_p = true; > > > > > > + return false; > > > > > > + } > > > > > > + class_stack.safe_push (t); > > > > > > + > > > > > > + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown > > > > > > (); > > > > > > + for (tree member = TYPE_FIELDS (t); member; member = > > > > > > DECL_CHAIN > > > > > > (member)) > > > > > > + if (TREE_CODE (member) == FIELD_DECL) > > > > > > + { > > > > > > + r = r || walk (TREE_TYPE (member)); > > > > > > + if (r.is_true ()) > > > > > > + break; > > > > > > + } > > > > > > + > > > > > > + if (!COMPLETE_TYPE_P (t)) > > > > > > + /* Until the type is laid out, we can't trust that its > > > > > > TYPE_FIELDS > > > > > > + won't change. */; > > > > > > > > > > We might move the COMPLETE_TYPE check... > > > > > > > > > > > + else if (r.is_true ()) > > > > > > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > > > > > > + t, boolean_true_node); > > > > > > + else if (r.is_false () > > > > > > > > > > ...into another exception for the is_false case, since I don't think > > > > > the > > > > > TYPE_FIELDS can change in a way that would invalidate is_true(). > > > > > > > > Sadly, it can! prune_lambda_captures can remove consteval-only > > > > TYPE_FIELDS (corresponding to folded-away captures) before the closure > > > > type has been laid out, so if we'd cache the result now we'd get the > > > > wrong answer after pruning. > > > > > > > > Without this second COMPLETE_TYPE_P check we'd get a bogus error in > > > > reflect/reflect_constant_array6.C due to treating the lambda as > > > > consteval even after pruning. > > > > > > Ah, lambdas, always keeping us on our toes. But then we could skip > > > walking > > > the fields at all if we're going to return unknown no matter what we see. > > > > In that lambda case, where TYPE_FIELDS is set but TYPE_COMPLETE_P is > > false, we won't always return unknown, we could return true if one of > > its TYPE_FIELDS is consteval-only. > > Hmm, we currently return true but don't cache it? So we could be returning > consteval-only about a type that later turns out not to be. That doesn't seem > better to me than just returning unknown. Always returning unknown for !COMPLETE_TYPE_P class types seems to break <meta>: /scratchpad/gcc-master-build/x86_64-pc-linux-gnu/libstdc++-v3/include/meta:117:7: error: ‘consteval’ function ‘virtual consteval const char* std::meta::exception::what() const’ overriding non-‘consteval’ function 117 | what() const noexcept override | ^~~~ In file included from /home/patrick/code/gcc-master/libstdc++-v3/libsupc++/new:43, from /scratchpad/gcc-master-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_construct.h:59, from /scratchpad/gcc-master-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/char_traits.h:59, from /scratchpad/gcc-master-build/x86_64-pc-linux-gnu/libstdc++-v3/include/string:45, from gcc/testsuite/g++.dg/reflect/reflect_constant_array6.C:5: /scratchpad/gcc-master-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/exception.h:83:5: note: overridden function is ‘virtual constexpr const char* std::exception::what() const’ 83 | what() const _GLIBCXX_TXN_SAFE_DYN noexcept { return "std::exception"; } | ^~~~ Reduced: struct exception { virtual void foo(); }; struct meta_exception : exception { decltype(^^int) *p; consteval virtual void foo() { } }; because check_for_override is called before the class is fully laid out and COMPLETE_TYPE_P is not yet set, but we still need consteval_only_p to return true at this point. That doesn't seem ideal :/
On 5/5/26 5:24 PM, Patrick Palka wrote: > On Tue, 5 May 2026, Jason Merrill wrote: > >> On 5/5/26 4:34 PM, Patrick Palka wrote: >>> On Tue, 5 May 2026, Jason Merrill wrote: >>> >>>> On 5/5/26 3:53 PM, Patrick Palka wrote: >>>>> On Tue, 5 May 2026, Jason Merrill wrote: >>>>>> On 5/5/26 1:20 PM, Patrick Palka wrote: >>>>>>> +/* Recursive workhorse of consteval_only_p. Returns true if T is >>>>>>> definitely >>>>>>> + consteval-only, false if it's definitely not, and unknown if we >>>>>>> saw >>>>>>> an >>>>>>> + incomplete type and therefore don't know. */ >>>>>>> + >>>>>>> +tristate >>>>>>> +consteval_only_p_walker::walk (tree t) >>>>>>> +{ >>>>>>> + t = TYPE_MAIN_VARIANT (t); >>>>>>> + >>>>>>> + if (REFLECTION_TYPE_P (t)) >>>>>>> + return true; >>>>>>> + else if (INDIRECT_TYPE_P (t)) >>>>>>> + return walk (TREE_TYPE (t)); >>>>>>> + else if (TREE_CODE (t) == ARRAY_TYPE) >>>>>>> + return walk (TREE_TYPE (t)); >>>>>>> + else if (FUNC_OR_METHOD_TYPE_P (t)) >>>>>>> + { >>>>>>> + tristate r = walk (TREE_TYPE (t)); >>>>>>> + for (tree parm = TYPE_ARG_TYPES (t); >>>>>>> + parm != NULL_TREE && parm != void_list_node; >>>>>>> + parm = TREE_CHAIN (parm)) >>>>>>> + { >>>>>>> + if (r.is_true ()) >>>>>>> + break; >>>>>>> + r = r || walk (TREE_VALUE (parm)); >>>>>>> + } >>>>>>> + return r; >>>>>>> + } >>>>>>> + else if (RECORD_OR_UNION_TYPE_P (t)) >>>>>>> + { >>>>>>> + if (tree *slot = hash_map_safe_get >>>>>>> (consteval_only_class_cache, >>>>>>> t)) >>>>>>> + return *slot == boolean_true_node; >>>>>>> + >>>>>>> + if (class_seen.add (t)) >>>>>>> + { >>>>>>> + /* Optimistically assume this already seen consteval-unknown >>>>>>> class >>>>>>> is >>>>>>> + not consteval only, for sake of mutually recursive >>>>>>> classes. */ >>>>>>> + optimistic_p = true; >>>>>>> + return false; >>>>>>> + } >>>>>>> + class_stack.safe_push (t); >>>>>>> + >>>>>>> + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown >>>>>>> (); >>>>>>> + for (tree member = TYPE_FIELDS (t); member; member = >>>>>>> DECL_CHAIN >>>>>>> (member)) >>>>>>> + if (TREE_CODE (member) == FIELD_DECL) >>>>>>> + { >>>>>>> + r = r || walk (TREE_TYPE (member)); >>>>>>> + if (r.is_true ()) >>>>>>> + break; >>>>>>> + } >>>>>>> + >>>>>>> + if (!COMPLETE_TYPE_P (t)) >>>>>>> + /* Until the type is laid out, we can't trust that its >>>>>>> TYPE_FIELDS >>>>>>> + won't change. */; >>>>>> >>>>>> We might move the COMPLETE_TYPE check... >>>>>> >>>>>>> + else if (r.is_true ()) >>>>>>> + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, >>>>>>> + t, boolean_true_node); >>>>>>> + else if (r.is_false () >>>>>> >>>>>> ...into another exception for the is_false case, since I don't think >>>>>> the >>>>>> TYPE_FIELDS can change in a way that would invalidate is_true(). >>>>> >>>>> Sadly, it can! prune_lambda_captures can remove consteval-only >>>>> TYPE_FIELDS (corresponding to folded-away captures) before the closure >>>>> type has been laid out, so if we'd cache the result now we'd get the >>>>> wrong answer after pruning. >>>>> >>>>> Without this second COMPLETE_TYPE_P check we'd get a bogus error in >>>>> reflect/reflect_constant_array6.C due to treating the lambda as >>>>> consteval even after pruning. >>>> >>>> Ah, lambdas, always keeping us on our toes. But then we could skip >>>> walking >>>> the fields at all if we're going to return unknown no matter what we see. >>> >>> In that lambda case, where TYPE_FIELDS is set but TYPE_COMPLETE_P is >>> false, we won't always return unknown, we could return true if one of >>> its TYPE_FIELDS is consteval-only. >> >> Hmm, we currently return true but don't cache it? So we could be returning >> consteval-only about a type that later turns out not to be. That doesn't seem >> better to me than just returning unknown. > > Always returning unknown for !COMPLETE_TYPE_P class types seems to break <meta>: > > /scratchpad/gcc-master-build/x86_64-pc-linux-gnu/libstdc++-v3/include/meta:117:7: error: ‘consteval’ function ‘virtual consteval const char* std::meta::exception::what() const’ overriding non-‘consteval’ function > 117 | what() const noexcept override > | ^~~~ > In file included from /home/patrick/code/gcc-master/libstdc++-v3/libsupc++/new:43, > from /scratchpad/gcc-master-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_construct.h:59, > from /scratchpad/gcc-master-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/char_traits.h:59, > from /scratchpad/gcc-master-build/x86_64-pc-linux-gnu/libstdc++-v3/include/string:45, > from gcc/testsuite/g++.dg/reflect/reflect_constant_array6.C:5: > /scratchpad/gcc-master-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/exception.h:83:5: note: overridden function is ‘virtual constexpr const char* std::exception::what() const’ > 83 | what() const _GLIBCXX_TXN_SAFE_DYN noexcept { return "std::exception"; } > | ^~~~ > > Reduced: > > struct exception { > virtual void foo(); > }; > > struct meta_exception : exception { > decltype(^^int) *p; > consteval virtual void foo() { } > }; > > because check_for_override is called before the class is fully laid out > and COMPLETE_TYPE_P is not yet set, but we still need consteval_only_p > to return true at this point. That doesn't seem ideal :/ How about always returning unknown for closures, but returning/caching true for other classes? Jason
On Tue, 5 May 2026, Jason Merrill wrote: > On 5/5/26 5:24 PM, Patrick Palka wrote: > > On Tue, 5 May 2026, Jason Merrill wrote: > > > > > On 5/5/26 4:34 PM, Patrick Palka wrote: > > > > On Tue, 5 May 2026, Jason Merrill wrote: > > > > > > > > > On 5/5/26 3:53 PM, Patrick Palka wrote: > > > > > > On Tue, 5 May 2026, Jason Merrill wrote: > > > > > > > On 5/5/26 1:20 PM, Patrick Palka wrote: > > > > > > > > +/* Recursive workhorse of consteval_only_p. Returns true if T > > > > > > > > is > > > > > > > > definitely > > > > > > > > + consteval-only, false if it's definitely not, and unknown if > > > > > > > > we > > > > > > > > saw > > > > > > > > an > > > > > > > > + incomplete type and therefore don't know. */ > > > > > > > > + > > > > > > > > +tristate > > > > > > > > +consteval_only_p_walker::walk (tree t) > > > > > > > > +{ > > > > > > > > + t = TYPE_MAIN_VARIANT (t); > > > > > > > > + > > > > > > > > + if (REFLECTION_TYPE_P (t)) > > > > > > > > + return true; > > > > > > > > + else if (INDIRECT_TYPE_P (t)) > > > > > > > > + return walk (TREE_TYPE (t)); > > > > > > > > + else if (TREE_CODE (t) == ARRAY_TYPE) > > > > > > > > + return walk (TREE_TYPE (t)); > > > > > > > > + else if (FUNC_OR_METHOD_TYPE_P (t)) > > > > > > > > + { > > > > > > > > + tristate r = walk (TREE_TYPE (t)); > > > > > > > > + for (tree parm = TYPE_ARG_TYPES (t); > > > > > > > > + parm != NULL_TREE && parm != void_list_node; > > > > > > > > + parm = TREE_CHAIN (parm)) > > > > > > > > + { > > > > > > > > + if (r.is_true ()) > > > > > > > > + break; > > > > > > > > + r = r || walk (TREE_VALUE (parm)); > > > > > > > > + } > > > > > > > > + return r; > > > > > > > > + } > > > > > > > > + else if (RECORD_OR_UNION_TYPE_P (t)) > > > > > > > > + { > > > > > > > > + if (tree *slot = hash_map_safe_get > > > > > > > > (consteval_only_class_cache, > > > > > > > > t)) > > > > > > > > + return *slot == boolean_true_node; > > > > > > > > + > > > > > > > > + if (class_seen.add (t)) > > > > > > > > + { > > > > > > > > + /* Optimistically assume this already seen consteval-unknown > > > > > > > > class > > > > > > > > is > > > > > > > > + not consteval only, for sake of mutually recursive > > > > > > > > classes. */ > > > > > > > > + optimistic_p = true; > > > > > > > > + return false; > > > > > > > > + } > > > > > > > > + class_stack.safe_push (t); > > > > > > > > + > > > > > > > > + tristate r = COMPLETE_TYPE_P (t) ? false : > > > > > > > > tristate::unknown > > > > > > > > (); > > > > > > > > + for (tree member = TYPE_FIELDS (t); member; member = > > > > > > > > DECL_CHAIN > > > > > > > > (member)) > > > > > > > > + if (TREE_CODE (member) == FIELD_DECL) > > > > > > > > + { > > > > > > > > + r = r || walk (TREE_TYPE (member)); > > > > > > > > + if (r.is_true ()) > > > > > > > > + break; > > > > > > > > + } > > > > > > > > + > > > > > > > > + if (!COMPLETE_TYPE_P (t)) > > > > > > > > + /* Until the type is laid out, we can't trust that its > > > > > > > > TYPE_FIELDS > > > > > > > > + won't change. */; > > > > > > > > > > > > > > We might move the COMPLETE_TYPE check... > > > > > > > > > > > > > > > + else if (r.is_true ()) > > > > > > > > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > > > > > > > > + t, boolean_true_node); > > > > > > > > + else if (r.is_false () > > > > > > > > > > > > > > ...into another exception for the is_false case, since I don't > > > > > > > think > > > > > > > the > > > > > > > TYPE_FIELDS can change in a way that would invalidate is_true(). > > > > > > > > > > > > Sadly, it can! prune_lambda_captures can remove consteval-only > > > > > > TYPE_FIELDS (corresponding to folded-away captures) before the > > > > > > closure > > > > > > type has been laid out, so if we'd cache the result now we'd get the > > > > > > wrong answer after pruning. > > > > > > > > > > > > Without this second COMPLETE_TYPE_P check we'd get a bogus error in > > > > > > reflect/reflect_constant_array6.C due to treating the lambda as > > > > > > consteval even after pruning. > > > > > > > > > > Ah, lambdas, always keeping us on our toes. But then we could skip > > > > > walking > > > > > the fields at all if we're going to return unknown no matter what we > > > > > see. > > > > > > > > In that lambda case, where TYPE_FIELDS is set but TYPE_COMPLETE_P is > > > > false, we won't always return unknown, we could return true if one of > > > > its TYPE_FIELDS is consteval-only. > > > > > > Hmm, we currently return true but don't cache it? So we could be > > > returning > > > consteval-only about a type that later turns out not to be. That doesn't > > > seem > > > better to me than just returning unknown. > > > > Always returning unknown for !COMPLETE_TYPE_P class types seems to break > > <meta>: > > > > /scratchpad/gcc-master-build/x86_64-pc-linux-gnu/libstdc++-v3/include/meta:117:7: > > error: ‘consteval’ function ‘virtual consteval const char* > > std::meta::exception::what() const’ overriding non-‘consteval’ function > > 117 | what() const noexcept override > > | ^~~~ > > In file included from > > /home/patrick/code/gcc-master/libstdc++-v3/libsupc++/new:43, > > from > > /scratchpad/gcc-master-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_construct.h:59, > > from > > /scratchpad/gcc-master-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/char_traits.h:59, > > from > > /scratchpad/gcc-master-build/x86_64-pc-linux-gnu/libstdc++-v3/include/string:45, > > from > > gcc/testsuite/g++.dg/reflect/reflect_constant_array6.C:5: > > /scratchpad/gcc-master-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/exception.h:83:5: > > note: overridden function is ‘virtual constexpr const char* > > std::exception::what() const’ > > 83 | what() const _GLIBCXX_TXN_SAFE_DYN noexcept { return > > "std::exception"; } > > | ^~~~ > > > > Reduced: > > > > struct exception { > > virtual void foo(); > > }; > > > > struct meta_exception : exception { > > decltype(^^int) *p; > > consteval virtual void foo() { } > > }; > > > > because check_for_override is called before the class is fully laid out > > and COMPLETE_TYPE_P is not yet set, but we still need consteval_only_p > > to return true at this point. That doesn't seem ideal :/ > > How about always returning unknown for closures, but returning/caching true > for other classes? Makes sense, like so? Passes dg.exp=*reflect* so far. -- >8 -- PR c++/125179 gcc/cp/ChangeLog: * reflect.cc: (consteval_only_type_r): Remove this cp_walk_tree callback and replace with ... (consteval_only_walker): ... this recursive memoized implementation. (consteval_only_p): Define in terms of consteval_only_walker. --- gcc/cp/reflect.cc | 135 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 94 insertions(+), 41 deletions(-) diff --git a/gcc/cp/reflect.cc b/gcc/cp/reflect.cc index 3b9f56ea5484..f9e3f81f0acd 100644 --- a/gcc/cp/reflect.cc +++ b/gcc/cp/reflect.cc @@ -8561,47 +8561,26 @@ splice (tree refl) return refl; } -/* A walker for consteval_only_p. It cannot be a lambda, because we - have to call this recursively, sigh. */ +/* A cache of the known boolean result of consteval_only_p_walker::walk + for class types. */ -static tree -consteval_only_type_r (tree *tp, int *walk_subtrees, void *data) -{ - tree t = *tp; - /* Types can contain themselves recursively, hence this. */ - auto visited = static_cast<hash_set<tree> *>(data); - - if (!TYPE_P (t)) - return NULL_TREE; +static GTY((cache)) type_tree_cache_map *consteval_only_class_cache; - if (REFLECTION_TYPE_P (t)) - return t; - - if (typedef_variant_p (t)) - /* Tell cp_walk_subtrees to look through typedefs. */ - *walk_subtrees = 2; - - if (RECORD_OR_UNION_TYPE_P (t)) - { - /* Don't walk template arguments; A<info>::type isn't a consteval-only - type. */ - *walk_subtrees = 0; - /* So we have to walk the fields manually. */ - for (tree member = TYPE_FIELDS (t); - member; member = DECL_CHAIN (member)) - if (TREE_CODE (member) == FIELD_DECL) - if (tree r = cp_walk_tree (&TREE_TYPE (member), - consteval_only_type_r, visited, visited)) - return r; - } +struct consteval_only_p_walker +{ + /* The stack of class types we're recursively inside. */ + auto_vec<tree> class_stack; + /* The set of class types we've seen. */ + hash_set<tree> class_seen; + /* True if we've optimistically assumed an already-seen + consteval-unknown class type is not consteval. */ + bool optimistic_p = false; - return NULL_TREE; -} + tristate walk (tree); +}; -/* True if T is a consteval-only type as per [basic.types.general]: - "A type is consteval-only if it is either std::meta::info or a type - compounded from a consteval-only type", or something that has - a consteval-only type. */ +/* True if T is a consteval-only type as per [basic.types.general], or + is a declaration with such a type, or a TREE_VEC thereof. */ bool consteval_only_p (tree t) @@ -8612,7 +8591,7 @@ consteval_only_p (tree t) if (!TYPE_P (t)) t = TREE_TYPE (t); - if (!t) + if (!t || t == error_mark_node) return false; if (TREE_CODE (t) == TREE_VEC) @@ -8634,9 +8613,83 @@ consteval_only_p (tree t) which could be consteval-only, depending on T. */ t = complete_type (t); - /* Classes with std::meta::info members are also consteval-only. */ - hash_set<tree> visited; - return !!cp_walk_tree (&t, consteval_only_type_r, &visited, &visited); + consteval_only_p_walker walker; + return walker.walk (t).is_true (); +} + +/* Recursive workhorse of consteval_only_p. Returns true if T is definitely + consteval-only, false if it's definitely not, and unknown if we saw an + incomplete type and therefore don't know. */ + +tristate +consteval_only_p_walker::walk (tree t) +{ + t = TYPE_MAIN_VARIANT (t); + + if (REFLECTION_TYPE_P (t)) + return true; + else if (INDIRECT_TYPE_P (t)) + return walk (TREE_TYPE (t)); + else if (TREE_CODE (t) == ARRAY_TYPE) + return walk (TREE_TYPE (t)); + else if (FUNC_OR_METHOD_TYPE_P (t)) + { + tristate r = walk (TREE_TYPE (t)); + for (tree parm = TYPE_ARG_TYPES (t); + parm != NULL_TREE && parm != void_list_node; + parm = TREE_CHAIN (parm)) + { + if (r.is_true ()) + break; + r = r || walk (TREE_VALUE (parm)); + } + return r; + } + else if (RECORD_OR_UNION_TYPE_P (t)) + { + if (tree *slot = hash_map_safe_get (consteval_only_class_cache, t)) + return *slot == boolean_true_node; + + if (!COMPLETE_TYPE_P (t) && LAMBDA_TYPE_P (t)) + /* Defer until we've definitely gone through prune_lambda_captures. */ + return tristate::unknown (); + + if (class_seen.add (t)) + { + /* Optimistically assume this already seen consteval-unknown class is + not consteval-only, for sake of mutually recursive classes. */ + optimistic_p = true; + return false; + } + class_stack.safe_push (t); + + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown (); + for (tree member = TYPE_FIELDS (t); member; member = DECL_CHAIN (member)) + if (TREE_CODE (member) == FIELD_DECL) + { + r = r || walk (TREE_TYPE (member)); + if (r.is_true ()) + break; + } + + if (r.is_true ()) + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, + t, boolean_true_node); + else if (r.is_false () + /* The optimistic assumption above is at odds with caching + 'false' results for a nested class type. */ + && (class_stack.length () == 1 || !optimistic_p)) + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, + t, boolean_false_node); + + class_stack.pop (); + return r; + } + else if (TYPE_PTRMEM_P (t)) + return (walk (TYPE_PTRMEM_CLASS_TYPE (t)) + || walk (TYPE_PTRMEM_POINTED_TO_TYPE (t))); + else + return false; } /* A walker for check_out_of_consteval_use_r. It cannot be a lambda, because
On 5/5/26 5:48 PM, Patrick Palka wrote: > On Tue, 5 May 2026, Jason Merrill wrote: > >> On 5/5/26 5:24 PM, Patrick Palka wrote: >>> On Tue, 5 May 2026, Jason Merrill wrote: >>> >>>> On 5/5/26 4:34 PM, Patrick Palka wrote: >>>>> On Tue, 5 May 2026, Jason Merrill wrote: >>>>> >>>>>> On 5/5/26 3:53 PM, Patrick Palka wrote: >>>>>>> On Tue, 5 May 2026, Jason Merrill wrote: >>>>>>>> On 5/5/26 1:20 PM, Patrick Palka wrote: >>>>>>>>> +/* Recursive workhorse of consteval_only_p. Returns true if T >>>>>>>>> is >>>>>>>>> definitely >>>>>>>>> + consteval-only, false if it's definitely not, and unknown if >>>>>>>>> we >>>>>>>>> saw >>>>>>>>> an >>>>>>>>> + incomplete type and therefore don't know. */ >>>>>>>>> + >>>>>>>>> +tristate >>>>>>>>> +consteval_only_p_walker::walk (tree t) >>>>>>>>> +{ >>>>>>>>> + t = TYPE_MAIN_VARIANT (t); >>>>>>>>> + >>>>>>>>> + if (REFLECTION_TYPE_P (t)) >>>>>>>>> + return true; >>>>>>>>> + else if (INDIRECT_TYPE_P (t)) >>>>>>>>> + return walk (TREE_TYPE (t)); >>>>>>>>> + else if (TREE_CODE (t) == ARRAY_TYPE) >>>>>>>>> + return walk (TREE_TYPE (t)); >>>>>>>>> + else if (FUNC_OR_METHOD_TYPE_P (t)) >>>>>>>>> + { >>>>>>>>> + tristate r = walk (TREE_TYPE (t)); >>>>>>>>> + for (tree parm = TYPE_ARG_TYPES (t); >>>>>>>>> + parm != NULL_TREE && parm != void_list_node; >>>>>>>>> + parm = TREE_CHAIN (parm)) >>>>>>>>> + { >>>>>>>>> + if (r.is_true ()) >>>>>>>>> + break; >>>>>>>>> + r = r || walk (TREE_VALUE (parm)); >>>>>>>>> + } >>>>>>>>> + return r; >>>>>>>>> + } >>>>>>>>> + else if (RECORD_OR_UNION_TYPE_P (t)) >>>>>>>>> + { >>>>>>>>> + if (tree *slot = hash_map_safe_get >>>>>>>>> (consteval_only_class_cache, >>>>>>>>> t)) >>>>>>>>> + return *slot == boolean_true_node; >>>>>>>>> + >>>>>>>>> + if (class_seen.add (t)) >>>>>>>>> + { >>>>>>>>> + /* Optimistically assume this already seen consteval-unknown >>>>>>>>> class >>>>>>>>> is >>>>>>>>> + not consteval only, for sake of mutually recursive >>>>>>>>> classes. */ >>>>>>>>> + optimistic_p = true; >>>>>>>>> + return false; >>>>>>>>> + } >>>>>>>>> + class_stack.safe_push (t); >>>>>>>>> + >>>>>>>>> + tristate r = COMPLETE_TYPE_P (t) ? false : >>>>>>>>> tristate::unknown >>>>>>>>> (); >>>>>>>>> + for (tree member = TYPE_FIELDS (t); member; member = >>>>>>>>> DECL_CHAIN >>>>>>>>> (member)) >>>>>>>>> + if (TREE_CODE (member) == FIELD_DECL) >>>>>>>>> + { >>>>>>>>> + r = r || walk (TREE_TYPE (member)); >>>>>>>>> + if (r.is_true ()) >>>>>>>>> + break; >>>>>>>>> + } >>>>>>>>> + >>>>>>>>> + if (!COMPLETE_TYPE_P (t)) >>>>>>>>> + /* Until the type is laid out, we can't trust that its >>>>>>>>> TYPE_FIELDS >>>>>>>>> + won't change. */; >>>>>>>> >>>>>>>> We might move the COMPLETE_TYPE check... >>>>>>>> >>>>>>>>> + else if (r.is_true ()) >>>>>>>>> + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, >>>>>>>>> + t, boolean_true_node); >>>>>>>>> + else if (r.is_false () >>>>>>>> >>>>>>>> ...into another exception for the is_false case, since I don't >>>>>>>> think >>>>>>>> the >>>>>>>> TYPE_FIELDS can change in a way that would invalidate is_true(). >>>>>>> >>>>>>> Sadly, it can! prune_lambda_captures can remove consteval-only >>>>>>> TYPE_FIELDS (corresponding to folded-away captures) before the >>>>>>> closure >>>>>>> type has been laid out, so if we'd cache the result now we'd get the >>>>>>> wrong answer after pruning. >>>>>>> >>>>>>> Without this second COMPLETE_TYPE_P check we'd get a bogus error in >>>>>>> reflect/reflect_constant_array6.C due to treating the lambda as >>>>>>> consteval even after pruning. >>>>>> >>>>>> Ah, lambdas, always keeping us on our toes. But then we could skip >>>>>> walking >>>>>> the fields at all if we're going to return unknown no matter what we >>>>>> see. >>>>> >>>>> In that lambda case, where TYPE_FIELDS is set but TYPE_COMPLETE_P is >>>>> false, we won't always return unknown, we could return true if one of >>>>> its TYPE_FIELDS is consteval-only. >>>> >>>> Hmm, we currently return true but don't cache it? So we could be >>>> returning >>>> consteval-only about a type that later turns out not to be. That doesn't >>>> seem >>>> better to me than just returning unknown. >>> >>> Always returning unknown for !COMPLETE_TYPE_P class types seems to break >>> <meta>: >>> >>> /scratchpad/gcc-master-build/x86_64-pc-linux-gnu/libstdc++-v3/include/meta:117:7: >>> error: ‘consteval’ function ‘virtual consteval const char* >>> std::meta::exception::what() const’ overriding non-‘consteval’ function >>> 117 | what() const noexcept override >>> | ^~~~ >>> In file included from >>> /home/patrick/code/gcc-master/libstdc++-v3/libsupc++/new:43, >>> from >>> /scratchpad/gcc-master-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_construct.h:59, >>> from >>> /scratchpad/gcc-master-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/char_traits.h:59, >>> from >>> /scratchpad/gcc-master-build/x86_64-pc-linux-gnu/libstdc++-v3/include/string:45, >>> from >>> gcc/testsuite/g++.dg/reflect/reflect_constant_array6.C:5: >>> /scratchpad/gcc-master-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/exception.h:83:5: >>> note: overridden function is ‘virtual constexpr const char* >>> std::exception::what() const’ >>> 83 | what() const _GLIBCXX_TXN_SAFE_DYN noexcept { return >>> "std::exception"; } >>> | ^~~~ >>> >>> Reduced: >>> >>> struct exception { >>> virtual void foo(); >>> }; >>> >>> struct meta_exception : exception { >>> decltype(^^int) *p; >>> consteval virtual void foo() { } >>> }; >>> >>> because check_for_override is called before the class is fully laid out >>> and COMPLETE_TYPE_P is not yet set, but we still need consteval_only_p >>> to return true at this point. That doesn't seem ideal :/ >> >> How about always returning unknown for closures, but returning/caching true >> for other classes? > > Makes sense, like so? Passes dg.exp=*reflect* so far. OK. > -- >8 -- > > PR c++/125179 > > gcc/cp/ChangeLog: > > * reflect.cc: (consteval_only_type_r): Remove this cp_walk_tree > callback and replace with ... > (consteval_only_walker): ... this recursive memoized > implementation. > (consteval_only_p): Define in terms of consteval_only_walker. > --- > gcc/cp/reflect.cc | 135 ++++++++++++++++++++++++++++++++-------------- > 1 file changed, 94 insertions(+), 41 deletions(-) > > diff --git a/gcc/cp/reflect.cc b/gcc/cp/reflect.cc > index 3b9f56ea5484..f9e3f81f0acd 100644 > --- a/gcc/cp/reflect.cc > +++ b/gcc/cp/reflect.cc > @@ -8561,47 +8561,26 @@ splice (tree refl) > return refl; > } > > -/* A walker for consteval_only_p. It cannot be a lambda, because we > - have to call this recursively, sigh. */ > +/* A cache of the known boolean result of consteval_only_p_walker::walk > + for class types. */ > > -static tree > -consteval_only_type_r (tree *tp, int *walk_subtrees, void *data) > -{ > - tree t = *tp; > - /* Types can contain themselves recursively, hence this. */ > - auto visited = static_cast<hash_set<tree> *>(data); > - > - if (!TYPE_P (t)) > - return NULL_TREE; > +static GTY((cache)) type_tree_cache_map *consteval_only_class_cache; > > - if (REFLECTION_TYPE_P (t)) > - return t; > - > - if (typedef_variant_p (t)) > - /* Tell cp_walk_subtrees to look through typedefs. */ > - *walk_subtrees = 2; > - > - if (RECORD_OR_UNION_TYPE_P (t)) > - { > - /* Don't walk template arguments; A<info>::type isn't a consteval-only > - type. */ > - *walk_subtrees = 0; > - /* So we have to walk the fields manually. */ > - for (tree member = TYPE_FIELDS (t); > - member; member = DECL_CHAIN (member)) > - if (TREE_CODE (member) == FIELD_DECL) > - if (tree r = cp_walk_tree (&TREE_TYPE (member), > - consteval_only_type_r, visited, visited)) > - return r; > - } > +struct consteval_only_p_walker > +{ > + /* The stack of class types we're recursively inside. */ > + auto_vec<tree> class_stack; > + /* The set of class types we've seen. */ > + hash_set<tree> class_seen; > + /* True if we've optimistically assumed an already-seen > + consteval-unknown class type is not consteval. */ > + bool optimistic_p = false; > > - return NULL_TREE; > -} > + tristate walk (tree); > +}; > > -/* True if T is a consteval-only type as per [basic.types.general]: > - "A type is consteval-only if it is either std::meta::info or a type > - compounded from a consteval-only type", or something that has > - a consteval-only type. */ > +/* True if T is a consteval-only type as per [basic.types.general], or > + is a declaration with such a type, or a TREE_VEC thereof. */ > > bool > consteval_only_p (tree t) > @@ -8612,7 +8591,7 @@ consteval_only_p (tree t) > if (!TYPE_P (t)) > t = TREE_TYPE (t); > > - if (!t) > + if (!t || t == error_mark_node) > return false; > > if (TREE_CODE (t) == TREE_VEC) > @@ -8634,9 +8613,83 @@ consteval_only_p (tree t) > which could be consteval-only, depending on T. */ > t = complete_type (t); > > - /* Classes with std::meta::info members are also consteval-only. */ > - hash_set<tree> visited; > - return !!cp_walk_tree (&t, consteval_only_type_r, &visited, &visited); > + consteval_only_p_walker walker; > + return walker.walk (t).is_true (); > +} > + > +/* Recursive workhorse of consteval_only_p. Returns true if T is definitely > + consteval-only, false if it's definitely not, and unknown if we saw an > + incomplete type and therefore don't know. */ > + > +tristate > +consteval_only_p_walker::walk (tree t) > +{ > + t = TYPE_MAIN_VARIANT (t); > + > + if (REFLECTION_TYPE_P (t)) > + return true; > + else if (INDIRECT_TYPE_P (t)) > + return walk (TREE_TYPE (t)); > + else if (TREE_CODE (t) == ARRAY_TYPE) > + return walk (TREE_TYPE (t)); > + else if (FUNC_OR_METHOD_TYPE_P (t)) > + { > + tristate r = walk (TREE_TYPE (t)); > + for (tree parm = TYPE_ARG_TYPES (t); > + parm != NULL_TREE && parm != void_list_node; > + parm = TREE_CHAIN (parm)) > + { > + if (r.is_true ()) > + break; > + r = r || walk (TREE_VALUE (parm)); > + } > + return r; > + } > + else if (RECORD_OR_UNION_TYPE_P (t)) > + { > + if (tree *slot = hash_map_safe_get (consteval_only_class_cache, t)) > + return *slot == boolean_true_node; > + > + if (!COMPLETE_TYPE_P (t) && LAMBDA_TYPE_P (t)) > + /* Defer until we've definitely gone through prune_lambda_captures. */ > + return tristate::unknown (); > + > + if (class_seen.add (t)) > + { > + /* Optimistically assume this already seen consteval-unknown class is > + not consteval-only, for sake of mutually recursive classes. */ > + optimistic_p = true; > + return false; > + } > + class_stack.safe_push (t); > + > + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown (); > + for (tree member = TYPE_FIELDS (t); member; member = DECL_CHAIN (member)) > + if (TREE_CODE (member) == FIELD_DECL) > + { > + r = r || walk (TREE_TYPE (member)); > + if (r.is_true ()) > + break; > + } > + > + if (r.is_true ()) > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > + t, boolean_true_node); > + else if (r.is_false () > + /* The optimistic assumption above is at odds with caching > + 'false' results for a nested class type. */ > + && (class_stack.length () == 1 || !optimistic_p)) > + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, > + t, boolean_false_node); > + > + class_stack.pop (); > + return r; > + } > + else if (TYPE_PTRMEM_P (t)) > + return (walk (TYPE_PTRMEM_CLASS_TYPE (t)) > + || walk (TYPE_PTRMEM_POINTED_TO_TYPE (t))); > + else > + return false; > } > > /* A walker for check_out_of_consteval_use_r. It cannot be a lambda, because
On Tue, May 05, 2026 at 05:48:15PM -0400, Patrick Palka wrote: > +struct consteval_only_p_walker > +{ > + /* The stack of class types we're recursively inside. */ > + auto_vec<tree> class_stack; Why is this auto_vec<tree> rather than just int? I mean, it is used as: > + class_stack.safe_push (t); and > + && (class_stack.length () == 1 || !optimistic_p)) and > + class_stack.pop (); and nowhere else. So, if it is int and the first one does ++class_stack_cnt; the second && (class_stack_cnt == 1 || !optimistic_p) and the last one --class_stack_cnt; I think it should behave identically except for (tiny bit) smaller compile time memory + compile time due to no need to allocate/free it. Jakub
On Tue, 5 May 2026, Jakub Jelinek wrote: > On Tue, May 05, 2026 at 05:48:15PM -0400, Patrick Palka wrote: > > +struct consteval_only_p_walker > > +{ > > + /* The stack of class types we're recursively inside. */ > > + auto_vec<tree> class_stack; > > Why is this auto_vec<tree> rather than just int? > I mean, it is used as: > > > + class_stack.safe_push (t); > > and > > > + && (class_stack.length () == 1 || !optimistic_p)) > > and > > > + class_stack.pop (); > > and nowhere else. So, if it is int and the first one does > ++class_stack_cnt; > the second > && (class_stack_cnt == 1 || !optimistic_p) > and the last one > --class_stack_cnt; > I think it should behave identically except for (tiny bit) > smaller compile time memory + compile time due to no need to allocate/free > it. It's a vestige of one of the earlier overly clever versions and indeed not really necessary. Will replace with an int before pushing.
diff --git a/gcc/cp/reflect.cc b/gcc/cp/reflect.cc index b36c4be42736..10b5ed2b109f 100644 --- a/gcc/cp/reflect.cc +++ b/gcc/cp/reflect.cc @@ -8561,47 +8561,10 @@ splice (tree refl) return refl; } -/* A walker for consteval_only_p. It cannot be a lambda, because we - have to call this recursively, sigh. */ +static tristate consteval_only_p_1 (tree, hash_set<tree>&); -static tree -consteval_only_type_r (tree *tp, int *walk_subtrees, void *data) -{ - tree t = *tp; - /* Types can contain themselves recursively, hence this. */ - auto visited = static_cast<hash_set<tree> *>(data); - - if (!TYPE_P (t)) - return NULL_TREE; - - if (REFLECTION_TYPE_P (t)) - return t; - - if (typedef_variant_p (t)) - /* Tell cp_walk_subtrees to look through typedefs. */ - *walk_subtrees = 2; - - if (RECORD_OR_UNION_TYPE_P (t)) - { - /* Don't walk template arguments; A<info>::type isn't a consteval-only - type. */ - *walk_subtrees = 0; - /* So we have to walk the fields manually. */ - for (tree member = TYPE_FIELDS (t); - member; member = DECL_CHAIN (member)) - if (TREE_CODE (member) == FIELD_DECL) - if (tree r = cp_walk_tree (&TREE_TYPE (member), - consteval_only_type_r, visited, visited)) - return r; - } - - return NULL_TREE; -} - -/* True if T is a consteval-only type as per [basic.types.general]: - "A type is consteval-only if it is either std::meta::info or a type - compounded from a consteval-only type", or something that has - a consteval-only type. */ +/* True if T is a consteval-only type as per [basic.types.general], or + is a declaration with such a type, or a TREE_VEC thereof. */ bool consteval_only_p (tree t) @@ -8612,7 +8575,7 @@ consteval_only_p (tree t) if (!TYPE_P (t)) t = TREE_TYPE (t); - if (!t) + if (!t || t == error_mark_node) return false; if (TREE_CODE (t) == TREE_VEC) @@ -8634,9 +8597,80 @@ consteval_only_p (tree t) which could be consteval-only, depending on T. */ t = complete_type (t); - /* Classes with std::meta::info members are also consteval-only. */ - hash_set<tree> visited; - return !!cp_walk_tree (&t, consteval_only_type_r, &visited, &visited); + hash_set<tree> class_set; + return consteval_only_p_1 (t, class_set).is_true (); +} + +/* A cache of the boolean result of consteval_only_p_1 for class types, when + the result is known. */ + +static GTY((cache)) type_tree_cache_map *consteval_only_class_cache; + +/* Recursive workhorse of consteval_only_p. Returns true if T is definitely + consteval-only, false if it's definitely not, and unknown if we saw an + incomplete type and therefore don't know. CLASS_SET is the set of class types + we're recursively inside. */ + +static tristate +consteval_only_p_1 (tree t, hash_set<tree>& class_set) +{ + t = TYPE_MAIN_VARIANT (t); + + if (REFLECTION_TYPE_P (t)) + return true; + else if (INDIRECT_TYPE_P (t)) + return consteval_only_p_1 (TREE_TYPE (t), class_set); + else if (TREE_CODE (t) == ARRAY_TYPE) + return consteval_only_p_1 (TREE_TYPE (t), class_set); + else if (FUNC_OR_METHOD_TYPE_P (t)) + { + tristate r = consteval_only_p_1 (TREE_TYPE (t), class_set); + for (tree parm = TYPE_ARG_TYPES (t); + parm != NULL_TREE && parm != void_list_node; + parm = TREE_CHAIN (parm)) + { + r = r || consteval_only_p_1 (TREE_VALUE (parm), class_set); + if (r.is_true ()) + break; + } + return r; + } + else if (RECORD_OR_UNION_TYPE_P (t)) + { + if (tree *slot = hash_map_safe_get (consteval_only_class_cache, t)) + return *slot == boolean_true_node; + + if (class_set.add (t)) + /* Handle struct A { A* p; } etc. */ + return false; + + tristate r = COMPLETE_TYPE_P (t) ? false : tristate::unknown (); + for (tree member = TYPE_FIELDS (t); member; member = DECL_CHAIN (member)) + if (TREE_CODE (member) == FIELD_DECL) + { + r = r || consteval_only_p_1 (TREE_TYPE (member), class_set); + if (r.is_true ()) + break; + } + + if (COMPLETE_TYPE_P (t)) + { + if (r.is_true ()) + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, + t, boolean_true_node); + else if (r.is_false ()) + hash_map_safe_put<hm_ggc> (consteval_only_class_cache, + t, boolean_false_node); + } + + class_set.remove (t); + return r; + } + else if (TYPE_PTRMEM_P (t)) + return (consteval_only_p_1 (TYPE_PTRMEM_CLASS_TYPE (t), class_set) + || consteval_only_p_1 (TYPE_PTRMEM_POINTED_TO_TYPE (t), class_set)); + else + return false; } /* A walker for check_out_of_consteval_use_r. It cannot be a lambda, because