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/>
signature.asc
Description: PGP signature