Message ID | 201902040553.AA04223@tamuki.linet.gr.jp |
---|---|
State | New |
Headers | show |
Series | Using array_length macro outside function | expand |
* TAMUKI Shoichi: > [1] to calculate the number of elements in the array. > > [1] https://sourceware.org/ml/libc-alpha/2017-10/msg01054.html > > Now, I tried to further improve this as patched below; > > ---------------------------------------------------------------------- > diff --git a/time/tst-strftime2.c b/time/tst-strftime2.c > index 57d2144..6c2d359 100644 > --- a/time/tst-strftime2.c > +++ b/time/tst-strftime2.c > @@ -25,8 +25,10 @@ > #include <string.h> > > static const char *locales[] = { "ja_JP.UTF-8", "lo_LA.UTF-8", "th_TH.UTF-8" }; > +#define nlocales array_length (locales) > +static char ref[nlocales][nformats][ndates][100]; The other problem is that array_length (locales) is not a constant expression because a statement expression is never a constant expression. I had not considered these problems when I wrote the array_length macro. I would like to retain the verification that the macro argument is not a pointer. What do you think about the patch below? Thanks, Florian array_length: Make usable as a constant expression Do not use a statement expression and _Static_assert in array_length, so that array_length can be used at file scope and as a constant expression. The compiler error message looks like this, so this is still acceptable: t.c:3:28: error: zero width for bit-field ‘ARGUMENT_NOT_AN_ARRAY’ 0 * sizeof (struct { int ARGUMENT_NOT_AN_ARRAY: \ ^~~~~~~~~~~~~~~~~~~~~ t.c:10:7: note: in expansion of macro ‘array_length’ int b[array_length (c) + 1]; ^~~~~~~~~~~~ 2019-02-04 Florian Weimer <fweimer@redhat.com> * include/array_length.h (array_length): Do not use a statement expression and _Static_assert, so that array_length can be used at file scope and as a constant expression. diff --git a/include/array_length.h b/include/array_length.h index 65f583063d..cee8897ba7 100644 --- a/include/array_length.h +++ b/include/array_length.h @@ -21,13 +21,11 @@ /* array_length (VAR) is the number of elements in the array VAR. VAR must evaluate to an array, not a pointer. */ -#define array_length(var) \ - __extension__ ({ \ - _Static_assert (!__builtin_types_compatible_p \ - (__typeof (var), __typeof (&(var)[0])), \ - "argument must be an array"); \ - sizeof (var) / sizeof ((var)[0]); \ - }) +#define array_length(var) \ + (sizeof (var) / sizeof ((var)[0]) \ + + 0 * sizeof (struct { int ARGUMENT_NOT_AN_ARRAY: \ + 1 - (__builtin_types_compatible_p \ + (__typeof (var), __typeof (&(var)[0]))); })) /* array_end (VAR) is a pointer one past the end of the array VAR. VAR must evaluate to an array, not a pointer. */
On Mon, 4 Feb 2019, Florian Weimer wrote: > Do not use a statement expression and _Static_assert in array_length, > so that array_length can be used at file scope and as a constant > expression. > > The compiler error message looks like this, so this is still > acceptable: > > t.c:3:28: error: zero width for bit-field ‘ARGUMENT_NOT_AN_ARRAY’ > 0 * sizeof (struct { int ARGUMENT_NOT_AN_ARRAY: \ You can use _Static_assert inside a struct; you don't need to do things with bit-field sizes.
On 2/4/19 6:34 AM, Florian Weimer wrote: > +#define array_length(var) \ > + (sizeof (var) / sizeof ((var)[0]) \ > + + 0 * sizeof (struct { int ARGUMENT_NOT_AN_ARRAY: \ > + 1 - (__builtin_types_compatible_p \ > + (__typeof (var), __typeof (&(var)[0]))); })) In Gnulib apps we've needed static assertions in expressions so often that we have a macro 'verify_expr (R, E)' that means "verify R statically, and yield the value of E". With this macro, you could rewrite the above definition as: #define array_length(var) \ verify_expr (!__builtin_types_compatible_p (__typeof (var), \ __typeof (&(var)[0])), \ sizeof (var) / sizeof ((var)[0])) which is quite a bit easier to follow. So I suggest appending something like the following to include/verify.h: #define verify_expr(r, e) \ (sizeof (struct { _Static_assert (r, #r); char __dummy; }) \ ? (e) : (e)) Then we can have array_length.h include verify.h and use the abovementioned simpler array_length. (The Gnulib verify_expr caters to non-GNU and pre-C99 compilers and so is more complicated, but we don't need that complexity in glibc.)
Hello Florian-san, From: Florian Weimer <fweimer@redhat.com> Subject: Re: Using array_length macro outside function Date: Mon, 04 Feb 2019 15:34:09 +0100 > The other problem is that array_length (locales) is not a constant > expression because a statement expression is never a constant > expression. I had not considered these problems when I wrote the > array_length macro. > > I would like to retain the verification that the macro argument is not a > pointer. What do you think about the patch below? Thank you. Applying your patch, I have confirmed that the test can be compiled successfully. Regards, TAMUKI Shoichi
* Joseph Myers: > On Mon, 4 Feb 2019, Florian Weimer wrote: > >> Do not use a statement expression and _Static_assert in array_length, >> so that array_length can be used at file scope and as a constant >> expression. >> >> The compiler error message looks like this, so this is still >> acceptable: >> >> t.c:3:28: error: zero width for bit-field ‘ARGUMENT_NOT_AN_ARRAY’ >> 0 * sizeof (struct { int ARGUMENT_NOT_AN_ARRAY: \ > > You can use _Static_assert inside a struct; you don't need to do things > with bit-field sizes. That's certainly nicer. Please consider the patch below. Thanks, Florian array_length: Make usable as a constant expression Do not use a statement expression in array_length, so that array_length can be used at file scope and as a constant expression. Instead, put the _Static_assert into a struct (as a declaration), and nest this in the expression using a sizeof expression. 2019-02-04 Florian Weimer <fweimer@redhat.com> * include/array_length.h (array_length): Do not use a statement expression and _Static_assert, so that array_length can be used at file scope and as a constant expression. diff --git a/include/array_length.h b/include/array_length.h index 65f583063d..db98a69899 100644 --- a/include/array_length.h +++ b/include/array_length.h @@ -22,12 +22,12 @@ /* array_length (VAR) is the number of elements in the array VAR. VAR must evaluate to an array, not a pointer. */ #define array_length(var) \ - __extension__ ({ \ - _Static_assert (!__builtin_types_compatible_p \ - (__typeof (var), __typeof (&(var)[0])), \ - "argument must be an array"); \ - sizeof (var) / sizeof ((var)[0]); \ - }) + (sizeof (var) / sizeof ((var)[0]) \ + + 0 * sizeof (struct { \ + _Static_assert (!__builtin_types_compatible_p \ + (__typeof (var), __typeof (&(var)[0])), \ + "argument must be an array"); \ + })) /* array_end (VAR) is a pointer one past the end of the array VAR. VAR must evaluate to an array, not a pointer. */
Hello Florian-san, From: Florian Weimer <fweimer@redhat.com> Subject: Re: Using array_length macro outside function Date: Tue, 05 Feb 2019 13:29:08 +0100 > > You can use _Static_assert inside a struct; you don't need to do things > > with bit-field sizes. > > That's certainly nicer. Please consider the patch below. Looks good to me. Applying the revised patch, I have also confirmed that the test can be compiled successfully. Tested-by: TAMUKI Shoichi <tamuki@linet.gr.jp> Regards, TAMUKI Shoichi
diff --git a/time/tst-strftime2.c b/time/tst-strftime2.c index 57d2144..6c2d359 100644 --- a/time/tst-strftime2.c +++ b/time/tst-strftime2.c @@ -25,8 +25,10 @@ #include <string.h> static const char *locales[] = { "ja_JP.UTF-8", "lo_LA.UTF-8", "th_TH.UTF-8" }; +#define nlocales array_length (locales) static const char *formats[] = { "%EY", "%_EY", "%-EY" }; +#define nformats array_length (formats) static const struct { @@ -40,8 +42,9 @@ static const struct { 1, 3, 97 }, { 1, 3, 98 } }; +#define ndates array_length (dates) -static char ref[3][3][6][100]; +static char ref[nlocales][nformats][ndates][100]; static void mkreftable (void) @@ -51,9 +54,9 @@ mkreftable (void) static const int yrj[] = { 63, 64, 1, 2, 9, 10 }; static const int yrb[] = { 2531, 2532, 2532, 2533, 2540, 2541 }; - for (i = 0; i < array_length (locales); i++) - for (j = 0; j < array_length (formats); j++) - for (k = 0; k < array_length (dates); k++) + for (i = 0; i < nlocales; i++) + for (j = 0; j < nformats; j++) + for (k = 0; k < ndates; k++) { if (i == 0) { @@ -93,7 +96,7 @@ do_test (void) size_t r, e; mkreftable (); - for (i = 0; i < array_length (locales); i++) + for (i = 0; i < nlocales; i++) { if (setlocale (LC_ALL, locales[i]) == NULL) { @@ -101,9 +104,9 @@ do_test (void) continue; } printf ("[%s]\n", locales[i]); - for (j = 0; j < array_length (formats); j++) + for (j = 0; j < nformats; j++) { - for (k = 0; k < array_length (dates); k++) + for (k = 0; k < ndates; k++) { ttm.tm_mday = dates[k].d; ttm.tm_mon = dates[k].m; ---------------------------------------------------------------------- and it resulted in an error and the compilation failed: ---------------------------------------------------------------------- gcc tst-strftime2.c -c -std=gnu11 -fgnu89-inline -O2 -g -m64 -fmessage-length=0 -D_FORTIFY_SOURCE=2 -funwind-tables \ -fasynchronous-unwind-tables -U_FORTIFY_SOURCE -g -Wall -Wwrite-strings -Wundef -Werror -fmerge-all-constants -frounding-math \ -fstack-protector -Wstrict-prototypes -Wold-style-definition -fmath-errno -I../include \ -I/home/tamuki/rpmbuild/BUILD/glibc-2.29/cc-base/time -I/home/tamuki/rpmbuild/BUILD/glibc-2.29/cc-base \ -I../sysdeps/unix/sysv/linux/x86_64/64 -I../sysdeps/unix/sysv/linux/x86_64 -I../sysdeps/unix/sysv/linux/x86/include \ -I../sysdeps/unix/sysv/linux/x86 -I../sysdeps/x86/nptl -I../sysdeps/unix/sysv/linux/wordsize-64 -I../sysdeps/x86_64/nptl \ -I../sysdeps/unix/sysv/linux/include -I../sysdeps/unix/sysv/linux -I../sysdeps/nptl -I../sysdeps/pthread -I../sysdeps/gnu \ -I../sysdeps/unix/inet -I../sysdeps/unix/sysv -I../sysdeps/unix/x86_64 -I../sysdeps/unix -I../sysdeps/posix \ -I../sysdeps/x86_64/64 -I../sysdeps/x86_64/fpu/multiarch -I../sysdeps/x86_64/fpu -I../sysdeps/x86/fpu/include \ -I../sysdeps/x86/fpu -I../sysdeps/x86_64/multiarch -I../sysdeps/x86_64 -I../sysdeps/x86 -I../sysdeps/ieee754/float128 \ -I../sysdeps/ieee754/ldbl-96/include -I../sysdeps/ieee754/ldbl-96 -I../sysdeps/ieee754/dbl-64/wordsize-64 \ -I../sysdeps/ieee754/dbl-64 -I../sysdeps/ieee754/flt-32 -I../sysdeps/wordsize-64 -I../sysdeps/ieee754 \ -I../sysdeps/generic -I.. -I../libio -I. -D_LIBC_REENTRANT \ -include /home/tamuki/rpmbuild/BUILD/glibc-2.29/cc-base/libc-modules.h -DMODULE_NAME=testsuite \ -include ../include/libc-symbols.h -DPIC -DTOP_NAMESPACE=glibc \ -o /home/tamuki/rpmbuild/BUILD/glibc-2.29/cc-base/time/tst-strftime2.o -MD -MP -MF \ /home/tamuki/rpmbuild/BUILD/glibc-2.29/cc-base/time/tst-strftime2.o.dt -MT \ /home/tamuki/rpmbuild/BUILD/glibc-2.29/cc-base/time/tst-strftime2.o In file included from tst-strftime2.c:21:0: ../include/array_length.h:25:17: error: braced-group within expression allowed only inside a function __extension__ ({ \ ^ tst-strftime2.c:28:18: note: in expansion of macro 'array_length' #define nlocales array_length (locales) ^~~~~~~~~~~~ tst-strftime2.c:47:17: note: in expansion of macro 'nlocales' static char ref[nlocales][nformats][ndates][100]; ^~~~~~~~ ../include/array_length.h:25:17: error: braced-group within expression allowed only inside a function __extension__ ({ \ ^ tst-strftime2.c:31:18: note: in expansion of macro 'array_length' #define nformats array_length (formats) ^~~~~~~~~~~~ tst-strftime2.c:47:27: note: in expansion of macro 'nformats' static char ref[nlocales][nformats][ndates][100]; ^~~~~~~~ ../include/array_length.h:25:17: error: braced-group within expression allowed only inside a function __extension__ ({ \ ^ tst-strftime2.c:45:16: note: in expansion of macro 'array_length' #define ndates array_length (dates) ^~~~~~~~~~~~ tst-strftime2.c:47:37: note: in expansion of macro 'ndates' static char ref[nlocales][nformats][ndates][100]; ^~~~~~ tst-strftime2.c:47:13: error: 'ref' defined but not used [-Werror=unused-variable] static char ref[nlocales][nformats][ndates][100]; ^~~ cc1: all warnings being treated as errors make[2]: *** [../o-iterator.mk:9: /home/tamuki/rpmbuild/BUILD/glibc-2.29/cc-base/time/tst-strftime2.o] Error 1 ---------------------------------------------------------------------- Can we use array_length macro outside function? If the type validation is removed from the current array_length macro as patched below, it can be compiled successfully. ---------------------------------------------------------------------- diff --git a/include/array_length.h b/include/array_length.h index 65f5830..c9ce702 100644 --- a/include/array_length.h +++ b/include/array_length.h @@ -21,13 +21,7 @@ /* array_length (VAR) is the number of elements in the array VAR. VAR must evaluate to an array, not a pointer. */ -#define array_length(var) \ - __extension__ ({ \ - _Static_assert (!__builtin_types_compatible_p \ - (__typeof (var), __typeof (&(var)[0])), \ - "argument must be an array"); \ - sizeof (var) / sizeof ((var)[0]); \ - }) +#define array_length(var) (sizeof (var) / sizeof ((var)[0])) /* array_end (VAR) is a pointer one past the end of the array VAR. VAR must evaluate to an array, not a pointer. */