Am Mittwoch, dem 07.08.2024 um 01:12 +0200 schrieb Alejandro Colomar:
Hi Alex,
a coupled of comments below.
> --- a/gcc/c/c-parser.cc
> +++ b/gcc/c/c-parser.cc
> @@ -74,7 +74,17 @@ along with GCC; see the file COPYING3. If not see
> #include "bitmap.h"
> #include "analyzer/analyzer-language.h"
> #include "toplev.h"
> +
> +#define c_parser_sizeof_expression(parser)
> \
> +(
> \
> + c_parser_sizeof_or_lengthof_expression (parser, RID_SIZEOF)
> \
> +)
>
> +#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?
...
> diff --git a/gcc/testsuite/gcc.dg/lengthof-compile.c
> b/gcc/testsuite/gcc.dg/lengthof-compile.c
> new file mode 100644
> index 00000000000..6b44704ca7e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/lengthof-compile.c
> @@ -0,0 +1,49 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Wno-declaration-after-statement -Wno-pedantic -Wno-vla" }
> */
> +
> +extern int x[];
> +
> +void
> +incomplete (int p[])
> +{
> + unsigned n;
> +
> + n = __lengthof__ (x); /* { dg-error "incomplete" } */
> +
> + /* We want to support the following one in the future,
> + but for now it should fail. */
> + n = __lengthof__ (p); /* { dg-error "invalid" } */
> +}
> +
> +void
> +fam (void)
> +{
> + struct {
> + int x;
> + int fam[];
> + } s;
> + unsigned n;
> +
> + n = __lengthof__ (s.fam); /* { dg-error "incomplete" } */
> +}
> +
> +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)]);
(With int (*x)[*] we would run into the issue that we can not
distinguish zero arrays from unspecified ones, PR 98539)
> +
> +void
> +func (void)
> +{
> + int i3[3];
> + int i5[5];
> + char c35[3][5];
> +
> + fix_fix (5, &c35, &i3);
> + fix_fix (5, &c35, &i5); /* { dg-error "incompatible-pointer-types" } */
> +
> + fix_var (5, &c35, &i3);
> + fix_var (5, &c35, &i5); /* { dg-error "incompatible-pointer-types" } */
> +
> + fix_uns (5, &c35, &i3);
> + fix_uns (5, &c35, &i5); /* { dg-error "incompatible-pointer-types" } */
> +}
> 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.
Otherwise it looks fine from my side.
Joseph needs to approve and may have more comments.
Martin
