Large array printing patch for Fortran

Large array printing patch for Fortran

[David Lecomber]

Resubmitting this (at last) - posted originally in May.

The element printing limit for Fortran arrays is applied only for
the outer dimension of an array.  Consequently, on entering a subroutine
with an argument of a (say) 1000x1000 array, the frame will take months
to print, which kind of ruins commands like 'bt', 'info args' etc.. 


2003-09-10  David Lecomber  <dsl@sources.redhat.com>

	* f-valprint.c: Apply array element printing limits to 
	multi-dimensional arrays

Index: f-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/f-valprint.c,v
retrieving revision 1.15
diff -u -p -r1.15 f-valprint.c
--- f-valprint.c	9 Sep 2003 23:09:37 -0000	1.15
+++ f-valprint.c	10 Sep 2003 14:26:11 -0000
@@ -47,7 +47,8 @@ static void f77_print_array (struct type
 			     enum val_prettyprint);
 static void f77_print_array_1 (int, int, struct type *, char *,
 			       CORE_ADDR, struct ui_file *, int, int, int,
-			       enum val_prettyprint);
+			       enum val_prettyprint,
+			       int *elts);
 static void f77_create_arrayprint_offset_tbl (struct type *,
 					      struct ui_file *);
 static void f77_get_dynamic_length_of_aggregate (struct type *);
@@ -271,31 +272,36 @@ f77_create_arrayprint_offset_tbl (struct
     }
 }
 
+
+
 /* Actual function which prints out F77 arrays, Valaddr == address in 
    the superior.  Address == the address in the inferior.  */
-
 static void
 f77_print_array_1 (int nss, int ndimensions, struct type *type, char *valaddr,
 		   CORE_ADDR address, struct ui_file *stream, int format,
-		   int deref_ref, int recurse, enum val_prettyprint pretty)
+		   int deref_ref, int recurse, enum val_prettyprint pretty,
+		   int *elts)
 {
   int i;
 
   if (nss != ndimensions)
     {
-      for (i = 0; i < F77_DIM_SIZE (nss); i++)
+      for (i = 0; (i < F77_DIM_SIZE (nss) && (*elts) < print_max); i++)
 	{
 	  fprintf_filtered (stream, "( ");
 	  f77_print_array_1 (nss + 1, ndimensions, TYPE_TARGET_TYPE (type),
 			     valaddr + i * F77_DIM_OFFSET (nss),
 			     address + i * F77_DIM_OFFSET (nss),
-			     stream, format, deref_ref, recurse, pretty);
+			     stream, format, deref_ref, recurse, pretty, elts);
 	  fprintf_filtered (stream, ") ");
 	}
+      if (*elts >= print_max && i < F77_DIM_SIZE (nss)) {
+	fprintf_filtered (stream, "...");
+      }
     }
   else
     {
-      for (i = 0; (i < F77_DIM_SIZE (nss) && i < print_max); i++)
+      for (i = 0; (i < F77_DIM_SIZE (nss) && (*elts) < print_max); i++, (*elts)++)
 	{
 	  val_print (TYPE_TARGET_TYPE (type),
 		     valaddr + i * F77_DIM_OFFSET (ndimensions),
@@ -306,7 +312,7 @@ f77_print_array_1 (int nss, int ndimensi
 	  if (i != (F77_DIM_SIZE (nss) - 1))
 	    fprintf_filtered (stream, ", ");
 
-	  if (i == print_max - 1)
+	  if ((( *elts) == print_max - 1) && (i != (F77_DIM_SIZE (nss) - 1)))
 	    fprintf_filtered (stream, "...");
 	}
     }
@@ -321,6 +327,7 @@ f77_print_array (struct type *type, char
 		 enum val_prettyprint pretty)
 {
   int ndimensions;
+  int elts = 0;
 
   ndimensions = calc_f77_array_dims (type);
 
@@ -335,7 +342,7 @@ f77_print_array (struct type *type, char
   f77_create_arrayprint_offset_tbl (type, stream);
 
   f77_print_array_1 (1, ndimensions, type, valaddr, address, stream, format,
-		     deref_ref, recurse, pretty);
+		     deref_ref, recurse, pretty, &elts);
 }
 \f
 

Re: Large array printing patch for Fortran

[Daniel Jacobowitz]

On Wed, Sep 10, 2003 at 03:35:36PM +0100, David Lecomber wrote:
> Resubmitting this (at last) - posted originally in May.
> 
> The element printing limit for Fortran arrays is applied only for
> the outer dimension of an array.  Consequently, on entering a subroutine
> with an argument of a (say) 1000x1000 array, the frame will take months
> to print, which kind of ruins commands like 'bt', 'info args' etc.. 

David,

[Assuming that this wasn't reviewed offlist - if so, whoever approved
it, please don't do that.]

Yes, I know that this patch hasn't been reviewed.  That's a shame.  Nag
the list if you get impatient, but please stop checking things in to
CVS!  You're listed as a "Write After Approval" maintainer.  If you
can't wait for approval I'll ask to have you removed from that list.

This was on my list to look at this week, when we were sure 6.0 was in
the hole.

> 
> 
> 2003-09-10  David Lecomber  <dsl@sources.redhat.com>
> 
> 	* f-valprint.c: Apply array element printing limits to 
> 	multi-dimensional arrays
> 
> Index: f-valprint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/f-valprint.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 f-valprint.c
> --- f-valprint.c	9 Sep 2003 23:09:37 -0000	1.15
> +++ f-valprint.c	10 Sep 2003 14:26:11 -0000
> @@ -47,7 +47,8 @@ static void f77_print_array (struct type
>  			     enum val_prettyprint);
>  static void f77_print_array_1 (int, int, struct type *, char *,
>  			       CORE_ADDR, struct ui_file *, int, int, int,
> -			       enum val_prettyprint);
> +			       enum val_prettyprint,
> +			       int *elts);
>  static void f77_create_arrayprint_offset_tbl (struct type *,
>  					      struct ui_file *);
>  static void f77_get_dynamic_length_of_aggregate (struct type *);
> @@ -271,31 +272,36 @@ f77_create_arrayprint_offset_tbl (struct
>      }
>  }
>  
> +
> +
>  /* Actual function which prints out F77 arrays, Valaddr == address in 
>     the superior.  Address == the address in the inferior.  */
> -
>  static void
>  f77_print_array_1 (int nss, int ndimensions, struct type *type, char *valaddr,
>  		   CORE_ADDR address, struct ui_file *stream, int format,
> -		   int deref_ref, int recurse, enum val_prettyprint pretty)
> +		   int deref_ref, int recurse, enum val_prettyprint pretty,
> +		   int *elts)
>  {
>    int i;
>  
>    if (nss != ndimensions)
>      {
> -      for (i = 0; i < F77_DIM_SIZE (nss); i++)
> +      for (i = 0; (i < F77_DIM_SIZE (nss) && (*elts) < print_max); i++)
>  	{
>  	  fprintf_filtered (stream, "( ");
>  	  f77_print_array_1 (nss + 1, ndimensions, TYPE_TARGET_TYPE (type),
>  			     valaddr + i * F77_DIM_OFFSET (nss),
>  			     address + i * F77_DIM_OFFSET (nss),
> -			     stream, format, deref_ref, recurse, pretty);
> +			     stream, format, deref_ref, recurse, pretty, elts);
>  	  fprintf_filtered (stream, ") ");
>  	}
> +      if (*elts >= print_max && i < F77_DIM_SIZE (nss)) {
> +	fprintf_filtered (stream, "...");
> +      }
>      }
>    else
>      {
> -      for (i = 0; (i < F77_DIM_SIZE (nss) && i < print_max); i++)
> +      for (i = 0; (i < F77_DIM_SIZE (nss) && (*elts) < print_max); i++, (*elts)++)
>  	{
>  	  val_print (TYPE_TARGET_TYPE (type),
>  		     valaddr + i * F77_DIM_OFFSET (ndimensions),
> @@ -306,7 +312,7 @@ f77_print_array_1 (int nss, int ndimensi
>  	  if (i != (F77_DIM_SIZE (nss) - 1))
>  	    fprintf_filtered (stream, ", ");
>  
> -	  if (i == print_max - 1)
> +	  if ((( *elts) == print_max - 1) && (i != (F77_DIM_SIZE (nss) - 1)))
>  	    fprintf_filtered (stream, "...");
>  	}
>      }
> @@ -321,6 +327,7 @@ f77_print_array (struct type *type, char
>  		 enum val_prettyprint pretty)
>  {
>    int ndimensions;
> +  int elts = 0;
>  
>    ndimensions = calc_f77_array_dims (type);
>  
> @@ -335,7 +342,7 @@ f77_print_array (struct type *type, char
>    f77_create_arrayprint_offset_tbl (type, stream);
>  
>    f77_print_array_1 (1, ndimensions, type, valaddr, address, stream, format,
> -		     deref_ref, recurse, pretty);
> +		     deref_ref, recurse, pretty, &elts);
>  }
>  \f
>  
> 

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: Large array printing patch for Fortran

[David Lecomber]

On Mon, Sep 15, 2003 at 11:58:35AM -0400, Daniel Jacobowitz wrote:
>
>Yes, I know that this patch hasn't been reviewed.  That's a shame.  Nag
>the list if you get impatient, but please stop checking things in to
>CVS!  You're listed as a "Write After Approval" maintainer.  If you
>can't wait for approval I'll ask to have you removed from that list.
>
>This was on my list to look at this week, when we were sure 6.0 was in
>the hole.

Hi Daniel,

You reviewed this one May 16th -- I made the typographical corrections
you asked for.  It could be that gdb-6.0 doesn't suffer from the bug
and the fix would be unnecessary (but nonetheless harmless) now.  If
it doesn't work, please roll it back.

I asked another gdb maintainer if I'd got the hang of what the
procedures were - after my naughty commit last week ;-) - and he
confirmed I now understood it.  On not getting a response from the
list, I took that to mean it's okay.  Well, actually he said 'wait for
responses'; I timed-out after 5 days.  My mistake, mea culpa, but I
wasn't given any indication that a reply would occur!

I do have a more complicated fix to post, one that you weren't
convinced about in May, and I'm getting the first one out of the way
before even attempting to explain the new one.


Cheers
David

Re: Large array printing patch for Fortran

[Daniel Jacobowitz]

On Mon, Sep 15, 2003 at 08:50:58PM +0100, David Lecomber wrote:
> On Mon, Sep 15, 2003 at 11:58:35AM -0400, Daniel Jacobowitz wrote:
> >
> >Yes, I know that this patch hasn't been reviewed.  That's a shame.  Nag
> >the list if you get impatient, but please stop checking things in to
> >CVS!  You're listed as a "Write After Approval" maintainer.  If you
> >can't wait for approval I'll ask to have you removed from that list.
> >
> >This was on my list to look at this week, when we were sure 6.0 was in
> >the hole.
> 
> Hi Daniel,
> 
> You reviewed this one May 16th -- I made the typographical corrections
> you asked for.  It could be that gdb-6.0 doesn't suffer from the bug
> and the fix would be unnecessary (but nonetheless harmless) now.  If
> it doesn't work, please roll it back.
> 
> I asked another gdb maintainer if I'd got the hang of what the
> procedures were - after my naughty commit last week ;-) - and he
> confirmed I now understood it.  On not getting a response from the
> list, I took that to mean it's okay.  Well, actually he said 'wait for
> responses'; I timed-out after 5 days.  My mistake, mea culpa, but I
> wasn't given any indication that a reply would occur!

Sorry, but that's life when all the maintainers are busy :(  Just
please don't time-out into committing.

In any case:
  - you left the whitespace change before f77_print_array_1.
  - you still haven't gotten GNU coding style right:

+      if (*elts >= print_max && i < F77_DIM_SIZE (nss)) {
+        fprintf_filtered (stream, "...");
+      }

Should be either:
+      if (*elts >= print_max && i < F77_DIM_SIZE (nss))
+        {
+          fprintf_filtered (stream, "...");
+        }
or
+      if (*elts >= print_max && i < F77_DIM_SIZE (nss))
+        fprintf_filtered (stream, "...");

Probably the latter.

+      for (i = 0; (i < F77_DIM_SIZE (nss) && (*elts) < print_max); i++, (*elts)++)
should be
+      for (i = 0; i < F77_DIM_SIZE (nss) && (*elts) < print_max;
+           i++, (*elts)++)

i.e. watch line length.


+         if ((( *elts) == print_max - 1) && (i != (F77_DIM_SIZE (nss) - 1)))
should be
+         if ((*elts == print_max - 1) && (i != F77_DIM_SIZE (nss) - 1))


That's why I asked you to repost first.  It saves us iterating inside
of CVS.

The patch itself is fine.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: Large array printing patch for Fortran

[David Lecomber]

This patch sets the formatting correctly in the patch I previously
applied.  

2003-09-29  David Lecomber  <dsl@sources.redhat.com>

	* f-valprint.c: Reformatting


Index: f-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/f-valprint.c,v
retrieving revision 1.18
diff -u -p -r1.18 f-valprint.c
--- f-valprint.c	16 Sep 2003 18:56:35 -0000	1.18
+++ f-valprint.c	29 Sep 2003 17:31:59 -0000
@@ -276,6 +276,7 @@ f77_create_arrayprint_offset_tbl (struct
 
 /* Actual function which prints out F77 arrays, Valaddr == address in 
    the superior.  Address == the address in the inferior.  */
+
 static void
 f77_print_array_1 (int nss, int ndimensions, struct type *type, char *valaddr,
 		   CORE_ADDR address, struct ui_file *stream, int format,
@@ -295,13 +296,13 @@ f77_print_array_1 (int nss, int ndimensi
 			     stream, format, deref_ref, recurse, pretty, elts);
 	  fprintf_filtered (stream, ") ");
 	}
-      if (*elts >= print_max && i < F77_DIM_SIZE (nss)) {
+      if (*elts >= print_max && i < F77_DIM_SIZE (nss)) 
 	fprintf_filtered (stream, "...");
-      }
     }
   else
     {
-      for (i = 0; (i < F77_DIM_SIZE (nss) && (*elts) < print_max); i++, (*elts)++)
+      for (i = 0; i < F77_DIM_SIZE (nss) && (*elts) < print_max; 
+	   i++, (*elts)++)
 	{
 	  val_print (TYPE_TARGET_TYPE (type),
 		     valaddr + i * F77_DIM_OFFSET (ndimensions),
@@ -312,7 +313,7 @@ f77_print_array_1 (int nss, int ndimensi
 	  if (i != (F77_DIM_SIZE (nss) - 1))
 	    fprintf_filtered (stream, ", ");
 
-	  if ((( *elts) == print_max - 1) && (i != (F77_DIM_SIZE (nss) - 1)))
+	  if ((*elts == print_max - 1) && (i != (F77_DIM_SIZE (nss) - 1)))
 	    fprintf_filtered (stream, "...");
 	}
     }

Re: Large array printing patch for Fortran

[David Carlton]

On Mon, 29 Sep 2003 18:35:17 +0100, David Lecomber <david@streamline-computing.com> said:

> This patch sets the formatting correctly in the patch I previously
> applied.  

> 2003-09-29  David Lecomber  <dsl@sources.redhat.com>

> 	* f-valprint.c: Reformatting

I'm sorry you find it frustrating that people don't approve your
patches as quickly as they should.  But you still have to wait for
approval before committing them.  I've told you this before, Daniel
has told you this before.  If you don't like waiting, nag people;
don't just go ahead and commit stuff.

There are two things wrong with your ChangeLog entry; I'm in a bad
enough mood that I'm not going to tell you what they are.  Look
through other ChangeLog entries, see how yours is different from
theirs.  (And, while you're at it, look at what's wrong with your
ChangeLog entry from 2003-09-15, too.)  Then, post the diffs here and
WAIT FOR SOMEBODY TO GIVE YOU EXPLICIT APPROVAL TO COMMIT THEM.

David Carlton
carlton@kealia.com

Re: Large array printing patch for Fortran

[Daniel Jacobowitz]

On Mon, Oct 06, 2003 at 09:45:45AM -0700, David Carlton wrote:
> On Mon, 29 Sep 2003 18:35:17 +0100, David Lecomber <david@streamline-computing.com> said:
> 
> > This patch sets the formatting correctly in the patch I previously
> > applied.  
> 
> > 2003-09-29  David Lecomber  <dsl@sources.redhat.com>
> 
> > 	* f-valprint.c: Reformatting
> 
> I'm sorry you find it frustrating that people don't approve your
> patches as quickly as they should.  But you still have to wait for
> approval before committing them.  I've told you this before, Daniel
> has told you this before.  If you don't like waiting, nag people;
> don't just go ahead and commit stuff.
> 
> There are two things wrong with your ChangeLog entry; I'm in a bad
> enough mood that I'm not going to tell you what they are.  Look
> through other ChangeLog entries, see how yours is different from
> theirs.  (And, while you're at it, look at what's wrong with your
> ChangeLog entry from 2003-09-15, too.)  Then, post the diffs here and
> WAIT FOR SOMEBODY TO GIVE YOU EXPLICIT APPROVAL TO COMMIT THEM.

(A) I approved this patch in private.
(B) It's an obvious, withspace only patch so I didn't feel the need to
copy the list when I approved it.
(C) As a write-after-approval maintainer he is allowed to commit truly
obvious patches.  Which this will be once he gets changelog style down.

I'm sorry I didn't post the approval in triplicate to the list.

David L, the problems David C refers to:
 - Generally include the names of modified functions, unless you're
globally touching the whole file.
 - ChangeLog entries end with a full stop.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: Large array printing patch for Fortran

[David Carlton]

On Mon, 6 Oct 2003 12:51:26 -0400, Daniel Jacobowitz <drow@mvista.com> said:

> (A) I approved this patch in private.
> (B) It's an obvious, withspace only patch so I didn't feel the need to
> copy the list when I approved it.
> (C) As a write-after-approval maintainer he is allowed to commit truly
> obvious patches.  Which this will be once he gets changelog style
> down.

My copious apologies in that case.  It's a bad morning. :-(

David Carlton
carlton@kealia.com

Re: Large array printing patch for Fortran

[Andrew Cagney]

> On Mon, 6 Oct 2003 12:51:26 -0400, Daniel Jacobowitz <drow@mvista.com> said:
> 
> 
>> (A) I approved this patch in private.
>> (B) It's an obvious, withspace only patch so I didn't feel the need to
>> copy the list when I approved it.
>> (C) As a write-after-approval maintainer he is allowed to commit truly
>> obvious patches.  Which this will be once he gets changelog style
>> down.
> 
> 
> My copious apologies in that case.  It's a bad morning. :-(

David, your confusion is to be expected.  Daniel, please try to keep all 
approval and patch related discussion on-list.  This helps ensure a 
reasonable level of transparency in the patch approval process.

This is the exact same rationale that goes into asking people to post 
all changes (no matter how "simple") and for people to post final 
versions of patches when revised from the original.

enjoy,
Andrew