Hi Martin,

On Sun, Aug 04, 2024 at 11:39:26AM GMT, Martin Uecker wrote:
> Am Sonntag, dem 04.08.2024 um 10:25 +0200 schrieb Alejandro Colomar:
> > Hi Martin,
> > 
> > On Sun, Aug 04, 2024 at 07:38:49AM GMT, Martin Uecker wrote:
> > > Am Sonntag, dem 04.08.2024 um 01:17 +0200 schrieb Alejandro Colomar:
> > > > 
> > > > FUTURE DIRECTIONS:
> > > > 
> > > >         We could make it work with array parameters to functions, and
> > > >         somehow magically return the length designator of the array,
> > > >         regardless of it being really a pointer.
> > > 
> > > And maybe flexible array members with "counted_by" attribute.
> > 
> > Hmmm; I didn't know that one.  Indeed.  I'll have a look at implementing
> > that in this patch set.
> 
> But maybe wait for this:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116016

Maybe; if I don't find a way to implement it now, their patches may give
me some tool to do it.  I'll still try to do it now, just to try.

I've drafted something by making build_counted_by_ref() an extern
function, and then I wrote

        diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
        index 9f5feb83345..875d471a1f4 100644
        --- a/gcc/c-family/c-common.cc
        +++ b/gcc/c-family/c-common.cc
        @@ -4078,6 +4078,7 @@ c_alignof_expr (location_t loc, tree expr)
         tree
         c_lengthof_type (location_t loc, tree type)
         {
        +  tree counted_by;
           enum tree_code type_code;
         
           type_code = TREE_CODE (type);
        @@ -4086,6 +4087,12 @@ c_lengthof_type (location_t loc, tree type)
               error_at (loc, "invalid application of %<lengthof%> to type 
%qT", type);
               return error_mark_node;
             }
        +
        +  counted_by = lookup_attribute ("counted_by", DECL_ATTRIBUTES (type));
        +  if (counted_by)
        +    // XXX: How to build the counted_by ref without the parent struct 
type?
        +    return build_counted_by_ref (NULL, type, counted_by);
        +
           if (!COMPLETE_TYPE_P (type))
             {
               error_at (loc,

But since I don't have the struct to which the FAM belongs, I don't know
how to get that working.  Do you have any idea?  Or maybe it's better to
just wait for those patches to be merged, and reuse their
infrastructure?


> > > > +
> > > > +/* Implement the lengthof keyword: Return the length of an array,
> > > > +   that is, the number of elements in the array.  */
> > > > +
> > > > +tree
> > > > +c_lengthof_type (location_t loc, tree type)
> > > > +{
> > > > +  enum tree_code type_code;
> > > > +
> > > > +  type_code = TREE_CODE (type);
> > > > +  if (!COMPLETE_TYPE_P (type))
> > > > +    {
> > > > +      error_at (loc,
> > > > +               "invalid application of %<lengthof%> to incomplete type 
> > > > %qT",
> > > > +               type);
> > > > +      return error_mark_node;
> > > > +    }
> > > > +  if (type_code != ARRAY_TYPE)
> > > > +    {
> > > > +      error_at (loc, "invalid application of %<lengthof%> to type 
> > > > %qT", type);
> > > > +      return error_mark_node;
> > > > +    }
> > > 
> > > I would swap these two errors, because the first is more specific and
> > > less helpful if you pass an incomplete struct, where it would be better
> > > to get the second error.
> > 
> > Agree.
> > 
> > BTW, I still don't understand what `if (! TYPE_DOMAIN (type))` means,
> > within array_type_nelts_minus_one().  What code triggers that condition?
> > Am I missing error handling for that?  Thanks!
> 
> For incomplete arrays, basically we have the following different
> variants for arrays:

Thanks!  This list is very useful.

> T[ ] incomplete: !TYPE_DOMAIN 
> T[1] constant size: TYPE_MAX_VALUE == INTEGER_CST

I guess that old flexible-array members using [1] are understood as
normal arrays [42], right?

> T[n] variable size: TYPE_MAX_VALUE != INTEGER_CST
> T[0] flexible array member: !TYPE_MAX_VALUE && !C_TYPE_VARIABLE_SIZE
>   (ISO version T[0] has TYPE_SIZE == NULL_TREE)

ISO version is T[], right?  Did ISO add support for zero-sized arrays?

> T[*] unspecified variable size: !TYPE_MAX_VALUE && C_TYPE_VARIABLE_SIZE

This is not allowed aoutside of function-prototype scope.  And there it
decays to pointer way before reaching __lengthof__.  So we can't handle
that at the moment.  However, we'll have to keep it in mind for when you
do the change to keep the array types of function prototypes.  When that
happens, I guess we'll have to reject these arrays.

> 
> The first should give an error.

Agree (and already implemented []).

> The next two should work giving an
> integer constant expression or run-time size, respectively. 

Agree (and already implemented [42] and [n]).

> The ISO FAM case should also give an error,

Agree (and already implemented []).  (Although I didn't really
distinguish it from an incomplete type.)

Although, if attributed with counted_by, we'd like it to work.  But this
is not yet implemented.

> while the GNU fam case
> could return zero to be consistent with sizeof (not sure).

Agree.  I've made it consistent with sizeof, and it returns 0.

BTW, I'd like to add full support for zero-sized arrays in the language.
I was discussing it with Jens Gustedt last week, and will start some
discussion about it in a separate thread.

> The last 
> case should return a non-constant.

The last case [*] is only allowed in prototypes.  How should we get the
non-constant value?  It's just another way to say [], isn't it?

> If you reuse the sizeof code, it should be mostly correct, but
> maybe the last case needs to be revisted. In the following
> examples
> 
> void foo(char (*a)[3][*], int (*x)[__lengthof__(*a)]);
> void bar(char (*a)[*][3], int (*x)[__lengthof__(*a)]);
> 
> the array '*x' should be a regular fixed size array in foo
> but a VLA in 'bar'.

In the function prototype, it seems to be completely ignoring
__lengthof__, just as it ignores any expression, so I don't know if it's
working (I could try to print some debugging values to stderr from the
compiler, if we care about it).

        $ cat muecker.h 
        void foo(char (*a)[3][*], int (*x)[__lengthof__(*a)]);
        void bar(char (*a)[*][3], int (*x)[__lengthof__(*a)]);
        void f(char (*a)[3][*], int (*x)[sizeof(*a)]);
        void b(char (*a)[*][3], int (*x)[sizeof(*a)]);
        $ /opt/local/gnu/gcc/lengthof/bin/gcc muecker.h -S
        $

I assume the code above is not reaching my code.

In the function definition, it doesn't accept [*] at all, so I can't
handle it:

        $ cat muecker.c
        void foo(char (*a)[3][*], int (*x)[__lengthof__(*a)]) {}
        void bar(char (*a)[*][3], int (*x)[__lengthof__(*a)]) {}
        void f(char (*a)[3][*], int (*x)[sizeof(*a)]) {}
        void b(char (*a)[*][3], int (*x)[sizeof(*a)]) {}
        $ /opt/local/gnu/gcc/lengthof/bin/gcc muecker.c -S
        muecker.c:1:1: error: ‘[*]’ not allowed in other than function 
prototype scope
            1 | void foo(char (*a)[3][*], int (*x)[__lengthof__(*a)]) {}
              | ^~~~
        muecker.c:2:1: error: ‘[*]’ not allowed in other than function 
prototype scope
            2 | void bar(char (*a)[*][3], int (*x)[__lengthof__(*a)]) {}
              | ^~~~
        muecker.c:3:1: error: ‘[*]’ not allowed in other than function 
prototype scope
            3 | void f(char (*a)[3][*], int (*x)[sizeof(*a)]) {}
              | ^~~~
        muecker.c:4:1: error: ‘[*]’ not allowed in other than function 
prototype scope
            4 | void b(char (*a)[*][3], int (*x)[sizeof(*a)]) {}
              | ^~~~


Have a lovely day!
Alex

> Martin

-- 
<https://www.alejandro-colomar.es/>

Attachment: signature.asc
Description: PGP signature

Reply via email to