Hi Martin, On Wed, Aug 07, 2024 at 09:13:07AM GMT, Martin Uecker wrote: > Am Mittwoch, dem 07.08.2024 um 01:12 +0200 schrieb Alejandro Colomar: > > +#define c_parser_lengthof_expression(parser) > > \ > > +( > > \ > > + c_parser_sizeof_or_lengthof_expression (parser, RID_LENGTHOF) > > \ > > +) > > + > > I suggest to avoid the macros. I think the original function calls are > clear enough and this is then just another detour for somebody trying > to follow the code. Or is there a reason I am missing?
I imitated the following ones that already exist: c-family/c-common.h:923: #define c_sizeof(LOC, T) c_sizeof_or_alignof_type (LOC, T, true, false, 1) cp/cp-tree.h:8318: #define cxx_sizeof(T) cxx_sizeof_or_alignof_type (input_location, T, SIZEOF_EXPR, false, true) c-family/c-common.h:924: #define c_alignof(LOC, T) c_sizeof_or_alignof_type (LOC, T, false, false, 1) But I'm fine using it raw. > > +void fix_fix (int i, char (*a)[3][5], int (*x)[__lengthof__ (*a)]); > > +void fix_var (int i, char (*a)[3][i], int (*x)[__lengthof__ (*a)]); > > +void fix_uns (int i, char (*a)[3][*], int (*x)[__lengthof__ (*a)]); > > > It would include a test that shows that when lengthof > is applied to [*] that it remains formally non-constant. For example, > you could test with -Wvla-parameter that the two declarations do not give a > warning: > > void foo(char (*a)[*], int x[*]); > void foo(char (*a)[*], int x[__lengthof__(*a)]); But [*] is a VLA. Do we want to return a constexpr for it? > (With int (*x)[*] we would run into the issue that we can not > distinguish zero arrays from unspecified ones, PR 98539) As Martin Sebor said, I need to choose between supporting well [0] or supporting well [*], but not both. I would personally prefer supporting [0], and consider that not supporting [*] is a bug in the implementation of [*] (and thus not my problem). However, since GCC doesn't support 0-length arrays, I'm not sure that would be correct. What do you think? Does anyone oppose treating [0] as a constexpr 0 length? That means not supporting well [*], but please fix it separately, which Martin Uecker is working on. :) > > diff --git a/gcc/testsuite/gcc.dg/lengthof.c > > b/gcc/testsuite/gcc.dg/lengthof.c > > new file mode 100644 > > index 00000000000..38da5df52a5 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/lengthof.c > > @@ -0,0 +1,127 @@ > > +/* { dg-do run } */ > > +/* { dg-options "-Wno-declaration-after-statement -Wno-pedantic -Wno-vla" > > } */ > > + > > +#undef NDEBUG > > +#include <assert.h> > > + > > +void > > +array (void) > > +{ > > + short a[7]; > > + > > + assert (__lengthof__ (a) == 7); > > + assert (__lengthof__ (long [0]) == 0); > > + assert (__lengthof__ (unsigned [99]) == 99); > > +} > > Instead of using assert you can use > > if (! ...) __builtin_abort(); > > to avoid the include in the testsuite. Ā Is it frowned upon to include something? I prefer assert(3). > Otherwise it looks fine from my side. > > Joseph needs to approve and may have more comments. Thanks! > > Martin Have a lovely day! Alex -- <https://www.alejandro-colomar.es/>
signature.asc
Description: PGP signature