Hi Jakub, On Wed, May 21, 2025 at 11:12:07PM +0200, Jakub Jelinek wrote: > > * c-common.h: Add RID_COUNTOF. > > (enum rid): Add RID_COUNTOF.
Okay.
>
> > (c_countof_type): New function prototype.
> > * c-common.def (COUNTOF_EXPR): New tree.
> > * c-common.cc
> > (c_common_reswords): Add RID_COUNTOF entry.
>
> No newline in between the line with file and (c_common_reswords).
> * c-common.cc (c_common_reswords): Add RID_COUNTOF entry.
> fits just fine. And even if the description wouldn't fit completely,
> you'd wrap on the first word that doesn't fit.
Okay.
> > warning_at (loc, OPT_Wc___compat,
> > "defining type in %qs expression is invalid in C++",
> > (in_sizeof
> > ? "sizeof"
> > - : (in_typeof ? "typeof" : "alignof")));
> > + : (in_typeof
> > + ? "typeof"
> > + : (in_alignof
> > + ? "alignof"
> > + : "_Countof"))));
>
> Why so many lines? Plus no idea why there are the ()s around, I see it is
> preexisting for the outermost where it can help emacs formatting, but the
> rest doesn't need that.
The preexisting ones are not just the outermost ones. The preexisting
code was
(in_sizeof
? "sizeof"
: (in_typeof ? "typeof" : "alignof")));
The only pattern I can find in that code is wrapping every ?:
subexpression in (). I would find not doing so for the last one I'm
adding inconsistent with it. On the other hand, I would agree that
limiting it to just the outermost ones would be more readable, but that
would mean removing existing ()s. So can you please confirm that I
should remove the existing inner ()?
> > +#define c_parser_sizeof_expression(parser)
> > \
> > +(
> > \
> > + c_parser_sizeof_or_countof_expression (parser, RID_SIZEOF)
> > \
> > +)
> >
> > +#define c_parser_countof_expression(parser)
> > \
> > +(
> > \
> > + c_parser_sizeof_or_countof_expression (parser, RID_COUNTOF)
> > \
> > +)
>
> Up to Joseph, but I'd say these should just be inline functions, not macros,
> and defined after the declarations.
I don't mind, so yeah, I'm okay with changing these to be inline.
Have a lovely night!
Alex
--
<https://www.alejandro-colomar.es/>
signature.asc
Description: PGP signature
