[PATCH v4] Provide new GCC builtin __builtin_counted_by_ref [PR116016]

[Jakub Jelinek]

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