On Sun, Nov 30, 2025 at 11:00:03PM +0530, Jason Merrill wrote:
> And some further feedback:
>
> > +cp_parser_reflect_expression (cp_parser *parser)
> > +{
> > + if (!flag_reflection)
> > + {
> > + error_at (cp_lexer_peek_token (parser->lexer)->location,
> > + "reflection is only available with %<-freflection%>");
> > + return error_mark_node;
> > + }
> > +
> > + /* Consume the '^^'. */
> > + cp_lexer_consume_token (parser->lexer);
> > +
> > + /* Get the location of the operand. */
> > + const location_t loc = cp_lexer_peek_token (parser->lexer)->location;
> > +
> > + 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 (?).
> Failing that, setting the flag should at least move into
> cp_parser_reflection_name, it seems dangerous to have it set while parsing
> template arguments.
Done in
<https://forge.sourceware.org/marek/gcc/commit/e1343d05bd2fd857ea14dd90b5f713fc5b6875f2>
> > + /* 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>.
> > @@ -10658,8 +10815,11 @@ cxx_eval_outermost_constant_expr (tree t, bool
> > allow_non_constant,
> > }
> > /* Check that immediate invocation does not return an expression
> > referencing
> > - any immediate function decls. */
> > - if (!non_constant_p && cxx_dialect >= cxx20)
> > + any immediate function decls. But allow
> > + consteval int fn () { return 42; }
> > + constexpr auto r = ^^fn;
> > + which is OK to do. */
> > + if (!non_constant_p && cxx_dialect >= cxx20 && !REFLECT_EXPR_P (r))
> > if (tree immediate_fndecl
> > = cp_walk_tree_without_duplicates (&r, find_immediate_fndecl,
> > NULL))
>
> This change seems redundant with the one in find_immediate_fndecl.
Oops, I guess the one in find_immediate_fndecl came later. Removed in
<https://forge.sourceware.org/marek/gcc/commit/70e2f3248a4d6c4e7790e0416b171f17a72fa4bd>
> > tree ctx = decl_namespace_context (fndecl);
> > if (!DECL_NAMESPACE_STD_META_P (ctx))
> > return false;
>
> This pattern seems to be the only use of DECL_NAMESPACE_STD_META_P, how
> about dropping that macro and instead defining decl_in_std_meta_p?
OK, done in
<https://forge.sourceware.org/marek/gcc/commit/768a6b280e2c92efb06f132f194102f3996e3d6e>
> > +/* Return true iff the next tokens start a splice-type-specifier.
> > + If REQUIRE_TYPENAME_P, we only return true if there is a preceding
> > + typename keyword. */
> > +
> > +static bool
> > +cp_parser_next_tokens_start_splice_type_spec_p (cp_parser *parser,
> > + bool require_typename_p)
> > +{
> > + if (cp_lexer_next_token_is (parser->lexer, CPP_OPEN_SPLICE))
> > + return !require_typename_p;
> > + return (cp_lexer_next_token_is_keyword (parser->lexer, RID_TYPENAME)
> > + && cp_lexer_nth_token_is (parser->lexer, 2, CPP_OPEN_SPLICE));
> > +}
>
> Doesn't this need to handle the optional typename template [: case?
I think this is fine. "typename template [:" and combination thereof is
tested in p2996-11.C, but there it's in a splice-scope-specifier.
> > +/* Return true iff the next tokens start a splice-scope-specifier. */
> > +
> > +static bool
> > +cp_parser_next_tokens_start_splice_scope_spec_p (cp_parser *parser)
> > +{
> > + if (cp_lexer_next_token_is (parser->lexer, CPP_OPEN_SPLICE))
> > + return true;
> > + return (cp_lexer_next_token_is_keyword (parser->lexer, RID_TEMPLATE)
> > + && cp_lexer_nth_token_is (parser->lexer, 2, CPP_OPEN_SPLICE));
> > +}
>
> Since these tokens could also start a splice-expression, maybe this should
> be called "...can_start..."?
Renamed in
<https://forge.sourceware.org/marek/gcc/commit/7bc96701c2efa3b39a3d4535301331cc33e275af>
> > +static tree
> > +cp_parser_splice_type_specifier (cp_parser *parser)
> > +{
> > + /* Consume the optional typename. */
> > + if (cp_lexer_next_token_is_keyword (parser->lexer, RID_TYPENAME))
> > + cp_lexer_consume_token (parser->lexer);
> > +
> > + tree type = cp_parser_splice_specifier (parser);
> > +
> > + if (TREE_CODE (type) == TYPE_DECL)
> > + type = TREE_TYPE (type);
> > +
> > + /* When we see [:T:] or [:T:]<arg> we don't know what it'll turn out
> > + to be. */
> > + if (dependent_splice_p (type))
> > + return make_splice_scope (type, /*type_p=*/true);
> > +
> > + if (!valid_splice_type_p (type))
> > + {
> > + cp_parser_error (parser, "reflection not usable in a splice type");
> > + type = NULL_TREE;
> > + }
>
> It seems unfortunate not to print what the problematic reflection is here;
> maybe we can use a regular error (if !simulate) instead of cp_parser_error?
Done. I also found out that this error is never exercised because mostly
we are parsing that splice-type-spec tentatively. But I can just do
void f(int, typename [: ^^:: :] i) {}
which now prints
error: reflection '::' not usable in a splice type
Added error10.C in
<https://forge.sourceware.org/marek/gcc/commit/dbb59d54054671e3e292a93786115f283493662e>
(tsubst_splice_scope already prints the expression.)
> > +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
The checks were added when looking up the names via
check_accessibility_of_qualified_id when parsing the reflection name.
> > + if (template_p)
> > + {
> > + /* [expr.prim.splice] For a splice-expression of the form template
> > + splice-specifier, the splice-specifier shall designate a function
> > + template. */
> > + if (!targs_p)
> > + {
> > + if (!really_overloaded_fn (t))
> > + {
> > + auto_diagnostic_group d;
> > + error_at (loc, "reflection not usable in a template splice");
> > + inform (loc, "only function templates are allowed here");
> > + return error_mark_node;
> > + }
> > + }
> > + /* [expr.prim.splice] For a splice-expression of the form
> > + template splice-specialization-specifier, the splice-specifier of the
> > + splice-specialization-specifier shall designate a template. */
> > + else
> > + {
> > + if (really_overloaded_fn (t)
> > + || get_template_info (t)
> > + || TREE_CODE (t) == TEMPLATE_ID_EXPR)
> > + /* OK */;
> > + else
> > + {
> > + auto_diagnostic_group d;
> > + error_at (loc, "reflection not usable in a template splice");
> > + inform (loc, "only templates are allowed here");
> > + return error_mark_node;
> > + }
> > + }
> > + }
> > + else if (/* No 'template' but there were template arguments? */
> > + targs_p
> > + /* No 'template' but the splice-specifier designates a template? */
> > + || really_overloaded_fn (t))
> > + {
> > + auto_diagnostic_group d;
> > + if (targs_p)
> > + error_at (loc, "reflection not usable in a splice expression with "
> > + "template arguments");
> > + else
> > + error_at (loc, "reflection not usable in a splice expression");
> > + location_t sloc = expr.get_start ();
> > + rich_location richloc (line_table, sloc);
> > + richloc.add_fixit_insert_before (sloc, "template ");
> > + inform (&richloc, "add %<template%> to denote a template");
> > + return error_mark_node;
>
> These "reflection not usable" errors also need to print the reflection.
Done in
<https://forge.sourceware.org/marek/gcc/commit/dbb59d54054671e3e292a93786115f283493662e>
> > + /* [expr.prim.splice]/2: "The expression is ill-formed if S [the
> > construct
> > + designated by splice-specifier] is
> > + -- a local entity such that there is a lambda scope that intervenes
> > + between the expression and the point at which S was introduced" */
> > + // TODO This should be moved to check_splice_expr.
>
> TODO
Done:
<https://forge.sourceware.org/marek/gcc/commit/c369cd1bb31bf199105b313fa9a4c5658b5b436f>
This also fixed a few bugs.
> > + if (outer_automatic_var_p (t))
> > + {
> > + auto_diagnostic_group d;
> > + error_at (loc, "cannot splice local entity %qD for which there is an
> > "
> > + "intervening lambda expression", t);
> > + inform (DECL_SOURCE_LOCATION (t), "%qD declared here", t);
> > + return error_mark_node;
> > + }
>
> Also, outer_automatic_var_p doesn't necessarily mean there are any lambdas
> involved; we could be in a local class member function.
Fixed in
<https://forge.sourceware.org/marek/gcc/commit/c369cd1bb31bf199105b313fa9a4c5658b5b436f>
as well.
> > + /* When doing foo.[: bar :], cp_parser_postfix_dot_deref_expression wants
> > + to see an identifier or a TEMPLATE_ID_EXPR, if we have something like
> > + s.template [: ^^S::var :]<int> where S::var is a variable template.
> > */
> > + if (member_access_p)
> > + {
> > + /* Grab the unresolved expression then. */
> > + t = unresolved;
> > + if (DECL_P (t))
> > + /* We cannot forget what context we came from, so build up
> > + a SCOPE_REF. */
> > + t = build_qualified_name (/*type=*/NULL_TREE, CP_DECL_CONTEXT (t),
> > + DECL_NAME (t), /*template_p=*/false);
> > + else if (OVL_P (t))
> > + t = OVL_NAME (t);
>
> It seems wrong to throw away the result of our earlier name lookup, can we
> instead improve ...dot_deref... to handle these forms as arguments?
This was partially fixed in
<https://forge.sourceware.org/marek/gcc/commit/dd022bfd042c6583fd68074abc2a36372a686dbc>
> > + {
> > + /* 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.
> > +cp_parser_splice_scope_specifier (cp_parser *parser, bool typename_p,
> > + bool template_p)
> > +{
> > + bool targs_p = false;
> > + cp_expr scope = cp_parser_splice_specifier (parser, template_p,
> > &targs_p);
> > + location_t loc = scope.get_location ();
> > + if (TREE_CODE (scope) == TYPE_DECL)
> > + scope = TREE_TYPE (scope);
> > +
> > + if (template_p && !targs_p)
> > + {
> > + error_at (loc, "extra %<template%> in a scope splice");
> > + return error_mark_node;
> > + }
> > + /* [expr.prim.id.qual] The template may only be omitted from the
> > + form template(opt) splice-specialization-specifier :: when the
> > + splice-specialization-specifier is preceded by typename. */
> > + if (targs_p && !typename_p)
> > + {
> > + // TODO add error
> > + }
>
> TODO
Done in
<https://forge.sourceware.org/marek/gcc/commit/3ce8b32fe6a599f63569591451ec4b4090626cdb>
> > + if (!valid_splice_scope_p (scope))
> > + {
> > + auto_diagnostic_group d;
> > + error_at (loc, "reflection not usable in a splice scope");
> > + if (TYPE_P (scope))
> > + inform (loc, "%qT is not a class, namespace, or enumeration",
> > + tree (scope));
>
> Let's print the reflection even if it isn't a type.
Done in
<https://forge.sourceware.org/marek/gcc/commit/dbb59d54054671e3e292a93786115f283493662e>
> > +/* Skip tokens until a non-nested closing CLOSE_TOKEN is the next
> > + token, or there are no more tokens. Return true in the first case,
> > + false otherwise. */
> > +
> > +// TODO: use instead of cp_parser_skip_to_closing_brace and
> > +// cp_parser_skip_up_to_closing_square_bracket
>
> Or perhaps you could enhance/use cp_parser_skip_balanced_tokens here instead
> of adding a new skip function?
OK, done:
<https://forge.sourceware.org/marek/gcc/commit/597b973db9742ab66172cd8bf21a0dbef20dd7c3>
I still think it would be nice to re-add cp_parser_skip_to_closing_token and
remove cp_parser_skip_to_closing_brace and
cp_parser_skip_up_to_closing_square_bracket. But not as part of the Reflection
patch set.
> > +/* We know the next token is '[:' (optionally preceded by a template or
> > + typename) and we are wondering if a '::' follows right after the
> > + closing ':]', or after the possible '<...>' after the ':]'. Return
> > + true if yes, false otherwise. */
> > +
> > +static bool
> > +cp_parser_splice_spec_is_nns_p (cp_parser *parser)
>
> All the callers first establish that we're looking at a splice and then call
> this function; I'd think we could reduce that to a single call to
> cp_parser_nth_token_starts_splice_nns_p?
Well, mostly I'm checking that '::' does *not* follow the ':]' or ':]<>'.
I've introduced cp_parser_nth_token_starts_splice_without_nns_p in
<https://forge.sourceware.org/marek/gcc/commit/7bc96701c2efa3b39a3d4535301331cc33e275af>.
> > + /* ??? 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.
> > @ -7098,6 +7577,20 @@ cp_parser_unqualified_id (cp_parser* parser,
> > if (cp_parser_parse_definitely (parser))
> > done = true;
> > }
> > + /* Allow r.~typename [:R:]. */
> > + else if (!done
> > + && cp_parser_next_tokens_start_splice_type_spec_p
> > + (parser, /*require_typename_p=*/true))
> > + {
> > + parser->scope = object_scope;
> > + parser->object_scope = NULL_TREE;
> > + parser->qualifying_scope = NULL_TREE;
> > + type_decl = cp_parser_splice_type_specifier (parser);
> > + if (!type_decl)
> > + return error_mark_node;
> > + /* We don't have a TYPE_DECL, so return early. */
> > + return build_min_nt_loc (loc, BIT_NOT_EXPR, type_decl);
>
> The comment seems to belong one line higher.
Fixed in
<https://forge.sourceware.org/marek/gcc/commit/7bc96701c2efa3b39a3d4535301331cc33e275af>
> > @@ -9037,15 +9565,32 @@ cp_parser_postfix_dot_deref_expression (cp_parser
> > *parser,
> > ordinary class member access expression, rather than a
> > pseudo-destructor-name. */
> > bool template_p;
> > + bool template_keyword_p = cp_parser_optional_template_keyword
> > (parser);
> > cp_token *token = cp_lexer_peek_token (parser->lexer);
> > - /* Parse the id-expression. */
> > - name = (cp_parser_id_expression
> > - (parser,
> > - cp_parser_optional_template_keyword (parser),
> > - /*check_dependency_p=*/true,
> > - &template_p,
> > - /*declarator_p=*/false,
> > - /*optional_p=*/false));
> > + /* See if there was this->[:R:]. Note that this->[: ^^S :]::i;
> > + is not a splice-expression. */
>
> "Note that [in] this->.... [the rhs] is not a splice-expression."
Fixed in
<https://forge.sourceware.org/marek/gcc/commit/7bc96701c2efa3b39a3d4535301331cc33e275af>
> > + [:^^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.
> > @@ -9087,13 +9641,17 @@ cp_parser_postfix_dot_deref_expression (cp_parser
> > *parser,
> > parser->qualifying_scope = NULL_TREE;
> > parser->object_scope = NULL_TREE;
> > }
> > - if (parser->scope && name && BASELINK_P (name))
> > + if ((parser->scope || splice_p) && name && BASELINK_P (name))
> > adjust_result_of_qualified_name_lookup
> > - (name, parser->scope, scope);
> > + (name,
> > + /* For obj->[:^^R:] we won't have parser->scope, but we still
> > + have to perform this adjustment. */
> > + (splice_p ? BINFO_TYPE (BASELINK_BINFO (name)) : parser->scope),
> > + scope);
>
> I think BASELINK_ACCESS_BINFO would be more accurate.
Changed in
<https://forge.sourceware.org/marek/gcc/commit/d63d99be1d7db7ed9871ec80c1f6a59f3c65b3e3>
> Still more to come...
Thanks a lot. I won't post a new series until the comments in
<https://gcc.gnu.org/pipermail/gcc-patches/2025-December/702950.html>
are addressed as well.
Marek