On Tue, Dec 09, 2025 at 08:38:26PM +0800, Jason Merrill wrote: > On 12/6/25 4:19 AM, Marek Polacek wrote: > > On Fri, Nov 28, 2025 at 08:25:05AM +0530, Jason Merrill wrote: > > > On 11/15/25 6:04 AM, Marek Polacek wrote: > > [snipping to just active comments] > > > > > +/* Helper macro to set SPLICE_EXPR_EXPRESSION_P. */ > > > > +#define SET_SPLICE_EXPR_EXPRESSION_P(NODE) \ > > > > + (SPLICE_EXPR_EXPRESSION_P (TREE_CODE (NODE) == SPLICE_EXPR \ > > > > + ? NODE : TREE_OPERAND (NODE, 0)) = true) > > > > > > What are these helpers for? What is the node that we expect to have a > > > splice in op0? > > > > These are to handle dependent_splice_p trees: either [:T:] or [:T:]<arg>, > > and I wanted one macro for both. The second one is a TEMPLATE_ID_EXPR. > > OK, please mention dependent_splice_p in the comment.
Done: <https://forge.sourceware.org/marek/gcc/commit/7528282b85f7716aa9bd0db1c59c8d9c15f00d03> > > > > @@ -5514,6 +5582,15 @@ cxx_init_decl_processing (void) > > > > /* Create the `std' namespace. */ > > > > push_namespace (get_identifier ("std")); > > > > std_node = current_namespace; > > > > + if (flag_reflection) > > > > + { > > > > + /* Note that we haven't initialized void_type_node yet, so > > > > + std_meta_node will be initially typeless; its type will be > > > > + set a little later in init_reflection. */ > > > > + push_namespace (get_identifier ("meta"), /*inline*/false); > > > > + std_meta_node = current_namespace; > > > > + pop_namespace (); > > > > + } > > > > > > ??? std_meta_node is a namespace, which are always typeless. > > > > This was an ICE in is_std_substitution on: > > > > if (!(TYPE_LANG_SPECIFIC (type) && TYPE_TEMPLATE_INFO (type))) > > > > where decl=namespace_decl meta and type was null due to the ordering > > issue mentioned in the comment. > > > > make_namespace uses void_type_node to build a namespace_decl, so all > > other namespaces have a non-null type. > > Why doesn't std_node have the same issue? Because is_std_substitution has this early exit: if (!DECL_NAMESPACE_STD_P (CP_DECL_CONTEXT (decl))) return false; > Incidentally, why do we need to initialize std_meta_node here rather than in > init_reflection? It seemed convenient since we just pushed into std:: which is where meta should reside. > > > > @@ -9808,6 +9918,20 @@ cp_finish_decl (tree decl, tree init, bool > > > > init_const_expr_p, > > > > record_types_used_by_current_var_decl (decl); > > > > } > > > > + /* CWG 3115: Every function of consteval-only type shall be an > > > > + immediate function. */ > > > > + if (TREE_CODE (decl) == FUNCTION_DECL > > > > + && !DECL_IMMEDIATE_FUNCTION_P (decl) > > > > + && consteval_only_p (decl) > > > > + /* But if the function can be escalated, merrily we roll along. > > > > */ > > > > + && !immediate_escalating_function_p (decl)) > > > > + { > > > > + error_at (DECL_SOURCE_LOCATION (decl), > > > > + "function of consteval-only type must be declared %qs", > > > > + "consteval"); > > > > + return; > > > > + } > > > > @@ -20471,6 +20641,16 @@ finish_function (bool inline_p) > > > > unused_but_set_errorcount = errorcount; > > > > } > > > > + /* CWG 3115: Every function of consteval-only type shall be an > > > > immediate > > > > + function. */ > > > > + if (!DECL_IMMEDIATE_FUNCTION_P (fndecl) > > > > + && consteval_only_p (fndecl) > > > > + && !immediate_escalating_function_p (fndecl) > > > > + && !is_std_allocator_allocate (fndecl)) > > > > + error_at (DECL_SOURCE_LOCATION (fndecl), > > > > + "function of consteval-only type must be declared %qs", > > > > + "consteval"); > > > > > > Do we need this in both places? > > > > I think so, one is for fn decls, the other one for fn defs. > > But every function definition is also a declaration. And this condition > seems like something you can check before finish_function. > > Maybe in grokfndecl? Though that won't help with instantiations, so perhaps > you need a separate check function to call from there and > tsubst_function_decl? Reworked in <https://forge.sourceware.org/marek/gcc/commit/21332410abe133b726172248e44825e48bfd4b00> I couldn't call the new function in tsubst_function_decl though -- it caused too many errors on code such as: indirect_result_t<_Proj&, _Iter> operator*() const; // not defined in bits/iterator_concepts.h, or _UninitDestroyGuard(const _UninitDestroyGuard&); in bits/stl_uninitialized.h. But it worked to call it in instantiate_body -- that is, when we instantiate a fn def. I had to add consteval to 4 std::meta::exception member fns. (I think this needs an LWG issue.) > > > > + c = BINFO_INHERITANCE_CHAIN (c); > > > > + tpvis = type_visibility (BINFO_TYPE (c)); > > > > + *walk_subtrees = 0; > > > > + break; > > > > + case REFLECT_DATA_MEMBER_SPEC: > > > > + /* For data member description determine visibility > > > > + from the type. */ > > > > + tpvis = type_visibility (TREE_VEC_ELT (r, 0)); > > > > + *walk_subtrees = 0; > > > > + break; > > > > + case REFLECT_PARM: > > > > + /* For function parameter reflection determine visibility > > > > + based on parent_of. */ > > > > + tpvis = expr_visibility (DECL_CONTEXT (r)); > > > > + *walk_subtrees = 0; > > > > + break; > > > > + case REFLECT_OBJECT: > > > > + c = get_base_address (r); > > > > + if ((VAR_P (c) && decl_function_context (c)) > > > > + || TREE_CODE (c) == PARM_DECL) > > > > + { > > > > + /* Make reflect_object of block scope variables > > > > + subobjects local. */ > > > > + tpvis = VISIBILITY_ANON; > > > > + *walk_subtrees = 0; > > > > + } > > > > > > We just stopped treating local declarations as VISIBILITY_ANON in > > > r16-5024-gf062a6b7985fce -- for this to differ from that, we need more > > > rationale. > > > > E.g. in > > > > template <int N, decltype(^^::) I> > > void > > foo () > > { > > } > > > > static void > > bar () > > { > > int v = 42; > > foo <101, ^^v> (); // #1 > > } > > > > #1 shouldn't be exported. If I follow r16-5024, decl_linkage will > > give me lk_none for 'v', as it should (it's a var at block scope), but > > type_visibility of its type says VISIBILITY_DEFAULT > > v is TU-local under https://eel.is/c++draft/basic.link#15.1.2 because it's > defined within TU-local bar(). > > So perhaps r16-5024 went too far one way, and this goes too far the other > way: for a decl with no linkage, we need to refer to the linkage of the > containing entity that does have a name with linkage. So like this? I haven't pushed this because I'm not sure that this is what we want, although it seems OK to me -- it changes places with a TODO in the testcase. (I think this could be checked in post-merge.) diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc index 1e717b3801f..07ee5568efd 100644 --- a/gcc/cp/decl2.cc +++ b/gcc/cp/decl2.cc @@ -2965,8 +2965,13 @@ min_vis_expr_r (tree *tp, int *walk_subtrees, void *data) if ((VAR_P (r) && decl_function_context (r)) || TREE_CODE (r) == PARM_DECL) { - /* Block scope variables are local to the TU. */ - tpvis = VISIBILITY_ANON; + /* For _DECLs with no linkage refer to the linkage of the + containing entity that does have a name with linkage. + ??? The r16-5024 change should perhaps follow this logic. */ + if (decl_linkage (r) == lk_none) + tpvis = expr_visibility (DECL_CONTEXT (r)); + else + tpvis = VISIBILITY_ANON; *walk_subtrees = 0; break; } diff --git a/gcc/testsuite/g++.dg/reflect/visibility1.C b/gcc/testsuite/g++.dg/reflect/visibility1.C index c47817b9b4e..331e8353e4a 100644 --- a/gcc/testsuite/g++.dg/reflect/visibility1.C +++ b/gcc/testsuite/g++.dg/reflect/visibility1.C @@ -49,8 +49,8 @@ inline void qux (int x) { int v = 42; - foo <106, ^^x> (); // { dg-final { scan-assembler-not "\t.weak\t_Z3fooILi106EL" } } - var in inline fn - TODO, shall this be exported? - foo <107, ^^v> (); // { dg-final { scan-assembler-not "\t.weak\t_Z3fooILi107EL" } } - var in inline fn - TODO, shall this be exported? + foo <106, ^^x> (); // { dg-final { scan-assembler "\t.weak\t_Z3fooILi106EL" } } - var in inline fn + foo <107, ^^v> (); // { dg-final { scan-assembler "\t.weak\t_Z3fooILi107EL" } } - var in inline fn } template <int N> @@ -58,8 +58,8 @@ void plugh (int x) { int v = 42; - foo <132, ^^x> (); // { dg-final { scan-assembler-not "\t.weak\t_Z3fooILi132EL" } } - var in public fn template instantiation - TODO, shall this be exported? - foo <133, ^^v> (); // { dg-final { scan-assembler-not "\t.weak\t_Z3fooILi133EL" } } - var in public fn template instantiation - TODO, shall this be exported? + foo <132, ^^x> (); // { dg-final { scan-assembler "\t.weak\t_Z3fooILi132EL" } } - var in public fn template instantiation + foo <133, ^^v> (); // { dg-final { scan-assembler "\t.weak\t_Z3fooILi133EL" } } - var in public fn template instantiation foo <134, parameters_of (parent_of (^^v))[0]> (); // { dg-final { scan-assembler "\t.weak\t_Z3fooILi134EL" { target *-*-linux* } } } - fn parm of public fn template instantiation } @@ -80,8 +80,8 @@ void fred (int x) { int v = 42; - foo <138, ^^x> (); // { dg-final { scan-assembler-not "\t.weak\t_Z3fooILi138EL" } } - var in TU-local fn template instantiation - foo <139, ^^v> (); // { dg-final { scan-assembler-not "\t.weak\t_Z3fooILi139EL" } } - var in Tu-local fn template instantiation + foo <138, ^^x> (); // { dg-final { scan-assembler "\t.weak\t_Z3fooILi138EL" } } - var in TU-local fn template instantiation + foo <139, ^^v> (); // { dg-final { scan-assembler "\t.weak\t_Z3fooILi139EL" } } - var in Tu-local fn template instantiation foo <140, parameters_of (parent_of (^^v))[0]> (); // { dg-final { scan-assembler-not "\t.weak\t_Z3fooILi140EL" { xfail *-*-* } } } - fn parm of TU-local fn template instantiation - TODO, I think this shouldn't be exported and the mangling of these 3 doesn't include the template parameter } @@ -108,8 +108,8 @@ xyzzy (int x) foo <122, ^^G <int>> (); // { dg-final { scan-assembler-not "\t.weak\t_Z3fooILi122EL" } } - specialization of TU-local template foo <123, ^^G <A>> (); // { dg-final { scan-assembler-not "\t.weak\t_Z3fooILi123EL" } } - specialization of TU-local template foo <124, ^^G <B>> (); // { dg-final { scan-assembler-not "\t.weak\t_Z3fooILi124EL" } } - specialization of TU-local template - foo <125, ^^x> (); // { dg-final { scan-assembler-not "\t.weak\t_Z3fooILi125EL" } } - var in public fn but non-comdat - TODO, shall this be exported? - foo <126, ^^v> (); // { dg-final { scan-assembler-not "\t.weak\t_Z3fooILi126EL" } } - var in public fn but non-comdat - TODO, shall this be exported? + foo <125, ^^x> (); // { dg-final { scan-assembler "\t.weak\t_Z3fooILi125EL" } } - var in public fn but non-comdat + foo <126, ^^v> (); // { dg-final { scan-assembler "\t.weak\t_Z3fooILi126EL" } } - var in public fn but non-comdat foo <127, std::meta::info {}> (); // { dg-final { scan-assembler "\t.weak\t_Z3fooILi127EL" { target *-*-linux* } } } - null reflection foo <128, ^^b> (); // { dg-final { scan-assembler "\t.weak\t_Z3fooILi128EL" { target *-*-linux* } } } - public variable foo <129, ^^c> (); // { dg-final { scan-assembler-not "\t.weak\t_Z3fooILi129EL" } } - TU-local variable > > > > + auto s = make_temp_override (cp_preserve_using_decl, true); > > > > > > This flag seems kind of a kludge; since we only need it to apply to a > > > single > > > lookup, I wonder how difficult it would be to use a LOOK_want flag > > > instead? > > > > I think not easy/possible, because strip_using_decls is called a lot > > in different contexts, I don't think it can always access the lookup flags > > (?). > > But it seems to me we only want to keep the USING_DECL for the single > cp_parser_lookup_name in cp_parser_reflection_name, not for any other name > lookup. > > This can be addressed in a followup. Ack. > > > > + /* This can happen for std::meta::info(^^int) where the cast > > > > has no > > > > + meaning. */ > > > > + if (REFLECTION_TYPE_P (type) && REFLECT_EXPR_P (op)) > > > > + { > > > > + r = op; > > > > + break; > > > > + } > > > > > > Why is this needed? I'd expect the existing code to work fine for a cast > > > to > > > the same type. > > > > This is to handle > > > > using info = decltype(^^int); > > void > > g () > > { > > constexpr auto r = info(^^int); > > } > > > > An analogical test would be: > > > > using T = int; > > void > > g () > > { > > constexpr auto i = T(42); > > } > > > > which is handled by > > > > if (same_type_ignoring_tlq_and_bounds_p (type, TREE_TYPE (op))) > > r = op; > > > > later in that function. But with info(^^int) we don't get that far > > because we do: > > > > if (op == oldop && tcode != UNARY_PLUS_EXPR) > > /* We didn't fold at the top so we could check for ptr-int > > conversion. */ > > return fold (t); > > > > because op and oldop are the same REFLECT_EXPR, whereas with T(42) > > op == 42 and oldop == NON_LVALUE_EXPR <42>. > > But why is taking that return a problem? So t is a NOP_EXPR: (info) ^^int and fold leaves that NOP_EXPR be. Then reduced_constant_expression_p in verify_constant says false for the NOP_EXPR and we error. (I have "case REFLECT_EXPR" in reduced_constant_expression_p.) > > > > +cp_parser_splice_expression (cp_parser *parser, bool template_p, > > > > + bool address_p, bool template_arg_p, > > > > + bool member_access_p, cp_id_kind *idk) > > > > +{ > > > > + bool targs_p = false; > > > > + > > > > + /* [class.access.base]/5: A member m is accessible at the point R > > > > when > > > > + designated in class N if > > > > + -- m is designated by a splice-expression */ > > > > + if (member_access_p) > > > > + push_deferring_access_checks (dk_no_check); > > > > + > > > > + cp_expr expr = cp_parser_splice_specifier (parser, template_p, > > > > &targs_p); > > > > + > > > > + if (member_access_p) > > > > + pop_deferring_access_checks (); > > > > > > I don't understand messing with access around parsing the splice; where is > > > the access check that would be affected? > > > > This is for > > > > struct S { > > int k; > > }; > > class D : S {} d; > > int i = d.[:^^S::k:]; > > Without the push/pop we'd error in cp_parser_splice_specifier -> ... -> > > cp_parser_reflect_expression -> cp_parser_parse_definitely -> > > pop_to_parent_deferring_access_checks -> perform_access_checks -> > > enforce_access and say: > > > > error: 'struct S S::S is inaccessible within this context > > error: 'int S::k' is inaccessible within this context > > This suggests that we're doing the lookup in the context of "d.", which is > wrong. That is, we should reject > > struct A { static int x; }; > int q = A().[:^^x:]; // error, no 'x' in scope > > but the branch currently accepts this. Under > https://eel.is/c++draft/basic.lookup.qual#general-2 'x' is not a > member-qualified name. Fixed, tested in splice3.C. > > > > + { > > > > + /* We may have to instantiate; for instance, if we're dealing > > > > with > > > > + a variable template. For &[: ^^S::x :], we have to create an > > > > + OFFSET_REF. For a VAR_DECL, we need the > > > > convert_from_reference. */ > > > > + cp_unevaluated u; > > > > + /* CWG 3109 adjusted [class.protected] to say that checking > > > > access to > > > > + protected non-static members is disabled for members > > > > designated by a > > > > + splice-expression. */ > > > > + push_deferring_access_checks (dk_no_check); > > > > > > I'm not sure where an access check would happen here, either? > > > finish_id_expression_1 already does this push/pop for the FIELD_DECL case. > > > > This is for: > > > > class Base { > > private: > > int m; > > public: > > static constexpr auto rm = ^^m; > > }; > > class Derived { static constexpr auto mp = &[:Base::rm:]; }; > > > > where in finish_id_expression_1 we don't go to the FIELD_DECL block, > > but to the one above it with finish_qualified_id_expr because scope is > > Base here. > > I don't see how this applies; finish_id_expression_1 would be looking at > Base::rm, which is a public static member. Ah! So cp_parser_splice_expression for [:Base::rm:] here calls cp_parser_splice_specifier which parses Base::rm which produces VCE<info>(rm) but then splice does cxx_constant_value which evaluates it to field_decl m. So then when we reach the finish_id_expression back in cp_parser_splice_expression, we're dealing with m and not rm. It's not clear to me if splice can avoid cxx_constant_value. I don't think so. > > > > + /* ??? It'd be nice to use saved_token_sentinel, but its rollback > > > > + uses cp_lexer_previous_token, but we may be the first token in the > > > > + file so there are no previous tokens. Sigh. */ > > > > + cp_lexer_save_tokens (parser->lexer); > > > > > > That sounds pretty simple to fix? > > > > Maybe. saved_token_sentinel::rollback would have to fix > > > > cp_lexer_set_source_position_from_token > > (cp_lexer_previous_token (lexer)); > > > > when there are no previous tokens, maybe skip the _set_source_position_ > > call. > > Sounds right. But this isn't necessary if you don't feel like pursuing it > now. Ok. I do want to come back to this because cp_parser_splice_spec_is_nns_p then could look a bit neater. > > > > + [:^^B::fn:]() // do not disable virtual dispatch > > > > + [:^^B:]::fn() // disable virtual dispatch > > > > + > > > > + so we check SPLICE_P. */ > > > > + if (parser->scope && !splice_p) > > > > *idk = CP_ID_KIND_QUALIFIED; > > > > > > Hmm, it seems wrong for parser->scope to still be set in the former case. > > > > So this is tested in reflect/member9.C. I thought that d.[:^^B::fn:]() > > should behave just like d.B::fn() which also leaves parser->scope set > > to B. > > I don't think it should, as above the splice is a separate lookup context. > > I agree with the comments and the testcase, but we should be clearing > parser->scope after the splice so we don't need any change here. Now finally done in <https://forge.sourceware.org/marek/gcc/commit/fdddd54b29f464e3985a8c8203f055db0cecc1a4> I think I'll post a new series tomorrow. Thanks, Marek
