Am Mittwoch, dem 07.08.2024 um 11:14 +0200 schrieb Alejandro Colomar: > Hi Martin, >
> > > +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? No, my point is only that we could have a test for not returning a constant. If __lengthof__ would incorrectly return an integer constant expression then you would get a warning with -Wvla-parameter. So adding these two declarations to the tests and activating the warning would ensure that the int[__lengthof__(*a)] is a VLA: https://godbolt.org/z/7P7qW15ah > > > (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. If you have only one array index this works. (and should already work correctly with your patch) > > 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? I think the logic in your patch is OK as is. It does not exactly what you want, as it now treats some [0] as [*] but I would not make the logic more complex here when we will fix it properly anyway. > > 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). It makes the tests run faster. At least people told me before to avoid includes in tests for this reason. But from my side assert is ok too. Martin > > > Otherwise it looks fine from my side. > > > > Joseph needs to approve and may have more comments. > > Thanks! > > > > > Martin > > Have a lovely day! > Alex > -- Univ.-Prof. Dr. rer. nat. Martin Uecker Graz University of Technology Institute of Biomedical Imaging
