Re: [PATCHv5 3/4] gdb/fortran: add support for parsing array strides in expressions

Simon Marchi
2020-10-12T13:21:57Z

Just some nits:

On 2020-10-11 2:12 p.m., Andrew Burgess wrote:
> @@ -314,6 +318,40 @@ subrange: ':' %prec ABOVE_COMMA
> write_exp_elt_opcode (pstate, OP_RANGE); }
> ;
>
> +/* And each of the four subrange types can also have a stride. */
> +subrange: exp ':' exp ':' exp %prec ABOVE_COMMA
> + { write_exp_elt_opcode (pstate, OP_RANGE);
> + write_exp_elt_longcst (pstate,
> + (RANGE_STANDARD
> + | RANGE_HAS_STRIDE));

It's no necessary to have "RANGE_STANDARD" here.

> @@ -141,6 +141,14 @@ value_f90_subarray (struct value *array,
> else
> high_bound = value_as_long (evaluate_subexp (nullptr, exp, pos, noside));
>
> + if ((range_flag & RANGE_HAS_STRIDE) == RANGE_HAS_STRIDE)

For consistency with the code just above this, this could be just:

if (range_flag & RANGE_HAS_STRIDE)

> diff --git a/gdb/testsuite/gdb.fortran/array-slices.exp b/gdb/testsuite/gdb.fortran/array-slices.exp
> index 31f95a3668d..a0e1d1fe8fc 100644
> --- a/gdb/testsuite/gdb.fortran/array-slices.exp
> +++ b/gdb/testsuite/gdb.fortran/array-slices.exp
> @@ -69,3 +69,18 @@ foreach result $array_contents msg $message_strings {
> }
>
> gdb_continue_to_breakpoint "continue to Final Breakpoint"
> +
> +# Next test that asking for an array with stride at the CLI gives an
> +# error.
> +clean_restart ${testfile}
> +
> +if ![fortran_runto_main] then {
> + perror "couldn't run to main"
> + continue
> +}
> +
> +gdb_breakpoint "show"
> +gdb_continue_to_breakpoint "show"
> +gdb_test "up" ".*"
> +gdb_test "p array (1:10:2, 1:10:2)" \
> + "Fortran array strides are not currently supported"

It would be better to give an name to this test, since the command ends
with a parenthesis, or remove the space, as mentioned here:

https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Do_not_use_.22tail_parentheses.22_on_test_messages

Simon