On Tue, Sep 10, 2024 at 09:28:04PM +0000, Qing Zhao wrote:
> @@ -11741,6 +11770,54 @@ c_parser_postfix_expression (c_parser *parser)
> set_c_expr_source_range (&expr, loc, close_paren_loc);
> break;
> }
> + case RID_BUILTIN_COUNTED_BY_REF:
> + {
> + vec<c_expr_t, va_gc> *cexpr_list;
> + c_expr_t *e_p;
> + location_t close_paren_loc;
> +
> + in_builtin_counted_by_ref = true;
> +
> + c_parser_consume_token (parser);
> + if (!c_parser_get_builtin_args (parser,
> + "__builtin_counted_by_ref",
> + &cexpr_list, false,
> + &close_paren_loc))
> + {
> + expr.set_error ();
> + goto error_exit;
Up to Joseph or Marek as C maintainers/reviewers, but I think it is a bad
idea to use such a generic name for a label inside of handling of one
specific keyword.
Either use RAII and just break; instead of goto error_exit;, like
struct in_builtin_counted_by_ref_sentinel {
~in_builtin_counted_by_ref_sentinel ()
{ in_builtin_counted_by_ref = false; }
} ibcbr_sentinel;
or add those in_builtin_counted_by_ref = false; lines before each break;
Or set it just when parsing the args?
Anyway, I'm not even convinced a global variable like that is a good idea.
The argument can contain arbitrary expressions in there (e.g. comma
expression, statement expression, ...), I strongly doubt you want to
have that special handling in all the places in the grammar rather than just
for the last COMPONENT_REF in there. And, there is no reason why
you couldn't have e.g. nested call inside of the argument:
__builtin_counted_by_ref (ptr[*__builtin_counted_by_ref
(something->fam1)]->fam2)
and that on the other side clears in_builtin_counted_by_ref after parsing
the inner one.
Jakub
