Re: [PATCH] Unbounded array support implemented (for Modula-2)

[Markus Deuling <deuling@de.ibm.com>]

Hi Gaius,

Gaius Mulley schrieb:
> Hi,
> 
> The patch below allows users debugging Modula-2 programs to print
> unbounded parameter contents, ptype the parameter declaration and
> perform unbounded array subscript queries.  It also adds TSIZE
> (pseudonym for SIZE) and implements HIGH (yields the last legal index
> for an unbounded array).
> 
> Wondering whether this is okay to commit?  Feel free to suggest
> improvements etc,
> 
> regards,
> Gaius
> 
> I've run check-gdb and get the following final results:
> 
> # of expected passes            11389
> # of unexpected failures        78
> # of unexpected successes       2
> # of expected failures          41
> # of known failures             39
> # of unresolved testcases       1
> # of untested testcases         8
> # of unsupported tests          14
> 
>  ... build-gdb/gdb/testsuite/../../gdb/gdb version
> # 6.6.50.20070724-cvs -nx
> 
> 

do you build GDB on x86? These are my results:

# of expected passes­···­·······11554
# of unexpected failures­·······29
# of unexpected successes­······1
# of expected failures­·­·······42
# of unknown successes­·­·······6
# of known failures­····­·······44
# of unresolved testcases­······1
# of untested testcases­­·······8
# of unsupported tests­·­·······16
/home/deuling/gdb/dev/build/gdb/testsuite/../../gdb/gdb version  6.6.50.20070725-cvs -nx

Maybe you should compare test results with and without your patch to see if your patch introduces regressions.
It also would be nice to have a new test case to test the new features.

> @@ -203,13 +239,15 @@
>        && TYPE_CODE (elttype) == TYPE_CODE_INT
>        && (format == 0 || format == 's')
>        && addr != 0)
> -      return val_print_string (addr, -1, TYPE_LENGTH (elttype), stream);
> +      return val_print_string (addr, -1, TYPE_LENGTH (elttype),
> +			       stream);
>    
>    return 0;
>  }
This seems to be unnecessary. The current line is < 80 chars.

> @@ -235,6 +273,49 @@
>      fputs_filtered ("???", stream);
>  }
>  
> +
> +/*
> + *  m2_print_array_contents - prints out the contents of an
> + *                            array up to a max_print values.
> + *                            It prints arrays of char as a string
> + *                            and all other data types as comma
> + *                            separated values.
> + */
The final */ should in the same line as "separated values." with two spaces between.
There are some more of that.

> +static struct value *
> +evaluate_subexp_modula2 (struct type *expect_type, struct expression *exp,
> +			 int *pos, enum noside noside)
> +{
> +  int pc = *pos;
> +  int i;
> +  char *name;
> +  enum exp_opcode op = exp->elts[*pos].opcode;
> +  struct value *arg1;
> +  struct value *arg2;
> +  struct type *type;
> +  switch (op)

I think "int i", "char *name" and "int pc" are unneeded in this function?
There should be an empty line after variable declaration to seperate it from the code.


Regards,

-- 
Markus Deuling
GNU Toolchain for Linux on Cell BE
deuling@de.ibm.com