> On Sep 10, 2024, at 17:47, Jakub Jelinek <ja...@redhat.com> wrote:
>
> 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;
Okay, I can add in_builtin_counted_by_ref = false lines before each break.
That might be the most simple and straightforward way to fix this concern. -:)
>
> 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.
Good point here. So, instead of of a boolean type, same as the other globals,
such as
In_typeof, in_sizeof, use an int type to record the level of the nesting might
be more proper.
What’s your suggestion here?
thanks.
Qing
>
> Jakub
>
