Hi Martin,

On Wed, Aug 07, 2024 at 12:07:00PM GMT, Martin Uecker wrote:
> > > 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. 

Ok.

> 
> 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've been thinking today that I'll add full support for [0], and let [*]
broken.

I'll also add tests for it.

> 
> > 
> > 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.

I'm detecting some issues with my patches.

        $ cat zero.c
        static int A[__lengthof__(int [0])];
        static int B[__lengthof__(A)];

        static int C[0];
        static int D[__lengthof__(C)];

        void fa(char (*a)[3][*], int (*x)[__lengthof__(*a)]);  // x: array
        void fb(char (*a)[*][3], int (*x)[__lengthof__(*a)]);  // x: vla
        void fc(char (*a)[3], int (*x)[__lengthof__(*a)]);  // x: array
        void fd(char (*a)[0], int (*x)[__lengthof__(*a)]);  // x: ?
        void fe(char (*a)[*], int (*x)[__lengthof__(*a)]);  // x: vla
        void ff(char (*a)[*], int (*x)[*]);  // x: array


        static int W[1];
        static int X[__lengthof__(W)];
        static int Y[0];
        static int Z[__lengthof__(Y)];

        $ /opt/local/gnu/gcc/lengthof/bin/gcc zero.c
        zero.c:18:12: error: variably modified ‘Z’ at file scope
           18 | static int Z[__lengthof__(Y)];
              |            ^


See that D, which is identical to Z, does not cause an error.
There's one case of [0] resulting in a constant expression, and another
in a VLA.  Can you please help investigate why it's happening?

I've added the following change on top of v5 to see some debugging info:

        diff --git i/gcc/c/c-typeck.cc w/gcc/c/c-typeck.cc
        index 98e8d31cb3b..9e05ee01a4a 100644
        --- i/gcc/c/c-typeck.cc
        +++ w/gcc/c/c-typeck.cc
        @@ -3462,6 +3462,8 @@ is_top_array_vla (tree type)
           bool zero, var;
           tree d;
         
        +fprintf(stderr, "ALX: %s(): %d\n", __func__, __LINE__);
        +
           if (TREE_CODE (type) != ARRAY_TYPE)
             return false;
           if (!COMPLETE_TYPE_P (type))
        @@ -3469,10 +3471,14 @@ is_top_array_vla (tree type)
         
           d = TYPE_DOMAIN (type);
           zero = !TYPE_MAX_VALUE (d);
        +fprintf(stderr, "ALX: zero: %d\n", !!zero);
           var = (!zero
                 && (TREE_CODE (TYPE_MIN_VALUE (d)) != INTEGER_CST
                     || TREE_CODE (TYPE_MAX_VALUE (d)) != INTEGER_CST));
        +fprintf(stderr, "ALX: var:    %d\n", !!var);
           var = var || (zero && C_TYPE_VARIABLE_SIZE (type));
        +fprintf(stderr, "ALX: var:    %d\n", !!var);
           return var;
         }
         
        @@ -3481,6 +3487,7 @@ is_top_array_vla (tree type)
         struct c_expr
         c_expr_lengthof_expr (location_t loc, struct c_expr expr)
         {
        +fprintf(stderr, "ALX: %s(): %d\n", __func__, __LINE__);
           struct c_expr ret;
           if (expr.value == error_mark_node)
             {
        @@ -3522,6 +3529,7 @@ c_expr_lengthof_expr (location_t loc, struct 
c_expr expr)
         struct c_expr
         c_expr_lengthof_type (location_t loc, struct c_type_name *t)
         {
        +fprintf(stderr, "ALX: %s(): %d\n", __func__, __LINE__);
           tree type;
           struct c_expr ret;
           tree type_expr = NULL_TREE;

which prints:

        $ /opt/local/gnu/gcc/lengthof/bin/gcc zero.c
        ALX: c_expr_lengthof_type(): 3531
        ALX: is_top_array_vla(): 3465
        ALX: zero: 1
        ALX: var:    0
        ALX: var:    0
        ALX: is_top_array_vla(): 3465
        ALX: zero: 1
        ALX: var:    0
        ALX: var:    0
        ALX: c_expr_lengthof_expr(): 3489
        ALX: is_top_array_vla(): 3465
        ALX: zero: 1
        ALX: var:    0
        ALX: var:    0
        ALX: is_top_array_vla(): 3465
        ALX: zero: 1
        ALX: var:    0
        ALX: var:    0
        ALX: c_expr_lengthof_expr(): 3489
        ALX: is_top_array_vla(): 3465
        ALX: zero: 1
        ALX: var:    0
        ALX: var:    0
        ALX: is_top_array_vla(): 3465
        ALX: zero: 1
        ALX: var:    0
        ALX: var:    0
        ALX: c_expr_lengthof_expr(): 3489
        ALX: is_top_array_vla(): 3465
        ALX: zero: 0
        ALX: var:    0
        ALX: var:    0
        ALX: is_top_array_vla(): 3465
        ALX: zero: 0
        ALX: var:    0
        ALX: var:    0
        ALX: c_expr_lengthof_expr(): 3489
        ALX: is_top_array_vla(): 3465
        ALX: zero: 1
        ALX: var:    0
        ALX: var:    1
        ALX: is_top_array_vla(): 3465
        ALX: zero: 1
        ALX: var:    0
        ALX: var:    1
        ALX: c_expr_lengthof_expr(): 3489
        ALX: is_top_array_vla(): 3465
        ALX: zero: 0
        ALX: var:    0
        ALX: var:    0
        ALX: is_top_array_vla(): 3465
        ALX: zero: 0
        ALX: var:    0
        ALX: var:    0
        ALX: c_expr_lengthof_expr(): 3489
        ALX: is_top_array_vla(): 3465
        ALX: zero: 1
        ALX: var:    0
        ALX: var:    1
        ALX: is_top_array_vla(): 3465
        ALX: zero: 1
        ALX: var:    0
        ALX: var:    1
        ALX: c_expr_lengthof_expr(): 3489
        ALX: is_top_array_vla(): 3465
        ALX: zero: 1
        ALX: var:    0
        ALX: var:    1
        ALX: is_top_array_vla(): 3465
        ALX: zero: 1
        ALX: var:    0
        ALX: var:    1
        ALX: c_expr_lengthof_expr(): 3489
        ALX: is_top_array_vla(): 3465
        ALX: zero: 0
        ALX: var:    0
        ALX: var:    0
        ALX: is_top_array_vla(): 3465
        ALX: zero: 0
        ALX: var:    0
        ALX: var:    0
        ALX: c_expr_lengthof_expr(): 3489
        ALX: is_top_array_vla(): 3465
        ALX: zero: 1
        ALX: var:    0
        ALX: var:    1
        ALX: is_top_array_vla(): 3465
        ALX: zero: 1
        ALX: var:    0
        ALX: var:    1
        zero.c:18:12: error: variably modified ‘Z’ at file scope
           18 | static int Z[__lengthof__(Y)];
              |            ^

If I make [0] always result in a constant expression (and thus break
some [*] cases), by doing

        -  var = var || (zero && C_TYPE_VARIABLE_SIZE (type));

Then the problem disappears.  But I'm worried that it might be hiding
the problem instead of removing it, since I don't really understand why
it's happening.  Do you know why?

Anyway, I'll remove that line to support [0].  But it would be
interesting to learn why this problem triggers.


Have a lovely night!
Alex

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

Attachment: signature.asc
Description: PGP signature

Reply via email to