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

Reply via email to