On Mon, Jul 29, 2024 at 10:55 AM Alejandro Colomar <a...@kernel.org> wrote:
>
> Hi Richard,
>
> On Mon, Jul 29, 2024 at 10:27:35AM GMT, Richard Biener wrote:
> > On Sun, Jul 28, 2024 at 4:16 PM Alejandro Colomar <a...@kernel.org> wrote:
> > >
> > > There were two identical definitions, and none of them are available
> > > where they are needed for implementing _Lengthof(). Merge them, and
> > > provide the single definition in gcc/tree.{h,cc}, where it's available
> > > for _Lengthof().
> > >
> > > Signed-off-by: Alejandro Colomar <a...@kernel.org>
> > > ---
> > > gcc/cp/cp-tree.h | 1 -
> > > gcc/cp/tree.cc | 13 -------------
> > > gcc/rust/backend/rust-tree.cc | 13 -------------
> > > gcc/rust/backend/rust-tree.h | 2 --
> > > gcc/tree.cc | 13 +++++++++++++
> > > gcc/tree.h | 1 +
> > > 6 files changed, 14 insertions(+), 29 deletions(-)
> > >
>
> [...]
>
> > > diff --git a/gcc/tree.cc b/gcc/tree.cc
> > > index 2d2d5b6db6e..3b0adb4cd9f 100644
> > > --- a/gcc/tree.cc
> > > +++ b/gcc/tree.cc
> > > @@ -3729,6 +3729,19 @@ array_type_nelts (const_tree type)
> > > ? max
> > > : fold_build2 (MINUS_EXPR, TREE_TYPE (max), max, min));
> > > }
> > > +
> > > +/* Return, as an INTEGER_CST node, the number of elements for TYPE
> > > + (which is an ARRAY_TYPE). This counts only elements of the top
> > > + array. */
> > > +
> > > +tree
> > > +array_type_nelts_top (tree type)
> > > +{
> > > + return fold_build2_loc (input_location,
> > > + PLUS_EXPR, sizetype,
> > > + array_type_nelts (type),
> > > + size_one_node);
> > > +}
> >
> > But this is now extremely confusing API with array_type_nelts above this
> > saying
> >
> > /* Return, as a tree node, the number of elements for TYPE (which is an
> > ARRAY_TYPE) minus one. This counts only elements of the top array. */
> >
> > so both are "_top". And there's build_array_type_nelts that's taking
> > the number of elements.
> >
> > Can you please rename the existing array_type_nelts to
> > array_type_nelts_minus_one? Then _top could be dropped as well from
> > the alternate API you add.
>
> I wanted to do that, but then I found other functions that are named
> similarly, such as build_array_type_nelts(), and thought that I wasn't
> sure if all of them should be renamed to _minus_one, or just some. So
> I decided to start without renaming.
Just array_type_nelts needs renaming, build_array_type_nelts is fine.
> But yeah, I think I should rename. I'll prepare a patch for renaming it
> independently of this patch set, and send it to be merged before this
> patch set.
Thanks.
> > I'll also note since array_type_nelts_top calls the other function and that
> > has
> >
> > /* If they did it with unspecified bounds, then we should have already
> > given an error about it before we got here. */
> > if (! TYPE_DOMAIN (type))
> > return error_mark_node;
> >
> > the function should handle error_mark_node (and pass that down).
>
> Hmmmm, now I understand that (! TYPE_DOMAIN (type))
>
> $ grep -rn return.array_type_nelts gcc
> gcc/cp/call.cc:12111: return array_type_nelts_top (c->type);
> gcc/c-family/c-common.cc:4090: return array_type_nelts_top (type);
>
> $ sed -n 12102,12119p gcc/cp/call.cc
> /* Return a tree representing the number of elements initialized by
> the
> list-initialization C. The caller must check that C converts to an
> array type. */
>
> static tree
> nelts_initialized_by_list_init (conversion *c)
> {
> /* If the array we're converting to has a dimension, we'll use
> that. */
> if (TYPE_DOMAIN (c->type))
> return array_type_nelts_top (c->type);
> else
> {
> /* Otherwise, we look at how many elements the constructor we're
> initializing from has. */
> tree ctor = conv_get_original_expr (c);
> return size_int (CONSTRUCTOR_NELTS (ctor));
> }
> }
The point is that if you make this a general API it should be safe to be used,
not depending on constraints that are apparently checked right now.
> It seems that would fail when measuring for example
>
> #define memberof(T, member) ((T){}.member)
>
> struct s {
> int x;
> int a[];
> };
>
> __lengthof__(memberof(struct s, a));
>
> I guess?
>
> $ cat len.c
> #include <stdio.h>
>
> #define memberof(T, member) ((T){}.member)
>
> struct s {
> int x;
> int y[];
> };
>
> int
> main(int argc, char *argv[argc + 1])
> {
> int a[42];
> size_t n;
>
> (void) argv;
>
> //n = __lengthof__(argv);
> //printf("__lengthof__(argv) == %zu\n", n);
>
> n = __lengthof__(a);
> printf("lengthof(a):\t %zu\n", n);
>
> n = __lengthof__(long [99]);
> printf("lengthof(long [99]):\t %zu\n", n);
>
> n = __lengthof__(short [n - 10]);
> printf("lengthof(short [n - 10]):\t %zu\n", n);
>
> int b[n / 2];
> n = __lengthof__(b);
> printf("lengthof(b):\t %zu\n", n);
>
> n = __lengthof__(memberof(struct s, y));
> printf("lengthof(memberof(struct s, y)):\t %zu\n", n);
> }
> alx@debian:~/tmp/gcc$ /opt/local/gnu/gcc/lengthof/bin/gcc len.c
> alx@debian:~/tmp/gcc$ ./a.out
> lengthof(a): 42
> lengthof(long [99]): 99
> lengthof(short [n - 10]): 89
> lengthof(b): 44
> lengthof(memberof(struct s, y)): 44
>
> Indeed, it's misbehaving. I'll have a look at that. I'll probably have
> to add an error similar to sizeof's one:
>
> len.c: In function ‘main’:
> len.c:37:19: error: invalid application of ‘sizeof’ to incomplete type ‘int[]’
> 37 | n = sizeof(memberof(struct s, y));
> | ^
>
> Thanks!
>
> >
> > Note array_type_nelts returns nelts - 1 because that avoids building
> > a new tree node for arrays with lower bound zero.
>
> What does it mean that the lower bound is zero? I didn't understand
> that part.
It means the function can return TYPE_MAX_VALUE of the TYPE_DOMAIN
unchanged.
Richard.
> >
> > Thanks,
> > Richard.
> >
> > > /* If arg is static -- a reference to an object in static storage -- then
> > > return the object. This is not the same as the C meaning of `static'.
>
> Have a lovely day!
> Alex
>
> --
> <https://www.alejandro-colomar.es/>
