[gdb] Fortran dynamic arrays

[gdb] Fortran dynamic arrays

[Tobias Burnus]

Hello all,

I'm badly missing the support for Fortran dynamic arrays in gdb. Jan 
Kratochvil had written a couple of patches which implement this support. 
They had been tested with several compilers, including GCC/gfortran 
(4.3.0+), IBM's xlf and Intel's ifort.

The problem is that the patches are quite bulky and no one stepped 
forward to review them. I'm willing to re-diff them, but only if someone 
is willing to review them.
(Patch history: Initial patch 16 Nov. 2007, updated patch 24 Nov. 2007 
(see links below); pinged for Dec 2007, Jan/Feb/March 2008.)

The original patch is available from:

0: http://sourceware.org/ml/gdb-patches/2007-11/msg00438.html
1: (checked in)
2: http://sourceware.org/ml/gdb-patches/2007-11/msg00440.html
3: http://sourceware.org/ml/gdb-patches/2007-11/msg00441.html
4: http://sourceware.org/ml/gdb-patches/2007-11/msg00442.html
5: http://sourceware.org/ml/gdb-patches/2007-11/msg00443.html
6: http://sourceware.org/ml/gdb-patches/2007-11/msg00444.html

An updated (i.e. rediffed), unsplit version is available from:
http://cvs.fedora.redhat.com/viewcvs/*checkout*/rpms/gdb/devel/gdb-6.8-bz377541-fortran-dynamic-arrays.patch
But I were not surprised if that one needs rediffing as well, regarding 
that URL cf. also
http://sourceware.org/ml/gdb-patches/2008-03/msg00115.html

Tobias

Re: [gdb] Fortran dynamic arrays

[Joel Brobecker]

> The problem is that the patches are quite bulky and no one stepped 
> forward to review them.

I'm sorry about this. I will try to see if I feel more comfortable
reviewing them today than I did back then.  We badly need more
reviewers...

Please send an updated set of patches (and if you haven't done so already,
could you show us how Fortran arrays are described in DWARF?).

-- 
Joel

Re: [gdb] Fortran dynamic arrays

[Ulrich Weigand]

Joel Brobecker wrote:
> > The problem is that the patches are quite bulky and no one stepped 
> > forward to review them.
> 
> I'm sorry about this. I will try to see if I feel more comfortable
> reviewing them today than I did back then.  We badly need more
> reviewers...
> 
> Please send an updated set of patches (and if you haven't done so already,
> could you show us how Fortran arrays are described in DWARF?).

As this is an area I'm interested as well, I'll try to help reviewing
those patches too ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

[patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays]

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 3900 bytes --]

Hi,

On Mon, 18 Aug 2008 13:11:20 +0200, Joel Brobecker wrote:
> could you show us how Fortran arrays are described in DWARF?).

They use regular DWARF BLOCKs.  Just so far GDB supported these blocks only for
DW_AT_location while for Fortran they are used even for other attributes.
There is also new DW_OP_push_object_address, DW_AT_data_location,
DW_AT_associated and DW_AT_allocated.

 <2><6e>: Abbrev Number: 5 (DW_TAG_variable)
    <6f>   DW_AT_name        : (indirect string, offset: 0x63): varv    
    <73>   DW_AT_decl_file   : 1        
    <74>   DW_AT_decl_line   : 23       
    <75>   DW_AT_type        : <0x197>  
    <79>   DW_AT_location    : 3 byte block: 91 f0 7e   (DW_OP_fbreg: -144)
 <1><197>: Abbrev Number: 13 (DW_TAG_array_type)
    <198>   DW_AT_name        : (indirect string, offset: 0x107): array3_real(kind=4)   
    <19c>   DW_AT_ordering    : 1       (column major)
    <19d>   DW_AT_data_location: 2 byte block: 97 6     (DW_OP_push_object_address; DW_OP_deref)
    <1a0>   DW_AT_associated  : 4 byte block: 97 6 30 2e        (DW_OP_push_object_address; DW_OP_deref; DW_OP_lit0; DW_OP_ne)
    <1a5>   DW_AT_type        : <0x190> 
    <1a9>   DW_AT_sibling     : <0x1e4> 
 <2><1ad>: Abbrev Number: 14 (DW_TAG_subrange_type)
    <1ae>   DW_AT_lower_bound : 4 byte block: 97 23 20 6        (DW_OP_push_object_address; DW_OP_plus_uconst: 32; DW_OP_deref)
    <1b3>   DW_AT_upper_bound : 4 byte block: 97 23 28 6        (DW_OP_push_object_address; DW_OP_plus_uconst: 40; DW_OP_deref)
    <1b8>   DW_AT_stride      : 6 byte block: 97 23 18 6 34 1e  (DW_OP_push_object_address; DW_OP_plus_uconst: 24; DW_OP_deref;
 DW_OP_lit4; DW_OP_mul)


The patchset for HEAD as a single file is at:
	http://people.redhat.com/jkratoch/fortran-dynamic-arrays-HEAD-7.patch
(just FYI, not for a review as it is not split)

Attaching its first part - a very useless one on its own - its target is to
get a new storage space for two bits of info (TYPE_BOUND_IS_DWARF_BLOCK and
TYPE_BOUND_IS_DWARF_BLOCK).  I do not think it is the best way but it is
a minimal change without expanding the structure size.

One of such spare space there is `enum array_bound_type'  Except for
BOUND_CANNOT_BE_DETERMINED it is never set anywhere.  BOUND_BY_* are read only
in f-valprint.c.  In fact it could be probably used for the new bits
TYPE_BOUND_IS_DWARF_BLOCK and possibly TYPE_HIGH_BOUND_IS_COUNT but I would
rather like to later remove `enum array_bound_type' and its fields at all.

The patch modifies a lot of sources trying to follow the new style of
main_type bitfields instead of the former FLAGS variable.  I do not think
current main_type layout is right but the patch tries to follow the current
main_type style.  The original version of this patch (in current Fedora
Rawhide) used the FLAGS style access:
+#define TYPE_BOUND_IS_DWARF_BLOCK_MASK 1
+#define TYPE_BOUND_IS_DWARF_BLOCK_VAR(range_type, fieldno) \
+  TYPE_FIELD_STATIC_KIND (range_type, fieldno)
+#define TYPE_HIGH_BOUND_IS_COUNT_MASK 2
+#define TYPE_HIGH_BOUND_IS_COUNT_VAR(range_type) \
+  TYPE_FIELD_STATIC_KIND (range_type, 1)
+         TYPE_BOUND_IS_DWARF_BLOCK_VAR (range_type, 1)
+           |= TYPE_BOUND_IS_DWARF_BLOCK_MASK;
+  if ((TYPE_BOUND_IS_DWARF_BLOCK_VAR (range_type, 0)
+       & TYPE_BOUND_IS_DWARF_BLOCK_MASK) != 0

Also I do not understand why exist all the macros like this one at all:
#define TYPE_MAIN_TYPE(thistype) (thistype)->main_type
Why we cannot use it expanded?  This way it is always one (or more)
"tags"-jump (VIM ctrl-]) indirections while navigating the source files.

In the attached patch FIELD_STATIC_KIND was removed so the pairs like
-      FIELD_BITPOS (*f) = sh->value;
-      FIELD_STATIC_KIND (*f) = 0;
were replaced by
+      SET_FIELD_BITPOS (*f, sh->value);
but still just assignments to FIELD_BITPOS (without an assignment to
FIELD_STATIC_KIND) were left as they are.


Regards,
Jan

[-- Attachment #2: fortran7-bit01.patch --]
[-- Type: text/plain, Size: 9489 bytes --]

2008-09-07  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Split two-bit STATIC_KIND to two separate bitfield single bits.
	* gdbtypes.h (struct main_type <static_kind>): Split to ...
	(struct main_type <bit0>, <bit1>): ... here.
	(FIELD_STATIC_KIND): Split to ...
	(FIELD_BIT0, FIELD_BIT1): ... here.
	(SET_FIELD_BITPOS): New.
	(SET_FIELD_PHYSNAME, SET_FIELD_PHYSADDR, TYPE_FIELD_STATIC)
	(TYPE_FIELD_STATIC_HAS_ADDR): Update for the split.
	(TYPE_FIELD_STATIC_KIND): Remove.
	(TYPE_FIELD_STATIC_HAS_NAME): New.
	* coffread.c, dwarf2read.c, eval.c, gdbtypes.c, mdebugread.c: Update
	throughout.

--- ./gdb/gdbtypes.h	2008-08-26 20:24:20.000000000 +0200
+++ ./gdb/gdbtypes.h	2008-09-07 13:39:11.000000000 +0200
@@ -463,11 +463,14 @@ struct main_type
        user.  */
     unsigned int artificial : 1;
 
-    /* This flag is zero for non-static fields, 1 for fields whose location
-       is specified by the label loc.physname, and 2 for fields whose location
-       is specified by loc.physaddr.  */
+    /* For a field of structure:
+	 BIT0 is 0 and BIT1 is 0: Static field using LOC.BITPOS.
+	 BIT0 is 1 and BIT1 is 0: Dynamic field using LOC.PHYSADDR.
+	 BIT0 is 0 and BIT1 is 1: Dynamic field using LOC.PHYSNAME.
+	 BIT0 is 1 and BIT1 is 1: Undefined.  */
 
-    unsigned int static_kind : 2;
+    unsigned int bit0 : 1;
+    unsigned int bit1 : 1;
 
     /* Size of this field, in bits, or zero if not packed.
        For an unpacked field, the field's type's length
@@ -858,13 +861,16 @@ extern void allocate_cplus_struct_type (
 #define FIELD_BITPOS(thisfld) ((thisfld).loc.bitpos)
 #define FIELD_ARTIFICIAL(thisfld) ((thisfld).artificial)
 #define FIELD_BITSIZE(thisfld) ((thisfld).bitsize)
-#define FIELD_STATIC_KIND(thisfld) ((thisfld).static_kind)
+#define FIELD_BIT0(thisfld) ((thisfld).bit0)
+#define FIELD_BIT1(thisfld) ((thisfld).bit1)
 #define FIELD_PHYSNAME(thisfld) ((thisfld).loc.physname)
 #define FIELD_PHYSADDR(thisfld) ((thisfld).loc.physaddr)
+#define SET_FIELD_BITPOS(thisfld, bitpos) \
+  ((thisfld).bit0 = 0, (thisfld).bit1 = 0, FIELD_PHYSADDR(thisfld) = (bitpos))
+#define SET_FIELD_PHYSADDR(thisfld, addr) \
+  ((thisfld).bit0 = 1, (thisfld).bit1 = 0, FIELD_PHYSADDR(thisfld) = (addr))
 #define SET_FIELD_PHYSNAME(thisfld, name) \
-  ((thisfld).static_kind = 1, FIELD_PHYSNAME(thisfld) = (name))
-#define SET_FIELD_PHYSADDR(thisfld, name) \
-  ((thisfld).static_kind = 2, FIELD_PHYSADDR(thisfld) = (name))
+  ((thisfld).bit0 = 0, (thisfld).bit1 = 1, FIELD_PHYSNAME(thisfld) = (name))
 #define TYPE_FIELD(thistype, n) TYPE_MAIN_TYPE(thistype)->fields[n]
 #define TYPE_FIELD_TYPE(thistype, n) FIELD_TYPE(TYPE_FIELD(thistype, n))
 #define TYPE_FIELD_NAME(thistype, n) FIELD_NAME(TYPE_FIELD(thistype, n))
@@ -903,11 +909,14 @@ extern void allocate_cplus_struct_type (
   (TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits == NULL ? 0 \
     : B_TST(TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits, (n)))
 
-#define TYPE_FIELD_STATIC(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].static_kind != 0)
-#define TYPE_FIELD_STATIC_KIND(thistype, n) TYPE_MAIN_TYPE (thistype)->fields[n].static_kind
-#define TYPE_FIELD_STATIC_HAS_ADDR(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].static_kind == 2)
-#define TYPE_FIELD_STATIC_PHYSNAME(thistype, n) FIELD_PHYSNAME(TYPE_FIELD(thistype, n))
+#define TYPE_FIELD_BIT0(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].bit0)
+#define TYPE_FIELD_BIT1(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].bit1)
+#define TYPE_FIELD_STATIC_HAS_ADDR(thistype, n) TYPE_FIELD_BIT0 (thistype, n)
+#define TYPE_FIELD_STATIC_HAS_NAME(thistype, n) TYPE_FIELD_BIT1 (thistype, n)
+#define TYPE_FIELD_STATIC(thistype, n) (TYPE_FIELD_STATIC_HAS_ADDR (thistype, n) \
+					|| TYPE_FIELD_STATIC_HAS_NAME (thistype, n))
 #define TYPE_FIELD_STATIC_PHYSADDR(thistype, n) FIELD_PHYSADDR(TYPE_FIELD(thistype, n))
+#define TYPE_FIELD_STATIC_PHYSNAME(thistype, n) FIELD_PHYSNAME(TYPE_FIELD(thistype, n))
 
 #define TYPE_FN_FIELDLISTS(thistype) TYPE_CPLUS_SPECIFIC(thistype)->fn_fieldlists
 #define TYPE_FN_FIELDLIST(thistype, n) TYPE_CPLUS_SPECIFIC(thistype)->fn_fieldlists[n]
--- ./gdb/coffread.c	2008-08-26 20:24:18.000000000 +0200
+++ ./gdb/coffread.c	2008-09-07 13:34:35.000000000 +0200
@@ -1943,9 +1943,8 @@ coff_read_struct_type (int index, int le
 	    obsavestring (name, strlen (name), &objfile->objfile_obstack);
 	  FIELD_TYPE (list->field) = decode_type (ms, ms->c_type, &sub_aux,
 						  objfile);
-	  FIELD_BITPOS (list->field) = 8 * ms->c_value;
+	  SET_FIELD_BITPOS (list->field, 8 * ms->c_value);
 	  FIELD_BITSIZE (list->field) = 0;
-	  FIELD_STATIC_KIND (list->field) = 0;
 	  nfields++;
 	  break;
 
@@ -1961,9 +1960,8 @@ coff_read_struct_type (int index, int le
 	    obsavestring (name, strlen (name), &objfile->objfile_obstack);
 	  FIELD_TYPE (list->field) = decode_type (ms, ms->c_type, &sub_aux,
 						  objfile);
-	  FIELD_BITPOS (list->field) = ms->c_value;
+	  SET_FIELD_BITPOS (list->field, ms->c_value);
 	  FIELD_BITSIZE (list->field) = sub_aux.x_sym.x_misc.x_lnsz.x_size;
-	  FIELD_STATIC_KIND (list->field) = 0;
 	  nfields++;
 	  break;
 
@@ -2080,11 +2078,10 @@ coff_read_enum_type (int index, int leng
 	  struct symbol *xsym = syms->symbol[j];
 	  SYMBOL_TYPE (xsym) = type;
 	  TYPE_FIELD_NAME (type, n) = SYMBOL_LINKAGE_NAME (xsym);
-	  TYPE_FIELD_BITPOS (type, n) = SYMBOL_VALUE (xsym);
+	  SET_FIELD_BITPOS (TYPE_FIELD (type, n), SYMBOL_VALUE (xsym));
 	  if (SYMBOL_VALUE (xsym) < 0)
 	    unsigned_enum = 0;
 	  TYPE_FIELD_BITSIZE (type, n) = 0;
-	  TYPE_FIELD_STATIC_KIND (type, n) = 0;
 	}
       if (syms == osyms)
 	break;
--- ./gdb/dwarf2read.c	2008-09-02 00:21:45.000000000 +0200
+++ ./gdb/dwarf2read.c	2008-09-07 13:34:35.000000000 +0200
@@ -3498,8 +3498,6 @@ dwarf2_add_field (struct field_info *fip
       /* Get type of field.  */
       fp->type = die_type (die, cu);
 
-      FIELD_STATIC_KIND (*fp) = 0;
-
       /* Get bit size of field (zero if none).  */
       attr = dwarf2_attr (die, DW_AT_bit_size, cu);
       if (attr)
@@ -3527,10 +3525,10 @@ dwarf2_add_field (struct field_info *fip
           else
             byte_offset = decode_locdesc (DW_BLOCK (attr), cu);
 
-          FIELD_BITPOS (*fp) = byte_offset * bits_per_byte;
+          SET_FIELD_BITPOS (*fp, byte_offset * bits_per_byte);
 	}
       else
-	FIELD_BITPOS (*fp) = 0;
+	SET_FIELD_BITPOS (*fp, 0);
       attr = dwarf2_attr (die, DW_AT_bit_offset, cu);
       if (attr)
 	{
@@ -3622,10 +3620,9 @@ dwarf2_add_field (struct field_info *fip
       /* C++ base class field.  */
       attr = dwarf2_attr (die, DW_AT_data_member_location, cu);
       if (attr)
-	FIELD_BITPOS (*fp) = (decode_locdesc (DW_BLOCK (attr), cu)
-			      * bits_per_byte);
+	SET_FIELD_BITPOS (*fp, decode_locdesc (DW_BLOCK (attr), cu)
+			       * bits_per_byte);
       FIELD_BITSIZE (*fp) = 0;
-      FIELD_STATIC_KIND (*fp) = 0;
       FIELD_TYPE (*fp) = die_type (die, cu);
       FIELD_NAME (*fp) = type_name_no_tag (fp->type);
       fip->nbaseclasses++;
@@ -4343,9 +4340,8 @@ process_enumeration_scope (struct die_in
 
 		  FIELD_NAME (fields[num_fields]) = SYMBOL_LINKAGE_NAME (sym);
 		  FIELD_TYPE (fields[num_fields]) = NULL;
-		  FIELD_BITPOS (fields[num_fields]) = SYMBOL_VALUE (sym);
+		  SET_FIELD_BITPOS (fields[num_fields], SYMBOL_VALUE (sym));
 		  FIELD_BITSIZE (fields[num_fields]) = 0;
-		  FIELD_STATIC_KIND (fields[num_fields]) = 0;
 
 		  num_fields++;
 		}
--- ./gdb/eval.c	2008-08-21 22:13:08.000000000 +0200
+++ ./gdb/eval.c	2008-09-07 13:34:35.000000000 +0200
@@ -319,7 +319,7 @@ evaluate_struct_tuple (struct value *str
 		  fieldno++;
 		  /* Skip static fields.  */
 		  while (fieldno < TYPE_NFIELDS (struct_type)
-			 && TYPE_FIELD_STATIC_KIND (struct_type, fieldno))
+			 && TYPE_FIELD_STATIC (struct_type, fieldno))
 		    fieldno++;
 		  subfieldno = fieldno;
 		  if (fieldno >= TYPE_NFIELDS (struct_type))
--- ./gdb/gdbtypes.c	2008-08-26 20:24:20.000000000 +0200
+++ ./gdb/gdbtypes.c	2008-09-07 13:34:35.000000000 +0200
@@ -3013,11 +3013,8 @@ copy_type_recursive (struct objfile *obj
 				xstrdup (TYPE_FIELD_STATIC_PHYSNAME (type, 
 								     i)));
 	  else
-	    {
-	      TYPE_FIELD_BITPOS (new_type, i) = 
-		TYPE_FIELD_BITPOS (type, i);
-	      TYPE_FIELD_STATIC_KIND (new_type, i) = 0;
-	    }
+	    SET_FIELD_BITPOS (TYPE_FIELD (new_type, i),
+			      TYPE_FIELD_BITPOS (type, i));
 	}
     }
 
--- ./gdb/mdebugread.c	2008-08-26 20:24:20.000000000 +0200
+++ ./gdb/mdebugread.c	2008-09-07 13:34:35.000000000 +0200
@@ -1054,11 +1054,10 @@ parse_symbol (SYMR *sh, union aux_ext *a
 		if (tsym.st != stMember)
 		  break;
 
-		FIELD_BITPOS (*f) = tsym.value;
+		SET_FIELD_BITPOS (*f, tsym.value);
 		FIELD_TYPE (*f) = t;
 		FIELD_NAME (*f) = debug_info->ss + cur_fdr->issBase + tsym.iss;
 		FIELD_BITSIZE (*f) = 0;
-		FIELD_STATIC_KIND (*f) = 0;
 
 		enum_sym = ((struct symbol *)
 			    obstack_alloc (&current_objfile->objfile_obstack,
@@ -1241,11 +1240,10 @@ parse_symbol (SYMR *sh, union aux_ext *a
     case stMember:		/* member of struct or union */
       f = &TYPE_FIELDS (top_stack->cur_type)[top_stack->cur_field++];
       FIELD_NAME (*f) = name;
-      FIELD_BITPOS (*f) = sh->value;
+      SET_FIELD_BITPOS (*f, sh->value);
       bitsize = 0;
       FIELD_TYPE (*f) = parse_type (cur_fd, ax, sh->index, &bitsize, bigend, name);
       FIELD_BITSIZE (*f) = bitsize;
-      FIELD_STATIC_KIND (*f) = 0;
       break;
 
     case stIndirect:		/* forward declaration on Irix5 */

Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays]

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> One of such spare space there is `enum array_bound_type' Except
Jan> for BOUND_CANNOT_BE_DETERMINED it is never set anywhere.
Jan> BOUND_BY_* are read only in f-valprint.c.  In fact it could be
Jan> probably used for the new bits TYPE_BOUND_IS_DWARF_BLOCK and
Jan> possibly TYPE_HIGH_BOUND_IS_COUNT but I would rather like to
Jan> later remove `enum array_bound_type' and its fields at all.

This sounds like a good cleanup to me.

Jan> The patch modifies a lot of sources trying to follow the new
Jan> style of main_type bitfields instead of the former FLAGS
Jan> variable.  I do not think current main_type layout is right but
Jan> the patch tries to follow the current main_type style.

Do you mean the main_type layout after this patch, or before this patch?
If before -- could you be more specific?

Tom

Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays]

[Jan Kratochvil]

On Mon, 08 Sep 2008 17:30:47 +0200, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> 
> Jan> The patch modifies a lot of sources trying to follow the new
> Jan> style of main_type bitfields instead of the former FLAGS
> Jan> variable.  I do not think current main_type layout is right but
> Jan> the patch tries to follow the current main_type style.
> 
> Do you mean the main_type layout after this patch, or before this patch?
> If before -- could you be more specific?

Both before and it gets a bit worse after.  There should be a union with
fields for each very specific TYPE_CODE_*.

TYPE_CODE_ARRAY is terrible - it has always exactly NFIELDS==1 with
fields[0].type->main_type pointing to TYPE_CODE_RANGE having exactly NFIELDS==2
(for the lower and upper bound).  If we try so much to save some main_type
memory:
* fields[0].loc is not used.
* nfields everywhere is not used.
* main_type for TYPE_CODE_RANGE is not used as DW_TAG_subrange_type's
  DW_AT_type is stored into TYPE_CODE_RANGE's TYPE_TARGET_TYPE.
We should have just one TYPE_CODE_ARRAY main_type pointing to the element-type
(DW_TAG_array_type's DW_AT_type - which is right as its target_type) also also
one simple pointer to the index type (DW_TAG_subrange_type's DW_AT_type
- currently stored in TYPE_CODE_RANGE's target_type).

target_type, vptr_fieldno, vptr_basetype and some cases of fields[] is used
only for some types and for other types they are left unused.  These fields
should be moved to the type_specific union.

fields[] with nfields should be moved into the type_specific union for
specific types TYPE_CODE_FUNC, TYPE_CODE_STRUCT and some others.

Simple union would make it all more clear, optimal for the memory size and
even allocatable with variable length.

Some better encapsulation would benefit the dynamic bounds I hacked into the
current code.  There should be a virtual method with MAIN_TYPE containing
generic abstract class type.
class debuginfo_value                  { virtual LONGEST get() = 0; };
class dwarf2_integer : debuginfo_value { virtual LONGEST get() { ... }; };
class dwarf2_block   : debuginfo_value { virtual LONGEST get() { ... }; };
class main_type { ...; debuginfo_value upper_bound_value; ... };
LONGEST is not right as we need to return also unbound/undefined results.
Just C++ requires pointer (or reference) as upper_bound_value which is
inefficient so a C implementation (knowing the maximum size of all the
inherited classes) should be better here, right?

If the memory of main_type is so critical (I doubt so but i did not measure
it) in my Fortran/dynamic-bounds patch main_type had to get three new pointers
almost always being NULL.  These three pointers may need to be set for any
type so the base struct main_type has to be extended for them.
We could instead use the Amiga "tags" data format with variable block size
allocation (but sure slow access time which should not matter here):
struct main_type { ...; void *tags[1]; };
memcpy (main_type->tags,
	{ DW_AT_allocated, 0x601010,
	  DW_AT_associated, 0x610348,
	  NULL /* terminator */
	});


Regards,
Jan

Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays]

[Joel Brobecker]

> One of such spare space there is `enum array_bound_type'  Except for
> BOUND_CANNOT_BE_DETERMINED it is never set anywhere.  BOUND_BY_* are
> read only in f-valprint.c.  In fact it could be probably used for the
> new bits TYPE_BOUND_IS_DWARF_BLOCK and possibly
> TYPE_HIGH_BOUND_IS_COUNT but I would rather like to later remove `enum
> array_bound_type' and its fields at all.

I haven't started looking at the actual patch yet, but hope to get to it
either tonight or tomorrow. But I would approve a separate patch that
garbage-collected these fields. I did verify that, indeed, they are not
used at all.

-- 
Joel

Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays]

[Joel Brobecker]

> Also I do not understand why exist all the macros like this one at all:
> #define TYPE_MAIN_TYPE(thistype) (thistype)->main_type
> Why we cannot use it expanded?  This way it is always one (or more)
> "tags"-jump (VIM ctrl-]) indirections while navigating the source files.

Personally, I find them to be very useful to quickly find who is using
field "main_type" in struct type.

> 2008-09-07  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Split two-bit STATIC_KIND to two separate bitfield single bits.
> 	* gdbtypes.h (struct main_type <static_kind>): Split to ...
> 	(struct main_type <bit0>, <bit1>): ... here.
> 	(FIELD_STATIC_KIND): Split to ...
> 	(FIELD_BIT0, FIELD_BIT1): ... here.
> 	(SET_FIELD_BITPOS): New.
> 	(SET_FIELD_PHYSNAME, SET_FIELD_PHYSADDR, TYPE_FIELD_STATIC)
> 	(TYPE_FIELD_STATIC_HAS_ADDR): Update for the split.
> 	(TYPE_FIELD_STATIC_KIND): Remove.
> 	(TYPE_FIELD_STATIC_HAS_NAME): New.
> 	* coffread.c, dwarf2read.c, eval.c, gdbtypes.c, mdebugread.c: Update
> 	throughout.

It would be interesting to hear what others think, but I don't really
like the idea of splitting the 2-bit static_kind field into two one-bit
fields named bit0 & bit1. I just find it obscures things even more.

The first goal, as I understand it, is to be able to extend range types
so that the bounds can be DWARF blocks. Here is a thought: Rename the
static_kind field into "field_loc_kind" which would be an enum with
4 possible values: loc_in_bitpos, loc_in_physaddr, loc_in_physname,
and loc_in_block.  The names are yet to be decided, but it's just
to give you an idea of what they would mean. 4 possible values mean
that, for now, we have room in the 2-bit field to store it. This
"field_loc_kind" would then be checked for the types were the loc kind
can be ambiguous (range types would be the next addition to the current
list).

Next, we add a new field in the "loc" union that would contain a pointer
to the dwarf block, and probably create some kind of routine to create
these range types. After that, we can update the DWARF reader to handle
DWARF blocks for range type bounds, and finally update Fortran to make
use of them.

What do others think?

-- 
Joel

Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays]

[Joel Brobecker]

> Both before and it gets a bit worse after.  There should be a union with
> fields for each very specific TYPE_CODE_*.

I would tend to agree, but I haven't thought about it much. I might be
biased by the fact that Ada's use of variant record is relatively common.
For instance, our structure implemented in Ada might roughly look like this:


   type GDB_Type_Kind is (Type_Code_Enum, Type_Code_Range, Type_Code_Array, ...
   --  All possible kinds of types.

   type GDB_Type (Kind : GDB_Type_Kind) is
      Boolean : Stub;
      Boolean : Incomplete;
      --  ... Any other field that's common to all types
      case Kind is
         when Type_Code_Enum =>
            Enumerates : List_Of_Enumerates;
         when Type_Code_Range | Type_Code_Int =>
            Low, High : Bound_Type;
         when Type_Code_Array =>
            Element_Type : GDB_Type_Pointer;
            Index : Range_Type_Pointer;
         when ... =>
            [etc]

As you suggest, this is easily achievable in C as well by using unions,
with the one difference is that Ada does some access check and makes sure
that we access fields that are consistent with the discriminant (field
named "Kind" in the example above).

I think that this would help clarify the whole structure, and tell us
what is and what is not used for each kind of type.  But I am not sure
of the drawbacks... For sure this would be a pretty significant change
througout GDB, but maybe an interesting one.

(that was my windy way of saying: I think I agree)

-- 
Joel

Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays]

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 723 bytes --]

On Fri, 19 Sep 2008 08:03:36 +0200, Joel Brobecker wrote:
> > One of such spare space there is `enum array_bound_type'  Except for
> > BOUND_CANNOT_BE_DETERMINED it is never set anywhere.  BOUND_BY_* are
> > read only in f-valprint.c.  In fact it could be probably used for the
> > new bits TYPE_BOUND_IS_DWARF_BLOCK and possibly
> > TYPE_HIGH_BOUND_IS_COUNT but I would rather like to later remove `enum
> > array_bound_type' and its fields at all.
> 
> I haven't started looking at the actual patch yet, but hope to get to it
> either tonight or tomorrow. But I would approve a separate patch that
> garbage-collected these fields. I did verify that, indeed, they are not
> used at all.

Here is the patch.


Thanks,
Jan

[-- Attachment #2: gdb-cvs-nobounds.patch --]
[-- Type: text/plain, Size: 21821 bytes --]

2008-09-22  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Replace TYPE_ARRAY_{UPPER,LOWER}_BOUND_TYPE by a bit if {un,}defined.
	* gdb/c-typeprint.c (c_type_print_varspec_suffix), gdb/m2-typeprint.c
	(m2_array), gdb/p-typeprint.c (pascal_type_print_varspec_prefix),
	gdb/valops.c (value_cast), gdb/varobj.c (c_number_of_children): Replace
	TYPE_ARRAY_UPPER_BOUND_TYPE compared to BOUND_CANNOT_BE_DETERMINED by
	TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED.
	* gdb/parse.c (follow_types): Use TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED.
	* gdb/f-valprint.c (f77_get_dynamic_upperbound): Replace with ...
	(f77_get_upperbound): ... this function handling now only
	TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED.
	(f77_get_dynamic_lowerbound): Replace with ...
	(f77_get_lowerbound): ... this function handling now only
	TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED.
	(f77_get_dynamic_length_of_aggregate, f77_create_arrayprint_offset_tbl):
	Update their callers.
	* gdb/eval.c (evaluate_subexp_standard): Update their callers.
	* gdb/f-lang.h (f77_get_dynamic_upperbound, f77_get_upperbound)
	(f77_get_dynamic_lowerbound, f77_get_lowerbound): Update their
	prototypes.
	(BOUND_FETCH_OK, BOUND_FETCH_ERROR): Remove.
	* gdb/f-typeprint.c (f_type_print_varspec_suffix, f_type_print_base):
	Remove the lower_bound_was_default variable.  Update the
	f77_get_dynamic_upperbound, f77_get_upperbound and
	TYPE_ARRAY_UPPER_BOUND_TYPE calls.
	* gdb/gdbtypes.c (print_bound_type): Remove the function.
	(recursive_dump_type): Replace the print_bound_type calls by simple
	prints.
	* gdb/gdbtypes.h (enum array_bound_type): Remove.
	(struct main_type): Remove the fields upper_bound_type and
	lower_bound_type.  Comment the new overload of the field artificial.
	(TYPE_ARRAY_UPPER_BOUND_TYPE): Replace by ...
	(TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED): ... this macro.
	(TYPE_ARRAY_LOWER_BOUND_TYPE): Replace by ...
	(TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED): ... this macro.
	* gdb/testsuite/gdb.base/break.c (array): New variable.
	(main): Artificially access ARRAY.
	* gdb/testsuite/gdb.base/maint.exp (maint print type): Rename to ...
	(maint print type argc): ... this testcase name, remove its removed
	upper_bound_type and lower_bound_type expectations.
	(maint print type array): New testcase.

--- gdb/c-typeprint.c	3 May 2008 22:20:13 -0000	1.42
+++ gdb/c-typeprint.c	22 Sep 2008 14:55:28 -0000
@@ -542,7 +542,7 @@ c_type_print_varspec_suffix (struct type
 
       fprintf_filtered (stream, "[");
       if (TYPE_LENGTH (TYPE_TARGET_TYPE (type)) > 0
-	&& TYPE_ARRAY_UPPER_BOUND_TYPE (type) != BOUND_CANNOT_BE_DETERMINED)
+	&& !TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type))
 	fprintf_filtered (stream, "%d",
 			  (TYPE_LENGTH (type)
 			   / TYPE_LENGTH (TYPE_TARGET_TYPE (type))));
--- gdb/eval.c	11 Sep 2008 14:27:34 -0000	1.98
+++ gdb/eval.c	22 Sep 2008 14:55:32 -0000
@@ -1994,13 +1994,8 @@ evaluate_subexp_standard (struct type *e
 	/* Internal type of array is arranged right to left */
 	for (i = 0; i < nargs; i++)
 	  {
-	    retcode = f77_get_dynamic_upperbound (tmp_type, &upper);
-	    if (retcode == BOUND_FETCH_ERROR)
-	      error (_("Cannot obtain dynamic upper bound"));
-
-	    retcode = f77_get_dynamic_lowerbound (tmp_type, &lower);
-	    if (retcode == BOUND_FETCH_ERROR)
-	      error (_("Cannot obtain dynamic lower bound"));
+	    upper = f77_get_upperbound (tmp_type);
+	    lower = f77_get_lowerbound (tmp_type);
 
 	    array_size_array[nargs - i - 1] = upper - lower + 1;
 
--- gdb/f-lang.h	11 Sep 2008 14:31:43 -0000	1.12
+++ gdb/f-lang.h	22 Sep 2008 14:55:32 -0000
@@ -83,9 +83,6 @@ extern SAVED_F77_COMMON_PTR find_common_
 #define BLANK_COMMON_NAME_MF77     "__BLNK__"	/* MF77 assigned  */
 #define BLANK_COMMON_NAME_LOCAL    "__BLANK"	/* Local GDB */
 
-#define BOUND_FETCH_OK 1
-#define BOUND_FETCH_ERROR -999
-
 /* When reasonable array bounds cannot be fetched, such as when 
    you ask to 'mt print symbols' and there is no stack frame and 
    therefore no way of knowing the bounds of stack-based arrays, 
@@ -97,9 +94,9 @@ extern SAVED_F77_COMMON_PTR find_common_
 extern char *real_main_name;	/* Name of main function */
 extern int real_main_c_value;	/* C_value field of main function */
 
-extern int f77_get_dynamic_upperbound (struct type *, int *);
+extern int f77_get_upperbound (struct type *);
 
-extern int f77_get_dynamic_lowerbound (struct type *, int *);
+extern int f77_get_lowerbound (struct type *);
 
 extern void f77_get_dynamic_array_length (struct type *);
 
--- gdb/f-typeprint.c	22 Apr 2008 06:53:18 -0000	1.23
+++ gdb/f-typeprint.c	22 Sep 2008 14:55:32 -0000
@@ -153,7 +153,6 @@ f_type_print_varspec_suffix (struct type
 			     int show, int passed_a_ptr, int demangled_args)
 {
   int upper_bound, lower_bound;
-  int lower_bound_was_default = 0;
   static int arrayprint_recurse_level = 0;
   int retcode;
 
@@ -176,35 +175,19 @@ f_type_print_varspec_suffix (struct type
       if (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_ARRAY)
 	f_type_print_varspec_suffix (TYPE_TARGET_TYPE (type), stream, 0, 0, 0);
 
-      retcode = f77_get_dynamic_lowerbound (type, &lower_bound);
-
-      lower_bound_was_default = 0;
-
-      if (retcode == BOUND_FETCH_ERROR)
-	fprintf_filtered (stream, "???");
-      else if (lower_bound == 1)	/* The default */
-	lower_bound_was_default = 1;
-      else
-	fprintf_filtered (stream, "%d", lower_bound);
-
-      if (lower_bound_was_default)
-	lower_bound_was_default = 0;
-      else
-	fprintf_filtered (stream, ":");
+      lower_bound = f77_get_lowerbound (type);
+      if (lower_bound != 1)	/* Not the default.  */
+	fprintf_filtered (stream, "%d:", lower_bound);
 
       /* Make sure that, if we have an assumed size array, we
          print out a warning and print the upperbound as '*' */
 
-      if (TYPE_ARRAY_UPPER_BOUND_TYPE (type) == BOUND_CANNOT_BE_DETERMINED)
+      if (TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type))
 	fprintf_filtered (stream, "*");
       else
 	{
-	  retcode = f77_get_dynamic_upperbound (type, &upper_bound);
-
-	  if (retcode == BOUND_FETCH_ERROR)
-	    fprintf_filtered (stream, "???");
-	  else
-	    fprintf_filtered (stream, "%d", upper_bound);
+	  upper_bound = f77_get_upperbound (type);
+	  fprintf_filtered (stream, "%d", upper_bound);
 	}
 
       if (TYPE_CODE (TYPE_TARGET_TYPE (type)) != TYPE_CODE_ARRAY)
@@ -354,16 +337,12 @@ f_type_print_base (struct type *type, st
     case TYPE_CODE_STRING:
       /* Strings may have dynamic upperbounds (lengths) like arrays. */
 
-      if (TYPE_ARRAY_UPPER_BOUND_TYPE (type) == BOUND_CANNOT_BE_DETERMINED)
+      if (TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type))
 	fprintfi_filtered (level, stream, "character*(*)");
       else
 	{
-	  retcode = f77_get_dynamic_upperbound (type, &upper_bound);
-
-	  if (retcode == BOUND_FETCH_ERROR)
-	    fprintf_filtered (stream, "character*???");
-	  else
-	    fprintf_filtered (stream, "character*%d", upper_bound);
+	  upper_bound = f77_get_upperbound (type);
+	  fprintf_filtered (stream, "character*%d", upper_bound);
 	}
       break;
 
--- gdb/f-valprint.c	11 Sep 2008 14:28:47 -0000	1.45
+++ gdb/f-valprint.c	22 Sep 2008 14:55:32 -0000
@@ -61,132 +61,28 @@ int f77_array_offset_tbl[MAX_FORTRAN_DIM
 #define F77_DIM_OFFSET(n) (f77_array_offset_tbl[n][0])
 
 int
-f77_get_dynamic_lowerbound (struct type *type, int *lower_bound)
+f77_get_lowerbound (struct type *type)
 {
-  struct frame_info *frame;
-  CORE_ADDR current_frame_addr;
-  CORE_ADDR ptr_to_lower_bound;
-
-  switch (TYPE_ARRAY_LOWER_BOUND_TYPE (type))
-    {
-    case BOUND_BY_VALUE_ON_STACK:
-      frame = deprecated_safe_get_selected_frame ();
-      current_frame_addr = get_frame_base (frame);
-      if (current_frame_addr > 0)
-	{
-	  *lower_bound =
-	    read_memory_integer (current_frame_addr +
-				 TYPE_ARRAY_LOWER_BOUND_VALUE (type),
-				 4);
-	}
-      else
-	{
-	  *lower_bound = DEFAULT_LOWER_BOUND;
-	  return BOUND_FETCH_ERROR;
-	}
-      break;
-
-    case BOUND_SIMPLE:
-      *lower_bound = TYPE_ARRAY_LOWER_BOUND_VALUE (type);
-      break;
-
-    case BOUND_CANNOT_BE_DETERMINED:
-      error (_("Lower bound may not be '*' in F77"));
-      break;
+  if (TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED (type))
+    error (_("Lower bound may not be '*' in F77"));
 
-    case BOUND_BY_REF_ON_STACK:
-      frame = deprecated_safe_get_selected_frame ();
-      current_frame_addr = get_frame_base (frame);
-      if (current_frame_addr > 0)
-	{
-	  struct gdbarch *arch = get_frame_arch (frame);
-	  ptr_to_lower_bound =
-	    read_memory_typed_address (current_frame_addr +
-				       TYPE_ARRAY_LOWER_BOUND_VALUE (type),
-				       builtin_type (arch)->builtin_data_ptr);
-	  *lower_bound = read_memory_integer (ptr_to_lower_bound, 4);
-	}
-      else
-	{
-	  *lower_bound = DEFAULT_LOWER_BOUND;
-	  return BOUND_FETCH_ERROR;
-	}
-      break;
-
-    case BOUND_BY_REF_IN_REG:
-    case BOUND_BY_VALUE_IN_REG:
-    default:
-      error (_("??? unhandled dynamic array bound type ???"));
-      break;
-    }
-  return BOUND_FETCH_OK;
+  return TYPE_ARRAY_LOWER_BOUND_VALUE (type);
 }
 
 int
-f77_get_dynamic_upperbound (struct type *type, int *upper_bound)
+f77_get_upperbound (struct type *type)
 {
-  struct frame_info *frame;
-  CORE_ADDR current_frame_addr = 0;
-  CORE_ADDR ptr_to_upper_bound;
-
-  switch (TYPE_ARRAY_UPPER_BOUND_TYPE (type))
-    {
-    case BOUND_BY_VALUE_ON_STACK:
-      frame = deprecated_safe_get_selected_frame ();
-      current_frame_addr = get_frame_base (frame);
-      if (current_frame_addr > 0)
-	{
-	  *upper_bound =
-	    read_memory_integer (current_frame_addr +
-				 TYPE_ARRAY_UPPER_BOUND_VALUE (type),
-				 4);
-	}
-      else
-	{
-	  *upper_bound = DEFAULT_UPPER_BOUND;
-	  return BOUND_FETCH_ERROR;
-	}
-      break;
-
-    case BOUND_SIMPLE:
-      *upper_bound = TYPE_ARRAY_UPPER_BOUND_VALUE (type);
-      break;
-
-    case BOUND_CANNOT_BE_DETERMINED:
-      /* we have an assumed size array on our hands. Assume that 
-         upper_bound == lower_bound so that we show at least 
-         1 element.If the user wants to see more elements, let 
-         him manually ask for 'em and we'll subscript the 
-         array and show him */
-      f77_get_dynamic_lowerbound (type, upper_bound);
-      break;
-
-    case BOUND_BY_REF_ON_STACK:
-      frame = deprecated_safe_get_selected_frame ();
-      current_frame_addr = get_frame_base (frame);
-      if (current_frame_addr > 0)
-	{
-	  struct gdbarch *arch = get_frame_arch (frame);
-	  ptr_to_upper_bound =
-	    read_memory_typed_address (current_frame_addr +
-				       TYPE_ARRAY_UPPER_BOUND_VALUE (type),
-				       builtin_type (arch)->builtin_data_ptr);
-	  *upper_bound = read_memory_integer (ptr_to_upper_bound, 4);
-	}
-      else
-	{
-	  *upper_bound = DEFAULT_UPPER_BOUND;
-	  return BOUND_FETCH_ERROR;
-	}
-      break;
+  if (TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type))
+    {
+      /* We have an assumed size array on our hands.  Assume that
+	 upper_bound == lower_bound so that we show at least 1 element.
+	 If the user wants to see more elements, let him manually ask for 'em
+	 and we'll subscript the array and show him.  */
 
-    case BOUND_BY_REF_IN_REG:
-    case BOUND_BY_VALUE_IN_REG:
-    default:
-      error (_("??? unhandled dynamic array bound type ???"));
-      break;
+      return f77_get_lowerbound (type);
     }
-  return BOUND_FETCH_OK;
+
+  return TYPE_ARRAY_UPPER_BOUND_VALUE (type);
 }
 
 /* Obtain F77 adjustable array dimensions */
@@ -212,13 +108,8 @@ f77_get_dynamic_length_of_aggregate (str
     f77_get_dynamic_length_of_aggregate (TYPE_TARGET_TYPE (type));
 
   /* Recursion ends here, start setting up lengths.  */
-  retcode = f77_get_dynamic_lowerbound (type, &lower_bound);
-  if (retcode == BOUND_FETCH_ERROR)
-    error (_("Cannot obtain valid array lower bound"));
-
-  retcode = f77_get_dynamic_upperbound (type, &upper_bound);
-  if (retcode == BOUND_FETCH_ERROR)
-    error (_("Cannot obtain valid array upper bound"));
+  lower_bound = f77_get_lowerbound (type);
+  upper_bound = f77_get_upperbound (type);
 
   /* Patch in a valid length value. */
 
@@ -241,16 +132,8 @@ f77_create_arrayprint_offset_tbl (struct
 
   while ((TYPE_CODE (tmp_type) == TYPE_CODE_ARRAY))
     {
-      if (TYPE_ARRAY_UPPER_BOUND_TYPE (tmp_type) == BOUND_CANNOT_BE_DETERMINED)
-	fprintf_filtered (stream, "<assumed size array> ");
-
-      retcode = f77_get_dynamic_upperbound (tmp_type, &upper);
-      if (retcode == BOUND_FETCH_ERROR)
-	error (_("Cannot obtain dynamic upper bound"));
-
-      retcode = f77_get_dynamic_lowerbound (tmp_type, &lower);
-      if (retcode == BOUND_FETCH_ERROR)
-	error (_("Cannot obtain dynamic lower bound"));
+      upper = f77_get_upperbound (tmp_type);
+      lower = f77_get_lowerbound (tmp_type);
 
       F77_DIM_SIZE (ndimen) = upper - lower + 1;
 
--- gdb/gdbtypes.c	11 Sep 2008 14:24:27 -0000	1.150
+++ gdb/gdbtypes.c	22 Sep 2008 14:55:34 -0000
@@ -2556,35 +2556,6 @@ print_cplus_stuff (struct type *type, in
     }
 }
 
-static void
-print_bound_type (int bt)
-{
-  switch (bt)
-    {
-    case BOUND_CANNOT_BE_DETERMINED:
-      printf_filtered ("(BOUND_CANNOT_BE_DETERMINED)");
-      break;
-    case BOUND_BY_REF_ON_STACK:
-      printf_filtered ("(BOUND_BY_REF_ON_STACK)");
-      break;
-    case BOUND_BY_VALUE_ON_STACK:
-      printf_filtered ("(BOUND_BY_VALUE_ON_STACK)");
-      break;
-    case BOUND_BY_REF_IN_REG:
-      printf_filtered ("(BOUND_BY_REF_IN_REG)");
-      break;
-    case BOUND_BY_VALUE_IN_REG:
-      printf_filtered ("(BOUND_BY_VALUE_IN_REG)");
-      break;
-    case BOUND_SIMPLE:
-      printf_filtered ("(BOUND_SIMPLE)");
-      break;
-    default:
-      printf_filtered (_("(unknown bound type)"));
-      break;
-    }
-}
-
 static struct obstack dont_print_type_obstack;
 
 void
@@ -2719,14 +2690,13 @@ recursive_dump_type (struct type *type, 
     }
   puts_filtered ("\n");
   printfi_filtered (spaces, "length %d\n", TYPE_LENGTH (type));
-  printfi_filtered (spaces, "upper_bound_type 0x%x ",
-		    TYPE_ARRAY_UPPER_BOUND_TYPE (type));
-  print_bound_type (TYPE_ARRAY_UPPER_BOUND_TYPE (type));
-  puts_filtered ("\n");
-  printfi_filtered (spaces, "lower_bound_type 0x%x ",
-		    TYPE_ARRAY_LOWER_BOUND_TYPE (type));
-  print_bound_type (TYPE_ARRAY_LOWER_BOUND_TYPE (type));
-  puts_filtered ("\n");
+  if (TYPE_CODE (type) == TYPE_CODE_ARRAY)
+    {
+      printfi_filtered (spaces, "upper bound undefined is %d\n",
+			TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type));
+      printfi_filtered (spaces, "lower bound undefined is %d\n",
+			TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED (type));
+    }
   printfi_filtered (spaces, "objfile ");
   gdb_print_host_address (TYPE_OBJFILE (type), gdb_stdout);
   printf_filtered ("\n");
--- gdb/gdbtypes.h	11 Sep 2008 14:31:43 -0000	1.91
+++ gdb/gdbtypes.h	22 Sep 2008 14:55:36 -0000
@@ -316,18 +316,6 @@ enum type_instance_flag_value
 #define TYPE_ADDRESS_CLASS_ALL(t) (TYPE_INSTANCE_FLAGS(t) \
 				   & TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL)
 
-
-/*  Array bound type.  */
-enum array_bound_type
-{
-  BOUND_SIMPLE = 0,
-  BOUND_BY_VALUE_IN_REG,
-  BOUND_BY_REF_IN_REG,
-  BOUND_BY_VALUE_ON_STACK,
-  BOUND_BY_REF_ON_STACK,
-  BOUND_CANNOT_BE_DETERMINED
-};
-
 /* This structure is space-critical.
    Its layout has been tweaked to reduce the space used.  */
 
@@ -337,12 +325,6 @@ struct main_type
 
   ENUM_BITFIELD(type_code) code : 8;
 
-  /* Array bounds.  These fields appear at this location because
-     they pack nicely here.  */
-
-  ENUM_BITFIELD(array_bound_type) upper_bound_type : 4;
-  ENUM_BITFIELD(array_bound_type) lower_bound_type : 4;
-
   /* Flags about this type.  These fields appear at this location
      because they packs nicely here.  See the TYPE_* macros for
      documentation about these fields.  */
@@ -460,7 +442,8 @@ struct main_type
 
     /* For a function or member type, this is 1 if the argument is marked
        artificial.  Artificial arguments should not be shown to the
-       user.  */
+       user.  For TYPE_CODE_RANGE it is set if the specific bound is not
+       defined.  */
     unsigned int artificial : 1;
 
     /* This flag is zero for non-static fields, 1 for fields whose location
@@ -817,10 +800,10 @@ extern void allocate_cplus_struct_type (
 
 /* Moto-specific stuff for FORTRAN arrays */
 
-#define TYPE_ARRAY_UPPER_BOUND_TYPE(thistype) \
-	TYPE_MAIN_TYPE(thistype)->upper_bound_type
-#define TYPE_ARRAY_LOWER_BOUND_TYPE(thistype) \
-	TYPE_MAIN_TYPE(thistype)->lower_bound_type
+#define TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED(arraytype) \
+   (TYPE_FIELD_ARTIFICIAL((TYPE_FIELD_TYPE((arraytype),0)),1))
+#define TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED(arraytype) \
+   (TYPE_FIELD_ARTIFICIAL((TYPE_FIELD_TYPE((arraytype),0)),0))
 
 #define TYPE_ARRAY_UPPER_BOUND_VALUE(arraytype) \
    (TYPE_FIELD_BITPOS((TYPE_FIELD_TYPE((arraytype),0)),1))
--- gdb/m2-typeprint.c	11 Sep 2008 14:22:32 -0000	1.16
+++ gdb/m2-typeprint.c	22 Sep 2008 14:55:36 -0000
@@ -202,7 +202,7 @@ static void m2_array (struct type *type,
 {
   fprintf_filtered (stream, "ARRAY [");
   if (TYPE_LENGTH (TYPE_TARGET_TYPE (type)) > 0
-      && TYPE_ARRAY_UPPER_BOUND_TYPE (type) != BOUND_CANNOT_BE_DETERMINED)
+      && !TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type))
     {
       if (TYPE_INDEX_TYPE (type) != 0)
 	{
--- gdb/p-typeprint.c	11 Sep 2008 14:22:32 -0000	1.22
+++ gdb/p-typeprint.c	22 Sep 2008 14:55:37 -0000
@@ -251,7 +251,7 @@ pascal_type_print_varspec_prefix (struct
 	fprintf_filtered (stream, "(");
       fprintf_filtered (stream, "array ");
       if (TYPE_LENGTH (TYPE_TARGET_TYPE (type)) > 0
-	&& TYPE_ARRAY_UPPER_BOUND_TYPE (type) != BOUND_CANNOT_BE_DETERMINED)
+	&& !TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type))
 	fprintf_filtered (stream, "[%d..%d] ",
 			  TYPE_ARRAY_LOWER_BOUND_VALUE (type),
 			  TYPE_ARRAY_UPPER_BOUND_VALUE (type)
--- gdb/parse.c	11 Sep 2008 14:22:32 -0000	1.79
+++ gdb/parse.c	22 Sep 2008 14:55:37 -0000
@@ -1268,8 +1268,7 @@ follow_types (struct type *follow_type)
 	  create_array_type ((struct type *) NULL,
 			     follow_type, range_type);
 	if (array_size < 0)
-	  TYPE_ARRAY_UPPER_BOUND_TYPE (follow_type)
-	    = BOUND_CANNOT_BE_DETERMINED;
+	  TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (follow_type) = 1;
 	break;
       case tp_function:
 	/* FIXME-type-allocation: need a way to free this type when we are
--- gdb/valops.c	11 Sep 2008 14:27:34 -0000	1.200
+++ gdb/valops.c	22 Sep 2008 14:55:41 -0000
@@ -361,8 +361,7 @@ value_cast (struct type *type, struct va
     {
       struct type *element_type = TYPE_TARGET_TYPE (type);
       unsigned element_length = TYPE_LENGTH (check_typedef (element_type));
-      if (element_length > 0
-	&& TYPE_ARRAY_UPPER_BOUND_TYPE (type) == BOUND_CANNOT_BE_DETERMINED)
+      if (element_length > 0 && TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type))
 	{
 	  struct type *range_type = TYPE_INDEX_TYPE (type);
 	  int val_length = TYPE_LENGTH (type2);
--- gdb/varobj.c	13 Sep 2008 23:37:05 -0000	1.117
+++ gdb/varobj.c	22 Sep 2008 14:55:43 -0000
@@ -1951,7 +1951,7 @@ c_number_of_children (struct varobj *var
     {
     case TYPE_CODE_ARRAY:
       if (TYPE_LENGTH (type) > 0 && TYPE_LENGTH (target) > 0
-	  && TYPE_ARRAY_UPPER_BOUND_TYPE (type) != BOUND_CANNOT_BE_DETERMINED)
+	  && !TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type))
 	children = TYPE_LENGTH (type) / TYPE_LENGTH (target);
       else
 	/* If we don't know how many elements there are, don't display
--- gdb/testsuite/gdb.base/break.c	6 Aug 2008 12:52:07 -0000	1.11
+++ gdb/testsuite/gdb.base/break.c	22 Sep 2008 14:55:46 -0000
@@ -62,6 +62,9 @@ extern void marker3 ();
 extern void marker4 ();
 #endif
 
+/* Test of the array boundaries debug information.  */
+static char array[2];
+
 /*
  *	This simple classical example of recursion is useful for
  *	testing stack backtraces and such.
@@ -96,6 +99,7 @@ char *argv[], **envp;
     /* We're used by a test that requires malloc, so make sure it is
        in the executable.  */
     (void)malloc (1);
+    array[0] = array[1];
 
     argc = (argc == 12345); /* This is silly, but we can step off of it */ /* set breakpoint 2 here */
     return argc;  /* set breakpoint 10 here */
--- gdb/testsuite/gdb.base/maint.exp	24 Aug 2008 16:39:57 -0000	1.35
+++ gdb/testsuite/gdb.base/maint.exp	22 Sep 2008 14:55:48 -0000
@@ -399,13 +399,11 @@ gdb_expect  {
         timeout         { fail "(timeout) maint print symbols" }
         }
 
-send_gdb "maint print type argc\n"
-gdb_expect  {
-        -re "type node $hex\r\nname .int. \\($hex\\)\r\ntagname .<NULL>. \\($hex\\)\r\ncode $hex \\(TYPE_CODE_INT\\)\r\nlength \[24\]\r\nupper_bound_type $hex \\(BOUND_SIMPLE\\)\r\nlower_bound_type $hex \\(BOUND_SIMPLE\\)\r\nobjfile $hex\r\ntarget_type $hex\r\npointer_type $hex\r\nreference_type $hex\r\ntype_chain $hex\r\ninstance_flags $hex\r\nflags\r\nnfields 0 $hex\r\nvptr_basetype $hex\r\nvptr_fieldno -1\r\ntype_specific $hex\r\n$gdb_prompt $"\
-                        { pass "maint print type" }
-        -re ".*$gdb_prompt $"       { fail "maint print type" }
-        timeout         { fail "(timeout) maint print type" }
-        }
+gdb_test "maint print type argc" "type node $hex\r\nname .int. \\($hex\\)\r\ntagname .<NULL>. \\($hex\\)\r\ncode $hex \\(TYPE_CODE_INT\\)\r\nlength \[24\]\r\nobjfile $hex\r\ntarget_type $hex\r\npointer_type $hex\r\nreference_type $hex\r\ntype_chain $hex\r\ninstance_flags $hex\r\nflags\r\nnfields 0 $hex\r\nvptr_basetype $hex\r\nvptr_fieldno -1\r\ntype_specific $hex"
+
+# TYPE_CODE_ARRAY has the additional two fields: `upper bound undefined is %d'
+# and `lower bound undefined is %d'
+gdb_test "maint print type array" "code $hex \\(TYPE_CODE_ARRAY\\)\r\nlength \[2\]\r\nupper bound undefined is 0\r\nlower bound undefined is 0\r\nobjfile $hex.*"
 
 if [istarget "hppa*-*-11*"] {
     setup_xfail hppa*-*-*11* CLLbs14860

Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays]

[Joel Brobecker]

> Here is the patch.

Argh, it took me a long time to break through the patch because
I had missed the fact that the upper-bound was indeed set to
BOUND_CANNOT_BE_DETERMINED once. I think you were pretty clear
about this in your previous message, but my (hopefully then)
fried brain didn't register. I'm sorry I missed that somehow,
because it changes slightly my point of view. To make it up to you,
I will try to help as much as I can.

Anyway, so the patch is using the TYPE_FIELD_ARTIFICIAL bit to
detect range types where a given bound value cannot be determined.
The patch as it is would work, and I wouldn't object to having it
checked in as a small step forward (with the agreement from at least
one other maintainer). However, I think we can implement this slightly
differently to make it more general.

struct field has the following two components:

    /* For a function or member type, this is 1 if the argument is marked
       artificial.  Artificial arguments should not be shown to the
       user.  */
    unsigned int artificial : 1;

    /* This flag is zero for non-static fields, 1 for fields whose location
       is specified by the label loc.physname, and 2 for fields whose location
       is specified by loc.physaddr.  */

    unsigned int static_kind : 2;

In practice, these are really what we call "discriminants" in Ada, which
means a value that we read in order to know what field to use in the
"loc" union. I propose that, instead of re-using the "artificial" flag
for undefined bounds, we merge the two components "artificial" and
"static_kind" together into a 3-bit field whose value would be an enum
that tells us what field to use for our field loc.

Because this field would be 3-bit long, it would allow us to have 7
different possible fields, in addition to the case where the bound
is undefined or the argument is artificial.  That would leave more
than enough room to then add a DWARF block as a possible field loc.

What does everyone think?

Some comments on your patch, that I did review pretty carefully:

> 2008-09-22  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Replace TYPE_ARRAY_{UPPER,LOWER}_BOUND_TYPE by a bit if {un,}defined.
> 	* gdb/c-typeprint.c (c_type_print_varspec_suffix), gdb/m2-typeprint.c
> 	(m2_array), gdb/p-typeprint.c (pascal_type_print_varspec_prefix),
> 	gdb/valops.c (value_cast), gdb/varobj.c (c_number_of_children): Replace
> 	TYPE_ARRAY_UPPER_BOUND_TYPE compared to BOUND_CANNOT_BE_DETERMINED by
> 	TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED.
[...]

I am not sure about the reason why you listed all the filenames with
the gdb/ subdirectory in it, but the gdb/ subdir has its own ChangeLog,
so it shouldn't be part of the file name. Perhaps it's because you wanted
to make it clear that the changes are in the gdb/ directory.  Usually,
it's obvious enough that I don't bother.  When I make changes to more
than one directory such that it will cause more than one ChangeLog to
be updated, I sometimes write something like this:

    gdb/
    2008-09-22  Joel Brobecker <....>
    [...]

    gdb/testsuite/
    2008-09-22  Joel Brobecker <...>

Most of the time, I don't even bother, as it is clear enough.  I do
make sure to write each ChangeLog separately as shown above, though.
I noticed that you grouped all your changes, including your testsuite
changes into one CL entry.

> -      retcode = f77_get_dynamic_lowerbound (type, &lower_bound);
> -
> -      lower_bound_was_default = 0;
> -
> -      if (retcode == BOUND_FETCH_ERROR)
> -	fprintf_filtered (stream, "???");
> -      else if (lower_bound == 1)	/* The default */
> -	lower_bound_was_default = 1;
> -      else
> -	fprintf_filtered (stream, "%d", lower_bound);
> -
> -      if (lower_bound_was_default)
> -	lower_bound_was_default = 0;
> -      else
> -	fprintf_filtered (stream, ":");
> +      lower_bound = f77_get_lowerbound (type);
> +      if (lower_bound != 1)	/* Not the default.  */
> +	fprintf_filtered (stream, "%d:", lower_bound);

Here, it looks like  we're slightly modifying the behavior - if the lower
bound was undefined, then we're throwing an error when we used not to.
I think that would be unfriendly if this issue is caused by bad debug
info. Should we check that the lower bound is not undefined first, and
print "???" if it is?

> @@ -2719,14 +2690,13 @@ recursive_dump_type (struct type *type, 
>      }
>    puts_filtered ("\n");
>    printfi_filtered (spaces, "length %d\n", TYPE_LENGTH (type));
> -  printfi_filtered (spaces, "upper_bound_type 0x%x ",
> -		    TYPE_ARRAY_UPPER_BOUND_TYPE (type));
> -  print_bound_type (TYPE_ARRAY_UPPER_BOUND_TYPE (type));
> -  puts_filtered ("\n");
> -  printfi_filtered (spaces, "lower_bound_type 0x%x ",
> -		    TYPE_ARRAY_LOWER_BOUND_TYPE (type));
> -  print_bound_type (TYPE_ARRAY_LOWER_BOUND_TYPE (type));
> -  puts_filtered ("\n");
> +  if (TYPE_CODE (type) == TYPE_CODE_ARRAY)
> +    {
> +      printfi_filtered (spaces, "upper bound undefined is %d\n",
> +			TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type));
> +      printfi_filtered (spaces, "lower bound undefined is %d\n",
> +			TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED (type));
> +    }

I think this is only repetitive, and shouldn't be displayed. The
artificial field value will be displayed a little later, and that
should be good enough.  No strong objection, though.

-- 
Joel

Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays]

[Jan Kratochvil]

On Wed, 24 Sep 2008 21:15:04 +0200, Joel Brobecker wrote:
> struct field has the following two components:
> 
>     /* For a function or member type, this is 1 if the argument is marked
>        artificial.  Artificial arguments should not be shown to the
>        user.  */
>     unsigned int artificial : 1;
> 
>     /* This flag is zero for non-static fields, 1 for fields whose location
>        is specified by the label loc.physname, and 2 for fields whose location
>        is specified by loc.physaddr.  */
> 
>     unsigned int static_kind : 2;
> 
> In practice, these are really what we call "discriminants" in Ada, which
> means a value that we read in order to know what field to use in the
> "loc" union.

I agree "static_kind" is "discriminant" but not "artificial".  Artificial is
just additional flag existing only for FIELDS of type TYPE_CODE_FUNC or type
TYPE_CODE_METHOD:
                                    FIELDs: artificial static_kind!=0
TYPE_CODE_ENUM                              no         no
TYPE_CODE_FUNC or TYPE_CODE_METHOD          yes        no
TYPE_CODE_STRUCT                            yes(_vptr) yes(static field)

Somehow I do not see how these two items are related - just each of them
exists for some types and does not exist for some other types.

Moreover the split into the big union gets more complicated when we merge
them.  Should I really rework the patch into the enum merging
artificial+static_kind?


> What does everyone think?

I expect MAIN_TYPE may get reworked into a big union with per-type fields and
all the bit-size alignment gets different etc.  But otherwise I accept any
decision on the layout of these.


> > 2008-09-22  Jan Kratochvil  <jan.kratochvil@redhat.com>
> > 
> > 	Replace TYPE_ARRAY_{UPPER,LOWER}_BOUND_TYPE by a bit if {un,}defined.
> > 	* gdb/c-typeprint.c (c_type_print_varspec_suffix), gdb/m2-typeprint.c
...
> I am not sure about the reason why you listed all the filenames with
> the gdb/ subdirectory in it,

Sorry, a typo from the ChangeLog script
  http://sourceware.org/ml/gdb-patches/2006-08/msg00195.html
, put there at least a warning now:
  warn "Extra directory" if $dfile=~m{(?:gdb|bfd|opcodes|libiberty)/};


> I do make sure to write each ChangeLog separately as shown above, though.
> I noticed that you grouped all your changes, including your testsuite
> changes into one CL entry.

Sorry, also a typo, already written the split way many types:
http://sourceware.org/ml/gdb-patches/2008-07/msg00145.html
I put there the directory prefixes sometimes only in the CVS commits:
http://sourceware.org/ml/gdb-cvs/2008-07/msg00048.html


> > -      retcode = f77_get_dynamic_lowerbound (type, &lower_bound);
> > -
> > -      lower_bound_was_default = 0;
> > -
> > -      if (retcode == BOUND_FETCH_ERROR)
> > -	fprintf_filtered (stream, "???");
> > -      else if (lower_bound == 1)	/* The default */
> > -	lower_bound_was_default = 1;
> > -      else
> > -	fprintf_filtered (stream, "%d", lower_bound);
> > -
> > -      if (lower_bound_was_default)
> > -	lower_bound_was_default = 0;
> > -      else
> > -	fprintf_filtered (stream, ":");
> > +      lower_bound = f77_get_lowerbound (type);
> > +      if (lower_bound != 1)	/* Not the default.  */
> > +	fprintf_filtered (stream, "%d:", lower_bound);
> 
> Here, it looks like  we're slightly modifying the behavior - if the lower
> bound was undefined, then we're throwing an error when we used not to.

That was a dead code.  BOUND_FETCH_ERROR could never have been returned by
f77_get_dynamic_lowerbound():
BOUND_SIMPLE returns BOUND_FETCH_OK.
BOUND_CANNOT_BE_DETERMINED already error()s itself and never returns.
Other BOUND_* cases were never set.

Another question is if BOUND_CANNOT_BE_DETERMINED should error() as currently
does or if it rather should return BOUND_FETCH_ERROR.  But that decision is
outside of the scope of my patch and changes the current GDB behavior.


> > @@ -2719,14 +2690,13 @@ recursive_dump_type (struct type *type, 
> >      }
> >    puts_filtered ("\n");
> >    printfi_filtered (spaces, "length %d\n", TYPE_LENGTH (type));
> > -  printfi_filtered (spaces, "upper_bound_type 0x%x ",
> > -		    TYPE_ARRAY_UPPER_BOUND_TYPE (type));
> > -  print_bound_type (TYPE_ARRAY_UPPER_BOUND_TYPE (type));
> > -  puts_filtered ("\n");
> > -  printfi_filtered (spaces, "lower_bound_type 0x%x ",
> > -		    TYPE_ARRAY_LOWER_BOUND_TYPE (type));
> > -  print_bound_type (TYPE_ARRAY_LOWER_BOUND_TYPE (type));
> > -  puts_filtered ("\n");
> > +  if (TYPE_CODE (type) == TYPE_CODE_ARRAY)
> > +    {
> > +      printfi_filtered (spaces, "upper bound undefined is %d\n",
> > +			TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type));
> > +      printfi_filtered (spaces, "lower bound undefined is %d\n",
> > +			TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED (type));
> > +    }
> 
> I think this is only repetitive, and shouldn't be displayed. The
> artificial field value will be displayed a little later, and that
> should be good enough.  No strong objection, though.

As I hope the "artificial" field may start to exist only for some types
I tried to make the patch more abstract to the specific implementation of
TYPE_ARRAY_{UPPER,LOWER}_BOUND_IS_UNDEFINED.  But no strong opinion on either
way.



Thanks,
Jan

Accessor macro wrappers removal [Re: [patch] static_kind -> bit0, bit1]

[Jan Kratochvil]

On Sat, 20 Sep 2008 00:12:21 +0200, Joel Brobecker wrote:
> > Also I do not understand why exist all the macros like this one at all:
> > #define TYPE_MAIN_TYPE(thistype) (thistype)->main_type
> > Why we cannot use it expanded?  This way it is always one (or more)
> > "tags"-jump (VIM ctrl-]) indirections while navigating the source files.
> 
> Personally, I find them to be very useful to quickly find who is using
> field "main_type" in struct type.

How does it differ from
  grep -- '->main_type\>' *.[ch]
?

[ other part of the mail left after update of the patch ]


Regards,
Jan

Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays]

[Daniel Jacobowitz]

On Wed, Sep 24, 2008 at 12:15:04PM -0700, Joel Brobecker wrote:
> Anyway, so the patch is using the TYPE_FIELD_ARTIFICIAL bit to
> detect range types where a given bound value cannot be determined.
> The patch as it is would work, and I wouldn't object to having it
> checked in as a small step forward (with the agreement from at least
> one other maintainer). However, I think we can implement this slightly
> differently to make it more general.

In my opinion, we should go with Jan's patch.  I don't think that
artificial is a discriminant; it has a clear meaning as a flag and
nothing to do with the location or the loc union.

What GCC does in this case is have two accessor macros for the same
flag bit, so that it is clear at the usage site what is meant by
checking the bit.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Accessor macro wrappers removal [Re: [patch] static_kind -> bit0, bit1]

[Daniel Jacobowitz]

On Fri, Sep 26, 2008 at 06:43:09AM +0200, Jan Kratochvil wrote:
> On Sat, 20 Sep 2008 00:12:21 +0200, Joel Brobecker wrote:
> > > Also I do not understand why exist all the macros like this one at all:
> > > #define TYPE_MAIN_TYPE(thistype) (thistype)->main_type
> > > Why we cannot use it expanded?  This way it is always one (or more)
> > > "tags"-jump (VIM ctrl-]) indirections while navigating the source files.
> > 
> > Personally, I find them to be very useful to quickly find who is using
> > field "main_type" in struct type.
> 
> How does it differ from
>   grep -- '->main_type\>' *.[ch]
> ?

"in struct type" is the difference.

For main_type it's not too bad.  But for e.g. TYPE_FIELD_TYPE you can
see the problem; there's lots of things that have a type.  If you're
looking just for the ones that come from a "struct field", it can be
pretty awful to locate them all.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays]

[Daniel Jacobowitz]

On Fri, Sep 19, 2008 at 03:12:21PM -0700, Joel Brobecker wrote:
> Next, we add a new field in the "loc" union that would contain a pointer
> to the dwarf block, and probably create some kind of routine to create
> these range types. After that, we can update the DWARF reader to handle
> DWARF blocks for range type bounds, and finally update Fortran to make
> use of them.
> 
> What do others think?

Agreed.  I would suggest LOC_COMPUTED as the role model, except I
don't see where to stash an ops pointer.  Maybe in TYPE_SPECIFIC, or
is that ugly?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays]

[Joel Brobecker]

> I agree "static_kind" is "discriminant" but not "artificial".  Artificial is
> just additional flag existing only for FIELDS of type TYPE_CODE_FUNC or type
> TYPE_CODE_METHOD:

Yes, I don't know why I was thinking that artificial was a discriminant
(meaning "no location").

> Moreover the split into the big union gets more complicated when we merge
> them.  Should I really rework the patch into the enum merging
> artificial+static_kind?

No that won't be necessary. I agree with Daniel that we should go with
your patch for now.  Improving the clarity of the discriminant can be
done separately.

> Sorry, a typo from the ChangeLog script
>   http://sourceware.org/ml/gdb-patches/2006-08/msg00195.html
> , put there at least a warning now:
>   warn "Extra directory" if $dfile=~m{(?:gdb|bfd|opcodes|libiberty)/};

No problem, please just remember to fix these before you commit.

> > > +      lower_bound = f77_get_lowerbound (type);
> > > +      if (lower_bound != 1)	/* Not the default.  */
> > > +	fprintf_filtered (stream, "%d:", lower_bound);
> > 
> > Here, it looks like  we're slightly modifying the behavior - if the lower
> > bound was undefined, then we're throwing an error when we used not to.
> 
> That was a dead code.  BOUND_FETCH_ERROR could never have been returned by
> f77_get_dynamic_lowerbound():
> BOUND_SIMPLE returns BOUND_FETCH_OK.
> BOUND_CANNOT_BE_DETERMINED already error()s itself and never returns.
> Other BOUND_* cases were never set.

Hmmm, you are right! So that piece was fine too.

> Another question is if BOUND_CANNOT_BE_DETERMINED should error() as
> currently does or if it rather should return BOUND_FETCH_ERROR.  But
> that decision is outside of the scope of my patch and changes the
> current GDB behavior.

Absolutely.

> > > +  if (TYPE_CODE (type) == TYPE_CODE_ARRAY)
> > > +    {
> > > +      printfi_filtered (spaces, "upper bound undefined is %d\n",
> > > +			TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type));
> > > +      printfi_filtered (spaces, "lower bound undefined is %d\n",
> > > +			TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED (type));
> > > +    }
> > 
> > I think this is only repetitive, and shouldn't be displayed. The
> > artificial field value will be displayed a little later, and that
> > should be good enough.  No strong objection, though.
> 
> As I hope the "artificial" field may start to exist only for some types
> I tried to make the patch more abstract to the specific implementation of
> TYPE_ARRAY_{UPPER,LOWER}_BOUND_IS_UNDEFINED.  But no strong opinion on
> either way.

That's only my opinion too, and there is no real reason why my opinion
should count too. But to me, this function does a low-level dump of
our type structure, and thus should follow closely the implementation.
If we start to introduce a higher-level view of this type, I think
it can potentially be confusing.  For now, I would prefer to leave
that part out.

Your patch is approved with the few corrections I mentioned.
Thank you.

-- 
Joel

Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays]

[Joel Brobecker]

> In my opinion, we should go with Jan's patch.  I don't think that
> artificial is a discriminant; it has a clear meaning as a flag and
> nothing to do with the location or the loc union.

Agreed. Just a thought (independent from this patch): If this is
a flag, should we try moving this flag up together with the rest
of the flags?

-- 
Joel

Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays]

[Daniel Jacobowitz]

On Fri, Sep 26, 2008 at 03:14:32PM -0700, Joel Brobecker wrote:
> > In my opinion, we should go with Jan's patch.  I don't think that
> > artificial is a discriminant; it has a clear meaning as a flag and
> > nothing to do with the location or the loc union.
> 
> Agreed. Just a thought (independent from this patch): If this is
> a flag, should we try moving this flag up together with the rest
> of the flags?

Aren't the others on the type?  This really is a flag on the field.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays]

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

[ type rearrangements ]

Joel> As you suggest, this is easily achievable in C as well by using
Joel> unions, with the one difference is that Ada does some access
Joel> check and makes sure that we access fields that are consistent
Joel> with the discriminant (field named "Kind" in the example above).

GCC accomplishes this using checking in the accessor macros.
For the most part this works well -- it certainly catches bugs.

Joel> I think that this would help clarify the whole structure, and tell us
Joel> what is and what is not used for each kind of type.  But I am not sure
Joel> of the drawbacks... For sure this would be a pretty significant change
Joel> througout GDB, but maybe an interesting one.

Joel> (that was my windy way of saying: I think I agree)

I think it would be a good idea provided that there's some data
indicating that the space savings is worthwhile.  I'm not sure what
would qualify here.

Tom

Re: Accessor macro wrappers removal [Re: [patch] static_kind -> bit0, bit1]

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

[accessor macros]

Joel> Personally, I find them to be very useful to quickly find who is using
Joel> field "main_type" in struct type.

Jan> How does it differ from
Jan>   grep -- '->main_type\>' *.[ch]
Jan> ?

Like Daniel said, this is a pain if unrelated fields have the same
name.  Also it is a pain if there are formatting oddities.

However, if I am really desperate I just rename the field I want to
find to something odd, then "make -k".  The compiler will tell me all
(or most) of the spots it is used :-)


One thing that is nice about the accessor macros is that it makes it
much simpler to play with certain kinds of field rearrangements.  This
isn't common, of course, but I did do it recently.  It turns out you
can push fields into struct general_symbol_info and save a word from
minimal_symbol and symbol.  Trying this out was easy since I only had
to modify a few macros (well, once I fixed the code not using them :-)


Anyway, I don't really care either way about this stuff.  It seems to
be a GNU cultural thing more than anything technical.

Tom

Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays]

[Joel Brobecker]

> Joel> (that was my windy way of saying: I think I agree)
> 
> I think it would be a good idea provided that there's some data
> indicating that the space savings is worthwhile.  I'm not sure what
> would qualify here.

I actually wasn't thinking about space savings, just code clarity.
If we can save space too, great. But I think the first goal in itself
is worthwhile.

-- 
Joel

Re: Accessor macro wrappers removal [Re: [patch] static_kind -> bit0, bit1]

[Jan Kratochvil]

On Fri, 26 Sep 2008 14:54:22 +0200, Daniel Jacobowitz wrote:
> On Fri, Sep 26, 2008 at 06:43:09AM +0200, Jan Kratochvil wrote:
> > On Sat, 20 Sep 2008 00:12:21 +0200, Joel Brobecker wrote:
> > > > Also I do not understand why exist all the macros like this one at all:
> > > > #define TYPE_MAIN_TYPE(thistype) (thistype)->main_type
> > > > Why we cannot use it expanded?  This way it is always one (or more)
> > > > "tags"-jump (VIM ctrl-]) indirections while navigating the source files.
> > > 
> > > Personally, I find them to be very useful to quickly find who is using
> > > field "main_type" in struct type.
> > 
> > How does it differ from
> >   grep -- '->main_type\>' *.[ch]
> > ?
> 
> "in struct type" is the difference.

* Either to use static getters/setters (Gnome):
  struct main_type *type_main_type_get (struct type *type);

* or in the C++ way (optional get/set prefixes instead of the overloading):
  class type {
    private: struct main_type *_main_type;
    public: struct main_type *main_type () { return _main_type; }
    public: void main_type (struct main_type *v) { _main_type = v; }
  };
  One can get here even the automatic reference counting / memory management.

* or to use semiunique fields prefix:
  struct type { struct main_type *tp_main_type; ... };


> For main_type it's not too bad.  But for e.g. TYPE_FIELD_TYPE you can
> see the problem; there's lots of things that have a type.  If you're
> looking just for the ones that come from a "struct field", it can be
> pretty awful to locate them all.

There is not the mess of mixed prefix (macros) vs. postfix (fields):
xml-tdesc.c:
if (TYPE_VECTOR (TYPE_FIELD_TYPE (data->current_union, i)))

It gets expanded to:
if ((((((data->current_union)->main_type->fields[i]).type))->main_type->flag_vector))
if (data->current_union->main_type->fields[i].type->main_type->flag_vector)

Which could be simplified with C++ (do you also find it more clear?):
if (data->current_union->fields()[i]->type()->flag_vector())


Regards,
Jan

Re: Accessor macro wrappers removal [Re: [patch] static_kind -> bit0, bit1]

[Daniel Jacobowitz]

On Thu, Oct 02, 2008 at 10:57:22PM +0200, Jan Kratochvil wrote:
> > "in struct type" is the difference.

[list of other ways to do the same thing]

But this is the one that GDB uses.  If you think there's a good
reason to switch, propose patches, but I don't see a point.

-- 
Daniel Jacobowitz
CodeSourcery

[patch] Fortran obsolete bounds type [Re: [patch] static_kind -> bit0, bit1]

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 869 bytes --]

On Sat, 27 Sep 2008 00:11:19 +0200, Joel Brobecker wrote:
> > > > +  if (TYPE_CODE (type) == TYPE_CODE_ARRAY)
> > > > +    {
> > > > +      printfi_filtered (spaces, "upper bound undefined is %d\n",
> > > > +			TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type));
> > > > +      printfi_filtered (spaces, "lower bound undefined is %d\n",
> > > > +			TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED (type));
> > > > +    }
...
> That's only my opinion too, and there is no real reason why my opinion
> should count too. But to me, this function does a low-level dump of
> our type structure, and thus should follow closely the implementation.
> If we start to introduce a higher-level view of this type, I think
> it can potentially be confusing.  For now, I would prefer to leave
> that part out.

Removed.


Committed.
http://sourceware.org/ml/gdb-cvs/2008-10/msg00019.html


Thanks,
Jan

[-- Attachment #2: gdb-cvs-nobounds2.patch --]
[-- Type: text/plain, Size: 19339 bytes --]

gdb/
2008-10-02  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Replace TYPE_ARRAY_{UPPER,LOWER}_BOUND_TYPE by a bit if {un,}defined.
	* c-typeprint.c (c_type_print_varspec_suffix), m2-typeprint.c
	(m2_array), p-typeprint.c (pascal_type_print_varspec_prefix),
	valops.c (value_cast), varobj.c (c_number_of_children): Replace
	TYPE_ARRAY_UPPER_BOUND_TYPE compared to BOUND_CANNOT_BE_DETERMINED by
	TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED.
	* parse.c (follow_types): Use TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED.
	* f-valprint.c (f77_get_dynamic_upperbound): Replace with ...
	(f77_get_upperbound): ... this function handling now only
	TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED.
	(f77_get_dynamic_lowerbound): Replace with ...
	(f77_get_lowerbound): ... this function handling now only
	TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED.
	(f77_get_dynamic_length_of_aggregate, f77_create_arrayprint_offset_tbl):
	Update their callers.
	* eval.c (evaluate_subexp_standard): Update their callers.
	* f-lang.h (f77_get_dynamic_upperbound, f77_get_upperbound)
	(f77_get_dynamic_lowerbound, f77_get_lowerbound): Update their
	prototypes.
	(BOUND_FETCH_OK, BOUND_FETCH_ERROR): Remove.
	* f-typeprint.c (f_type_print_varspec_suffix, f_type_print_base): Remove
	the lower_bound_was_default variable.  Update the
	f77_get_dynamic_upperbound, f77_get_upperbound and
	TYPE_ARRAY_UPPER_BOUND_TYPE calls.
	* gdbtypes.c (print_bound_type): Remove the function.
	(recursive_dump_type): Remove its calls printing UPPER_BOUND_TYPE and
	LOWER_BOUND_TYPE.
	* gdbtypes.h (enum array_bound_type): Remove.
	(struct main_type): Remove the fields upper_bound_type and
	lower_bound_type.  Comment the new overload of the field artificial.
	(TYPE_ARRAY_UPPER_BOUND_TYPE): Replace by ...
	(TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED): ... this macro.
	(TYPE_ARRAY_LOWER_BOUND_TYPE): Replace by ...
	(TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED): ... this macro.

gdb/testsuite/
2008-10-02  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/maint.exp (maint print type): Remove printing
	UPPER_BOUND_TYPE and LOWER_BOUND_TYPE.

--- src/gdb/c-typeprint.c	2008/09/27 21:29:29	1.43
+++ src/gdb/c-typeprint.c	2008/10/02 22:06:07	1.44
@@ -560,7 +560,7 @@
 
       fprintf_filtered (stream, "[");
       if (TYPE_LENGTH (TYPE_TARGET_TYPE (type)) > 0
-	&& TYPE_ARRAY_UPPER_BOUND_TYPE (type) != BOUND_CANNOT_BE_DETERMINED)
+	&& !TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type))
 	fprintf_filtered (stream, "%d",
 			  (TYPE_LENGTH (type)
 			   / TYPE_LENGTH (TYPE_TARGET_TYPE (type))));
--- src/gdb/eval.c	2008/09/11 14:27:34	1.98
+++ src/gdb/eval.c	2008/10/02 22:06:07	1.99
@@ -1994,13 +1994,8 @@
 	/* Internal type of array is arranged right to left */
 	for (i = 0; i < nargs; i++)
 	  {
-	    retcode = f77_get_dynamic_upperbound (tmp_type, &upper);
-	    if (retcode == BOUND_FETCH_ERROR)
-	      error (_("Cannot obtain dynamic upper bound"));
-
-	    retcode = f77_get_dynamic_lowerbound (tmp_type, &lower);
-	    if (retcode == BOUND_FETCH_ERROR)
-	      error (_("Cannot obtain dynamic lower bound"));
+	    upper = f77_get_upperbound (tmp_type);
+	    lower = f77_get_lowerbound (tmp_type);
 
 	    array_size_array[nargs - i - 1] = upper - lower + 1;
 
--- src/gdb/f-lang.h	2008/09/11 14:31:43	1.12
+++ src/gdb/f-lang.h	2008/10/02 22:06:07	1.13
@@ -83,9 +83,6 @@
 #define BLANK_COMMON_NAME_MF77     "__BLNK__"	/* MF77 assigned  */
 #define BLANK_COMMON_NAME_LOCAL    "__BLANK"	/* Local GDB */
 
-#define BOUND_FETCH_OK 1
-#define BOUND_FETCH_ERROR -999
-
 /* When reasonable array bounds cannot be fetched, such as when 
    you ask to 'mt print symbols' and there is no stack frame and 
    therefore no way of knowing the bounds of stack-based arrays, 
@@ -97,9 +94,9 @@
 extern char *real_main_name;	/* Name of main function */
 extern int real_main_c_value;	/* C_value field of main function */
 
-extern int f77_get_dynamic_upperbound (struct type *, int *);
+extern int f77_get_upperbound (struct type *);
 
-extern int f77_get_dynamic_lowerbound (struct type *, int *);
+extern int f77_get_lowerbound (struct type *);
 
 extern void f77_get_dynamic_array_length (struct type *);
 
--- src/gdb/f-typeprint.c	2008/04/22 06:53:18	1.23
+++ src/gdb/f-typeprint.c	2008/10/02 22:06:07	1.24
@@ -153,7 +153,6 @@
 			     int show, int passed_a_ptr, int demangled_args)
 {
   int upper_bound, lower_bound;
-  int lower_bound_was_default = 0;
   static int arrayprint_recurse_level = 0;
   int retcode;
 
@@ -176,35 +175,19 @@
       if (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_ARRAY)
 	f_type_print_varspec_suffix (TYPE_TARGET_TYPE (type), stream, 0, 0, 0);
 
-      retcode = f77_get_dynamic_lowerbound (type, &lower_bound);
-
-      lower_bound_was_default = 0;
-
-      if (retcode == BOUND_FETCH_ERROR)
-	fprintf_filtered (stream, "???");
-      else if (lower_bound == 1)	/* The default */
-	lower_bound_was_default = 1;
-      else
-	fprintf_filtered (stream, "%d", lower_bound);
-
-      if (lower_bound_was_default)
-	lower_bound_was_default = 0;
-      else
-	fprintf_filtered (stream, ":");
+      lower_bound = f77_get_lowerbound (type);
+      if (lower_bound != 1)	/* Not the default.  */
+	fprintf_filtered (stream, "%d:", lower_bound);
 
       /* Make sure that, if we have an assumed size array, we
          print out a warning and print the upperbound as '*' */
 
-      if (TYPE_ARRAY_UPPER_BOUND_TYPE (type) == BOUND_CANNOT_BE_DETERMINED)
+      if (TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type))
 	fprintf_filtered (stream, "*");
       else
 	{
-	  retcode = f77_get_dynamic_upperbound (type, &upper_bound);
-
-	  if (retcode == BOUND_FETCH_ERROR)
-	    fprintf_filtered (stream, "???");
-	  else
-	    fprintf_filtered (stream, "%d", upper_bound);
+	  upper_bound = f77_get_upperbound (type);
+	  fprintf_filtered (stream, "%d", upper_bound);
 	}
 
       if (TYPE_CODE (TYPE_TARGET_TYPE (type)) != TYPE_CODE_ARRAY)
@@ -354,16 +337,12 @@
     case TYPE_CODE_STRING:
       /* Strings may have dynamic upperbounds (lengths) like arrays. */
 
-      if (TYPE_ARRAY_UPPER_BOUND_TYPE (type) == BOUND_CANNOT_BE_DETERMINED)
+      if (TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type))
 	fprintfi_filtered (level, stream, "character*(*)");
       else
 	{
-	  retcode = f77_get_dynamic_upperbound (type, &upper_bound);
-
-	  if (retcode == BOUND_FETCH_ERROR)
-	    fprintf_filtered (stream, "character*???");
-	  else
-	    fprintf_filtered (stream, "character*%d", upper_bound);
+	  upper_bound = f77_get_upperbound (type);
+	  fprintf_filtered (stream, "character*%d", upper_bound);
 	}
       break;
 
--- src/gdb/f-valprint.c	2008/09/11 14:28:47	1.45
+++ src/gdb/f-valprint.c	2008/10/02 22:06:07	1.46
@@ -61,132 +61,28 @@
 #define F77_DIM_OFFSET(n) (f77_array_offset_tbl[n][0])
 
 int
-f77_get_dynamic_lowerbound (struct type *type, int *lower_bound)
+f77_get_lowerbound (struct type *type)
 {
-  struct frame_info *frame;
-  CORE_ADDR current_frame_addr;
-  CORE_ADDR ptr_to_lower_bound;
-
-  switch (TYPE_ARRAY_LOWER_BOUND_TYPE (type))
-    {
-    case BOUND_BY_VALUE_ON_STACK:
-      frame = deprecated_safe_get_selected_frame ();
-      current_frame_addr = get_frame_base (frame);
-      if (current_frame_addr > 0)
-	{
-	  *lower_bound =
-	    read_memory_integer (current_frame_addr +
-				 TYPE_ARRAY_LOWER_BOUND_VALUE (type),
-				 4);
-	}
-      else
-	{
-	  *lower_bound = DEFAULT_LOWER_BOUND;
-	  return BOUND_FETCH_ERROR;
-	}
-      break;
-
-    case BOUND_SIMPLE:
-      *lower_bound = TYPE_ARRAY_LOWER_BOUND_VALUE (type);
-      break;
-
-    case BOUND_CANNOT_BE_DETERMINED:
-      error (_("Lower bound may not be '*' in F77"));
-      break;
+  if (TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED (type))
+    error (_("Lower bound may not be '*' in F77"));
 
-    case BOUND_BY_REF_ON_STACK:
-      frame = deprecated_safe_get_selected_frame ();
-      current_frame_addr = get_frame_base (frame);
-      if (current_frame_addr > 0)
-	{
-	  struct gdbarch *arch = get_frame_arch (frame);
-	  ptr_to_lower_bound =
-	    read_memory_typed_address (current_frame_addr +
-				       TYPE_ARRAY_LOWER_BOUND_VALUE (type),
-				       builtin_type (arch)->builtin_data_ptr);
-	  *lower_bound = read_memory_integer (ptr_to_lower_bound, 4);
-	}
-      else
-	{
-	  *lower_bound = DEFAULT_LOWER_BOUND;
-	  return BOUND_FETCH_ERROR;
-	}
-      break;
-
-    case BOUND_BY_REF_IN_REG:
-    case BOUND_BY_VALUE_IN_REG:
-    default:
-      error (_("??? unhandled dynamic array bound type ???"));
-      break;
-    }
-  return BOUND_FETCH_OK;
+  return TYPE_ARRAY_LOWER_BOUND_VALUE (type);
 }
 
 int
-f77_get_dynamic_upperbound (struct type *type, int *upper_bound)
+f77_get_upperbound (struct type *type)
 {
-  struct frame_info *frame;
-  CORE_ADDR current_frame_addr = 0;
-  CORE_ADDR ptr_to_upper_bound;
-
-  switch (TYPE_ARRAY_UPPER_BOUND_TYPE (type))
-    {
-    case BOUND_BY_VALUE_ON_STACK:
-      frame = deprecated_safe_get_selected_frame ();
-      current_frame_addr = get_frame_base (frame);
-      if (current_frame_addr > 0)
-	{
-	  *upper_bound =
-	    read_memory_integer (current_frame_addr +
-				 TYPE_ARRAY_UPPER_BOUND_VALUE (type),
-				 4);
-	}
-      else
-	{
-	  *upper_bound = DEFAULT_UPPER_BOUND;
-	  return BOUND_FETCH_ERROR;
-	}
-      break;
-
-    case BOUND_SIMPLE:
-      *upper_bound = TYPE_ARRAY_UPPER_BOUND_VALUE (type);
-      break;
-
-    case BOUND_CANNOT_BE_DETERMINED:
-      /* we have an assumed size array on our hands. Assume that 
-         upper_bound == lower_bound so that we show at least 
-         1 element.If the user wants to see more elements, let 
-         him manually ask for 'em and we'll subscript the 
-         array and show him */
-      f77_get_dynamic_lowerbound (type, upper_bound);
-      break;
-
-    case BOUND_BY_REF_ON_STACK:
-      frame = deprecated_safe_get_selected_frame ();
-      current_frame_addr = get_frame_base (frame);
-      if (current_frame_addr > 0)
-	{
-	  struct gdbarch *arch = get_frame_arch (frame);
-	  ptr_to_upper_bound =
-	    read_memory_typed_address (current_frame_addr +
-				       TYPE_ARRAY_UPPER_BOUND_VALUE (type),
-				       builtin_type (arch)->builtin_data_ptr);
-	  *upper_bound = read_memory_integer (ptr_to_upper_bound, 4);
-	}
-      else
-	{
-	  *upper_bound = DEFAULT_UPPER_BOUND;
-	  return BOUND_FETCH_ERROR;
-	}
-      break;
+  if (TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type))
+    {
+      /* We have an assumed size array on our hands.  Assume that
+	 upper_bound == lower_bound so that we show at least 1 element.
+	 If the user wants to see more elements, let him manually ask for 'em
+	 and we'll subscript the array and show him.  */
 
-    case BOUND_BY_REF_IN_REG:
-    case BOUND_BY_VALUE_IN_REG:
-    default:
-      error (_("??? unhandled dynamic array bound type ???"));
-      break;
+      return f77_get_lowerbound (type);
     }
-  return BOUND_FETCH_OK;
+
+  return TYPE_ARRAY_UPPER_BOUND_VALUE (type);
 }
 
 /* Obtain F77 adjustable array dimensions */
@@ -212,13 +108,8 @@
     f77_get_dynamic_length_of_aggregate (TYPE_TARGET_TYPE (type));
 
   /* Recursion ends here, start setting up lengths.  */
-  retcode = f77_get_dynamic_lowerbound (type, &lower_bound);
-  if (retcode == BOUND_FETCH_ERROR)
-    error (_("Cannot obtain valid array lower bound"));
-
-  retcode = f77_get_dynamic_upperbound (type, &upper_bound);
-  if (retcode == BOUND_FETCH_ERROR)
-    error (_("Cannot obtain valid array upper bound"));
+  lower_bound = f77_get_lowerbound (type);
+  upper_bound = f77_get_upperbound (type);
 
   /* Patch in a valid length value. */
 
@@ -241,16 +132,8 @@
 
   while ((TYPE_CODE (tmp_type) == TYPE_CODE_ARRAY))
     {
-      if (TYPE_ARRAY_UPPER_BOUND_TYPE (tmp_type) == BOUND_CANNOT_BE_DETERMINED)
-	fprintf_filtered (stream, "<assumed size array> ");
-
-      retcode = f77_get_dynamic_upperbound (tmp_type, &upper);
-      if (retcode == BOUND_FETCH_ERROR)
-	error (_("Cannot obtain dynamic upper bound"));
-
-      retcode = f77_get_dynamic_lowerbound (tmp_type, &lower);
-      if (retcode == BOUND_FETCH_ERROR)
-	error (_("Cannot obtain dynamic lower bound"));
+      upper = f77_get_upperbound (tmp_type);
+      lower = f77_get_lowerbound (tmp_type);
 
       F77_DIM_SIZE (ndimen) = upper - lower + 1;
 
--- src/gdb/gdbtypes.c	2008/09/11 14:24:27	1.150
+++ src/gdb/gdbtypes.c	2008/10/02 22:06:07	1.151
@@ -2556,35 +2556,6 @@
     }
 }
 
-static void
-print_bound_type (int bt)
-{
-  switch (bt)
-    {
-    case BOUND_CANNOT_BE_DETERMINED:
-      printf_filtered ("(BOUND_CANNOT_BE_DETERMINED)");
-      break;
-    case BOUND_BY_REF_ON_STACK:
-      printf_filtered ("(BOUND_BY_REF_ON_STACK)");
-      break;
-    case BOUND_BY_VALUE_ON_STACK:
-      printf_filtered ("(BOUND_BY_VALUE_ON_STACK)");
-      break;
-    case BOUND_BY_REF_IN_REG:
-      printf_filtered ("(BOUND_BY_REF_IN_REG)");
-      break;
-    case BOUND_BY_VALUE_IN_REG:
-      printf_filtered ("(BOUND_BY_VALUE_IN_REG)");
-      break;
-    case BOUND_SIMPLE:
-      printf_filtered ("(BOUND_SIMPLE)");
-      break;
-    default:
-      printf_filtered (_("(unknown bound type)"));
-      break;
-    }
-}
-
 static struct obstack dont_print_type_obstack;
 
 void
@@ -2719,14 +2690,6 @@
     }
   puts_filtered ("\n");
   printfi_filtered (spaces, "length %d\n", TYPE_LENGTH (type));
-  printfi_filtered (spaces, "upper_bound_type 0x%x ",
-		    TYPE_ARRAY_UPPER_BOUND_TYPE (type));
-  print_bound_type (TYPE_ARRAY_UPPER_BOUND_TYPE (type));
-  puts_filtered ("\n");
-  printfi_filtered (spaces, "lower_bound_type 0x%x ",
-		    TYPE_ARRAY_LOWER_BOUND_TYPE (type));
-  print_bound_type (TYPE_ARRAY_LOWER_BOUND_TYPE (type));
-  puts_filtered ("\n");
   printfi_filtered (spaces, "objfile ");
   gdb_print_host_address (TYPE_OBJFILE (type), gdb_stdout);
   printf_filtered ("\n");
--- src/gdb/gdbtypes.h	2008/09/11 14:31:43	1.91
+++ src/gdb/gdbtypes.h	2008/10/02 22:06:07	1.92
@@ -316,18 +316,6 @@
 #define TYPE_ADDRESS_CLASS_ALL(t) (TYPE_INSTANCE_FLAGS(t) \
 				   & TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL)
 
-
-/*  Array bound type.  */
-enum array_bound_type
-{
-  BOUND_SIMPLE = 0,
-  BOUND_BY_VALUE_IN_REG,
-  BOUND_BY_REF_IN_REG,
-  BOUND_BY_VALUE_ON_STACK,
-  BOUND_BY_REF_ON_STACK,
-  BOUND_CANNOT_BE_DETERMINED
-};
-
 /* This structure is space-critical.
    Its layout has been tweaked to reduce the space used.  */
 
@@ -337,12 +325,6 @@
 
   ENUM_BITFIELD(type_code) code : 8;
 
-  /* Array bounds.  These fields appear at this location because
-     they pack nicely here.  */
-
-  ENUM_BITFIELD(array_bound_type) upper_bound_type : 4;
-  ENUM_BITFIELD(array_bound_type) lower_bound_type : 4;
-
   /* Flags about this type.  These fields appear at this location
      because they packs nicely here.  See the TYPE_* macros for
      documentation about these fields.  */
@@ -460,7 +442,8 @@
 
     /* For a function or member type, this is 1 if the argument is marked
        artificial.  Artificial arguments should not be shown to the
-       user.  */
+       user.  For TYPE_CODE_RANGE it is set if the specific bound is not
+       defined.  */
     unsigned int artificial : 1;
 
     /* This flag is zero for non-static fields, 1 for fields whose location
@@ -817,10 +800,10 @@
 
 /* Moto-specific stuff for FORTRAN arrays */
 
-#define TYPE_ARRAY_UPPER_BOUND_TYPE(thistype) \
-	TYPE_MAIN_TYPE(thistype)->upper_bound_type
-#define TYPE_ARRAY_LOWER_BOUND_TYPE(thistype) \
-	TYPE_MAIN_TYPE(thistype)->lower_bound_type
+#define TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED(arraytype) \
+   (TYPE_FIELD_ARTIFICIAL((TYPE_FIELD_TYPE((arraytype),0)),1))
+#define TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED(arraytype) \
+   (TYPE_FIELD_ARTIFICIAL((TYPE_FIELD_TYPE((arraytype),0)),0))
 
 #define TYPE_ARRAY_UPPER_BOUND_VALUE(arraytype) \
    (TYPE_FIELD_BITPOS((TYPE_FIELD_TYPE((arraytype),0)),1))
--- src/gdb/m2-typeprint.c	2008/09/27 21:29:30	1.17
+++ src/gdb/m2-typeprint.c	2008/10/02 22:06:07	1.18
@@ -222,7 +222,7 @@
 {
   fprintf_filtered (stream, "ARRAY [");
   if (TYPE_LENGTH (TYPE_TARGET_TYPE (type)) > 0
-      && TYPE_ARRAY_UPPER_BOUND_TYPE (type) != BOUND_CANNOT_BE_DETERMINED)
+      && !TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type))
     {
       if (TYPE_INDEX_TYPE (type) != 0)
 	{
--- src/gdb/p-typeprint.c	2008/09/27 21:29:30	1.23
+++ src/gdb/p-typeprint.c	2008/10/02 22:06:07	1.24
@@ -266,7 +266,7 @@
 	fprintf_filtered (stream, "(");
       fprintf_filtered (stream, "array ");
       if (TYPE_LENGTH (TYPE_TARGET_TYPE (type)) > 0
-	&& TYPE_ARRAY_UPPER_BOUND_TYPE (type) != BOUND_CANNOT_BE_DETERMINED)
+	&& !TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type))
 	fprintf_filtered (stream, "[%d..%d] ",
 			  TYPE_ARRAY_LOWER_BOUND_VALUE (type),
 			  TYPE_ARRAY_UPPER_BOUND_VALUE (type)
--- src/gdb/parse.c	2008/10/01 16:56:52	1.80
+++ src/gdb/parse.c	2008/10/02 22:06:07	1.81
@@ -1268,8 +1268,7 @@
 	  create_array_type ((struct type *) NULL,
 			     follow_type, range_type);
 	if (array_size < 0)
-	  TYPE_ARRAY_UPPER_BOUND_TYPE (follow_type)
-	    = BOUND_CANNOT_BE_DETERMINED;
+	  TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (follow_type) = 1;
 	break;
       case tp_function:
 	/* FIXME-type-allocation: need a way to free this type when we are
--- src/gdb/valops.c	2008/09/11 14:27:34	1.200
+++ src/gdb/valops.c	2008/10/02 22:06:07	1.201
@@ -361,8 +361,7 @@
     {
       struct type *element_type = TYPE_TARGET_TYPE (type);
       unsigned element_length = TYPE_LENGTH (check_typedef (element_type));
-      if (element_length > 0
-	&& TYPE_ARRAY_UPPER_BOUND_TYPE (type) == BOUND_CANNOT_BE_DETERMINED)
+      if (element_length > 0 && TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type))
 	{
 	  struct type *range_type = TYPE_INDEX_TYPE (type);
 	  int val_length = TYPE_LENGTH (type2);
--- src/gdb/varobj.c	2008/09/13 23:37:05	1.117
+++ src/gdb/varobj.c	2008/10/02 22:06:07	1.118
@@ -1951,7 +1951,7 @@
     {
     case TYPE_CODE_ARRAY:
       if (TYPE_LENGTH (type) > 0 && TYPE_LENGTH (target) > 0
-	  && TYPE_ARRAY_UPPER_BOUND_TYPE (type) != BOUND_CANNOT_BE_DETERMINED)
+	  && !TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type))
 	children = TYPE_LENGTH (type) / TYPE_LENGTH (target);
       else
 	/* If we don't know how many elements there are, don't display
--- src/gdb/testsuite/gdb.base/maint.exp	2008/08/24 16:39:57	1.35
+++ src/gdb/testsuite/gdb.base/maint.exp	2008/10/02 22:06:08	1.36
@@ -401,7 +401,7 @@
 
 send_gdb "maint print type argc\n"
 gdb_expect  {
-        -re "type node $hex\r\nname .int. \\($hex\\)\r\ntagname .<NULL>. \\($hex\\)\r\ncode $hex \\(TYPE_CODE_INT\\)\r\nlength \[24\]\r\nupper_bound_type $hex \\(BOUND_SIMPLE\\)\r\nlower_bound_type $hex \\(BOUND_SIMPLE\\)\r\nobjfile $hex\r\ntarget_type $hex\r\npointer_type $hex\r\nreference_type $hex\r\ntype_chain $hex\r\ninstance_flags $hex\r\nflags\r\nnfields 0 $hex\r\nvptr_basetype $hex\r\nvptr_fieldno -1\r\ntype_specific $hex\r\n$gdb_prompt $"\
+        -re "type node $hex\r\nname .int. \\($hex\\)\r\ntagname .<NULL>. \\($hex\\)\r\ncode $hex \\(TYPE_CODE_INT\\)\r\nlength \[24\]\r\nobjfile $hex\r\ntarget_type $hex\r\npointer_type $hex\r\nreference_type $hex\r\ntype_chain $hex\r\ninstance_flags $hex\r\nflags\r\nnfields 0 $hex\r\nvptr_basetype $hex\r\nvptr_fieldno -1\r\ntype_specific $hex\r\n$gdb_prompt $"\
                         { pass "maint print type" }
         -re ".*$gdb_prompt $"       { fail "maint print type" }
         timeout         { fail "(timeout) maint print type" }

Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays]

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 4257 bytes --]

On Sat, 20 Sep 2008 00:12:21 +0200, Joel Brobecker wrote:
> > 2008-09-07  Jan Kratochvil  <jan.kratochvil@redhat.com>
> > 
> > 	Split two-bit STATIC_KIND to two separate bitfield single bits.
> > 	* gdbtypes.h (struct main_type <static_kind>): Split to ...
> > 	(struct main_type <bit0>, <bit1>): ... here.
> > 	(FIELD_STATIC_KIND): Split to ...
> > 	(FIELD_BIT0, FIELD_BIT1): ... here.
> > 	(SET_FIELD_BITPOS): New.
> > 	(SET_FIELD_PHYSNAME, SET_FIELD_PHYSADDR, TYPE_FIELD_STATIC)
> > 	(TYPE_FIELD_STATIC_HAS_ADDR): Update for the split.
> > 	(TYPE_FIELD_STATIC_KIND): Remove.
> > 	(TYPE_FIELD_STATIC_HAS_NAME): New.
> > 	* coffread.c, dwarf2read.c, eval.c, gdbtypes.c, mdebugread.c: Update
> > 	throughout.
> 
> It would be interesting to hear what others think, but I don't really
> like the idea of splitting the 2-bit static_kind field into two one-bit
> fields named bit0 & bit1. I just find it obscures things even more.
> 
> The first goal, as I understand it, is to be able to extend range types
> so that the bounds can be DWARF blocks. Here is a thought: Rename the
> static_kind field into "field_loc_kind" which would be an enum with
> 4 possible values: loc_in_bitpos, loc_in_physaddr, loc_in_physname,
> and loc_in_block.

Done.  Including below reformatted diff to make the changes more related to
each other.

It already introduces the new field into the loc union:
+      /* For variable-sized arrays.  Passed to DWARF_LOCEXPR_BATON_EVAL.  */
+      struct dwarf2_locexpr_baton *dwarf_block;

as otherwise this change is not much useful.  The other way is to implement
there enum address_class but I find it as another larger patch to properly
separate it out from symbols, drop the `struct symbol_ops *ops' field etc.



Thanks,
Jan

Unsplit patch now at:
http://people.redhat.com/jkratoch/vla/fortran-dynamic-arrays-HEAD-b.patch


-#define FIELD_STATIC_KIND(thisfld) ((thisfld).static_kind)
+#define FIELD_LOC_KIND(thisfld) ((thisfld).loc_kind)
-#define TYPE_FIELD_STATIC_KIND(thistype, n) TYPE_MAIN_TYPE (thistype)->fields[n].static_kind
+#define TYPE_FIELD_LOC_KIND(thistype, n) FIELD_LOC_KIND (TYPE_FIELD (thistype, n))

-#define TYPE_FIELD_STATIC_HAS_ADDR(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].static_kind == 2)
+#define FIELD_STATIC(thisfld) (FIELD_LOC_KIND (thisfld) == FIELD_LOC_KIND_PHYSNAME || FIELD_LOC_KIND (thisfld) == FIELD_LOC_KIND_PHYSADDR)
-#define TYPE_FIELD_STATIC(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].static_kind != 0)
+#define TYPE_FIELD_STATIC(thistype, n) FIELD_STATIC (TYPE_FIELD (thistype, n))

-#define TYPE_FIELD_BITPOS(thistype, n) FIELD_BITPOS(TYPE_FIELD(thistype,n))
+#define TYPE_FIELD_BITPOS(thistype, n) FIELD_BITPOS (TYPE_FIELD (thistype, n))
+#define SET_FIELD_BITPOS(thisfld, bitpos) (FIELD_LOC_KIND (thisfld) = FIELD_LOC_KIND_BITPOS, FIELD_BITPOS (thisfld) = (bitpos))

-#define FIELD_PHYSADDR(thisfld) ((thisfld).loc.physaddr)
+#define FIELD_STATIC_PHYSADDR(thisfld) ((thisfld).loc.physaddr)
-#define TYPE_FIELD_STATIC_PHYSADDR(thistype, n) FIELD_PHYSADDR(TYPE_FIELD(thistype, n))
+#define TYPE_FIELD_STATIC_PHYSADDR(thistype, n) FIELD_STATIC_PHYSADDR (TYPE_FIELD (thistype, n))
-#define SET_FIELD_PHYSADDR(thisfld, name)  ((thisfld).static_kind = 2, FIELD_PHYSADDR(thisfld) = (name))
+#define SET_FIELD_PHYSADDR(thisfld, addr) (FIELD_LOC_KIND (thisfld) = FIELD_LOC_KIND_PHYSADDR, FIELD_STATIC_PHYSADDR (thisfld) = (addr))

-#define FIELD_PHYSNAME(thisfld) ((thisfld).loc.physname)
+#define FIELD_STATIC_PHYSNAME(thisfld) ((thisfld).loc.physname)
-#define TYPE_FIELD_STATIC_PHYSNAME(thistype, n) FIELD_PHYSNAME(TYPE_FIELD(thistype, n))
+#define TYPE_FIELD_STATIC_PHYSNAME(thistype, n) FIELD_STATIC_PHYSNAME (TYPE_FIELD (thistype, n))
-#define SET_FIELD_PHYSNAME(thisfld, name)  ((thisfld).static_kind = 1, FIELD_PHYSNAME(thisfld) = (name))
+#define SET_FIELD_PHYSNAME(thisfld, name) (FIELD_LOC_KIND (thisfld) = FIELD_LOC_KIND_PHYSNAME, FIELD_STATIC_PHYSNAME (thisfld) = (name))

+#define FIELD_DWARF_BLOCK(thisfld) ((thisfld).loc.dwarf_block)
+#define TYPE_FIELD_DWARF_BLOCK(thistype, n) FIELD_DWARF_BLOCK (TYPE_FIELD (thistype, n))
+#define SET_FIELD_DWARF_BLOCK(thisfld, addr) (FIELD_LOC_KIND (thisfld) = FIELD_LOC_KIND_DWARF_BLOCK, FIELD_DWARF_BLOCK (thisfld) = (addr))

[-- Attachment #2: fortran8-bit01.patch --]
[-- Type: text/plain, Size: 12911 bytes --]

2008-10-04  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Convert static_kind into loc_kind enum.
	* gdbtypes.h (enum field_loc_kind): New.
	(union field_location): New field dwarf_block.
	(struct field): Rename static_kind as loc_kind.
	(FIELD_STATIC_KIND): Rename to ...
	(FIELD_LOC_KIND): ... here.
	(TYPE_FIELD_STATIC_KIND): Rename to ...
	(TYPE_FIELD_LOC_KIND): ... here and use there now new FIELD_LOC_KIND.
	(TYPE_FIELD_STATIC_HAS_ADDR): Remove.
	(FIELD_STATIC): New.
	(TYPE_FIELD_STATIC): Base now on FIELD_STATIC.
	(TYPE_FIELD_BITPOS): Reformat.
	(SET_FIELD_BITPOS): New.
	(FIELD_PHYSADDR): Rename to ...
	(FIELD_STATIC_PHYSADDR): ... here.
	(TYPE_FIELD_STATIC_PHYSADDR): Follow the FIELD_PHYSADDR rename.
	(SET_FIELD_PHYSADDR): Use new FIELD_LOC_KIND.
	(FIELD_PHYSNAME): Rename to ...
	(FIELD_STATIC_PHYSNAME): ... here.
	(TYPE_FIELD_STATIC_PHYSNAME): Follow the FIELD_PHYSNAME rename.
	(SET_FIELD_PHYSNAME): Use new FIELD_LOC_KIND.
	(FIELD_DWARF_BLOCK, TYPE_FIELD_DWARF_BLOCK, SET_FIELD_DWARF_BLOCK): New.
	* coffread.c, dwarf2read.c, eval.c, gdbtypes.c, mdebugread.c, value.c:
	Update throughout.

--- gdb/coffread.c	1 Oct 2008 16:41:27 -0000	1.86
+++ gdb/coffread.c	4 Oct 2008 19:44:09 -0000
@@ -1943,9 +1943,8 @@ coff_read_struct_type (int index, int le
 	    obsavestring (name, strlen (name), &objfile->objfile_obstack);
 	  FIELD_TYPE (list->field) = decode_type (ms, ms->c_type, &sub_aux,
 						  objfile);
-	  FIELD_BITPOS (list->field) = 8 * ms->c_value;
+	  SET_FIELD_BITPOS (list->field, 8 * ms->c_value);
 	  FIELD_BITSIZE (list->field) = 0;
-	  FIELD_STATIC_KIND (list->field) = 0;
 	  nfields++;
 	  break;
 
@@ -1961,9 +1960,8 @@ coff_read_struct_type (int index, int le
 	    obsavestring (name, strlen (name), &objfile->objfile_obstack);
 	  FIELD_TYPE (list->field) = decode_type (ms, ms->c_type, &sub_aux,
 						  objfile);
-	  FIELD_BITPOS (list->field) = ms->c_value;
+	  SET_FIELD_BITPOS (list->field, ms->c_value);
 	  FIELD_BITSIZE (list->field) = sub_aux.x_sym.x_misc.x_lnsz.x_size;
-	  FIELD_STATIC_KIND (list->field) = 0;
 	  nfields++;
 	  break;
 
@@ -2080,11 +2078,10 @@ coff_read_enum_type (int index, int leng
 	  struct symbol *xsym = syms->symbol[j];
 	  SYMBOL_TYPE (xsym) = type;
 	  TYPE_FIELD_NAME (type, n) = SYMBOL_LINKAGE_NAME (xsym);
-	  TYPE_FIELD_BITPOS (type, n) = SYMBOL_VALUE (xsym);
+	  SET_FIELD_BITPOS (TYPE_FIELD (type, n), SYMBOL_VALUE (xsym));
 	  if (SYMBOL_VALUE (xsym) < 0)
 	    unsigned_enum = 0;
 	  TYPE_FIELD_BITSIZE (type, n) = 0;
-	  TYPE_FIELD_STATIC_KIND (type, n) = 0;
 	}
       if (syms == osyms)
 	break;
--- gdb/dwarf2read.c	30 Sep 2008 16:57:37 -0000	1.285
+++ gdb/dwarf2read.c	4 Oct 2008 19:44:18 -0000
@@ -3561,8 +3561,6 @@ dwarf2_add_field (struct field_info *fip
       /* Get type of field.  */
       fp->type = die_type (die, cu);
 
-      FIELD_STATIC_KIND (*fp) = 0;
-
       /* Get bit size of field (zero if none).  */
       attr = dwarf2_attr (die, DW_AT_bit_size, cu);
       if (attr)
@@ -3590,10 +3588,10 @@ dwarf2_add_field (struct field_info *fip
           else
             byte_offset = decode_locdesc (DW_BLOCK (attr), cu);
 
-          FIELD_BITPOS (*fp) = byte_offset * bits_per_byte;
+          SET_FIELD_BITPOS (*fp, byte_offset * bits_per_byte);
 	}
       else
-	FIELD_BITPOS (*fp) = 0;
+	SET_FIELD_BITPOS (*fp, 0);
       attr = dwarf2_attr (die, DW_AT_bit_offset, cu);
       if (attr)
 	{
@@ -3685,10 +3683,9 @@ dwarf2_add_field (struct field_info *fip
       /* C++ base class field.  */
       attr = dwarf2_attr (die, DW_AT_data_member_location, cu);
       if (attr)
-	FIELD_BITPOS (*fp) = (decode_locdesc (DW_BLOCK (attr), cu)
-			      * bits_per_byte);
+	SET_FIELD_BITPOS (*fp, decode_locdesc (DW_BLOCK (attr), cu)
+			       * bits_per_byte);
       FIELD_BITSIZE (*fp) = 0;
-      FIELD_STATIC_KIND (*fp) = 0;
       FIELD_TYPE (*fp) = die_type (die, cu);
       FIELD_NAME (*fp) = type_name_no_tag (fp->type);
       fip->nbaseclasses++;
@@ -4406,9 +4403,8 @@ process_enumeration_scope (struct die_in
 
 		  FIELD_NAME (fields[num_fields]) = SYMBOL_LINKAGE_NAME (sym);
 		  FIELD_TYPE (fields[num_fields]) = NULL;
-		  FIELD_BITPOS (fields[num_fields]) = SYMBOL_VALUE (sym);
+		  SET_FIELD_BITPOS (fields[num_fields], SYMBOL_VALUE (sym));
 		  FIELD_BITSIZE (fields[num_fields]) = 0;
-		  FIELD_STATIC_KIND (fields[num_fields]) = 0;
 
 		  num_fields++;
 		}
--- gdb/eval.c	2 Oct 2008 22:06:07 -0000	1.99
+++ gdb/eval.c	4 Oct 2008 19:44:22 -0000
@@ -319,7 +319,7 @@ evaluate_struct_tuple (struct value *str
 		  fieldno++;
 		  /* Skip static fields.  */
 		  while (fieldno < TYPE_NFIELDS (struct_type)
-			 && TYPE_FIELD_STATIC_KIND (struct_type, fieldno))
+			 && TYPE_FIELD_STATIC (struct_type, fieldno))
 		    fieldno++;
 		  subfieldno = fieldno;
 		  if (fieldno >= TYPE_NFIELDS (struct_type))
--- gdb/gdbtypes.c	2 Oct 2008 22:06:07 -0000	1.151
+++ gdb/gdbtypes.c	4 Oct 2008 19:44:26 -0000
@@ -2975,7 +2975,7 @@ copy_type_recursive (struct objfile *obj
 	  if (TYPE_FIELD_NAME (type, i))
 	    TYPE_FIELD_NAME (new_type, i) = 
 	      xstrdup (TYPE_FIELD_NAME (type, i));
-	  if (TYPE_FIELD_STATIC_HAS_ADDR (type, i))
+	  if (TYPE_FIELD_LOC_KIND (type, i) == FIELD_LOC_KIND_PHYSADDR)
 	    SET_FIELD_PHYSADDR (TYPE_FIELD (new_type, i),
 				TYPE_FIELD_STATIC_PHYSADDR (type, i));
 	  else if (TYPE_FIELD_STATIC (type, i))
@@ -2983,11 +2983,8 @@ copy_type_recursive (struct objfile *obj
 				xstrdup (TYPE_FIELD_STATIC_PHYSNAME (type, 
 								     i)));
 	  else
-	    {
-	      TYPE_FIELD_BITPOS (new_type, i) = 
-		TYPE_FIELD_BITPOS (type, i);
-	      TYPE_FIELD_STATIC_KIND (new_type, i) = 0;
-	    }
+	    SET_FIELD_BITPOS (TYPE_FIELD (new_type, i),
+			      TYPE_FIELD_BITPOS (type, i));
 	}
     }
 
--- gdb/gdbtypes.h	2 Oct 2008 22:06:07 -0000	1.92
+++ gdb/gdbtypes.h	4 Oct 2008 19:44:27 -0000
@@ -316,6 +316,16 @@ enum type_instance_flag_value
 #define TYPE_ADDRESS_CLASS_ALL(t) (TYPE_INSTANCE_FLAGS(t) \
 				   & TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL)
 
+/* Determine which field of the union main_type.fields[x].loc is used.  */
+
+enum field_loc_kind
+  {
+    FIELD_LOC_KIND_BITPOS,	/* bitpos */
+    FIELD_LOC_KIND_PHYSADDR,	/* physaddr */
+    FIELD_LOC_KIND_PHYSNAME,	/* physname */
+    FIELD_LOC_KIND_DWARF_BLOCK	/* dwarf_block */
+  };
+
 /* This structure is space-critical.
    Its layout has been tweaked to reduce the space used.  */
 
@@ -437,6 +448,9 @@ struct main_type
 
       CORE_ADDR physaddr;
       char *physname;
+
+      /* For variable length arrays.  Passed to dwarf_locexpr_baton_eval.  */
+      struct dwarf2_locexpr_baton *dwarf_block;
     }
     loc;
 
@@ -446,11 +460,8 @@ struct main_type
        defined.  */
     unsigned int artificial : 1;
 
-    /* This flag is zero for non-static fields, 1 for fields whose location
-       is specified by the label loc.physname, and 2 for fields whose location
-       is specified by loc.physaddr.  */
-
-    unsigned int static_kind : 2;
+    /* Discriminant for union field_location.  */
+    ENUM_BITFIELD(field_loc_kind) loc_kind : 2;
 
     /* Size of this field, in bits, or zero if not packed.
        For an unpacked field, the field's type's length
@@ -838,20 +849,38 @@ extern void allocate_cplus_struct_type (
 
 #define FIELD_TYPE(thisfld) ((thisfld).type)
 #define FIELD_NAME(thisfld) ((thisfld).name)
+#define FIELD_LOC_KIND(thisfld) ((thisfld).loc_kind)
+#define FIELD_STATIC(thisfld)					\
+  (FIELD_LOC_KIND (thisfld) == FIELD_LOC_KIND_PHYSNAME		\
+   || FIELD_LOC_KIND (thisfld) == FIELD_LOC_KIND_PHYSADDR)
 #define FIELD_BITPOS(thisfld) ((thisfld).loc.bitpos)
+#define FIELD_STATIC_PHYSNAME(thisfld) ((thisfld).loc.physname)
+#define FIELD_STATIC_PHYSADDR(thisfld) ((thisfld).loc.physaddr)
+#define FIELD_DWARF_BLOCK(thisfld) ((thisfld).loc.dwarf_block)
+#define SET_FIELD_BITPOS(thisfld, bitpos)			\
+  (FIELD_LOC_KIND (thisfld) = FIELD_LOC_KIND_BITPOS,		\
+   FIELD_BITPOS (thisfld) = (bitpos))
+#define SET_FIELD_PHYSNAME(thisfld, name)			\
+  (FIELD_LOC_KIND (thisfld) = FIELD_LOC_KIND_PHYSNAME,		\
+   FIELD_STATIC_PHYSNAME (thisfld) = (name))
+#define SET_FIELD_PHYSADDR(thisfld, addr)			\
+  (FIELD_LOC_KIND (thisfld) = FIELD_LOC_KIND_PHYSADDR,		\
+   FIELD_STATIC_PHYSADDR (thisfld) = (addr))
+#define SET_FIELD_DWARF_BLOCK(thisfld, addr)			\
+  (FIELD_LOC_KIND (thisfld) = FIELD_LOC_KIND_DWARF_BLOCK,	\
+   FIELD_DWARF_BLOCK (thisfld) = (addr))
 #define FIELD_ARTIFICIAL(thisfld) ((thisfld).artificial)
 #define FIELD_BITSIZE(thisfld) ((thisfld).bitsize)
-#define FIELD_STATIC_KIND(thisfld) ((thisfld).static_kind)
-#define FIELD_PHYSNAME(thisfld) ((thisfld).loc.physname)
-#define FIELD_PHYSADDR(thisfld) ((thisfld).loc.physaddr)
-#define SET_FIELD_PHYSNAME(thisfld, name) \
-  ((thisfld).static_kind = 1, FIELD_PHYSNAME(thisfld) = (name))
-#define SET_FIELD_PHYSADDR(thisfld, name) \
-  ((thisfld).static_kind = 2, FIELD_PHYSADDR(thisfld) = (name))
+
 #define TYPE_FIELD(thistype, n) TYPE_MAIN_TYPE(thistype)->fields[n]
 #define TYPE_FIELD_TYPE(thistype, n) FIELD_TYPE(TYPE_FIELD(thistype, n))
 #define TYPE_FIELD_NAME(thistype, n) FIELD_NAME(TYPE_FIELD(thistype, n))
-#define TYPE_FIELD_BITPOS(thistype, n) FIELD_BITPOS(TYPE_FIELD(thistype,n))
+#define TYPE_FIELD_LOC_KIND(thistype, n) FIELD_LOC_KIND (TYPE_FIELD (thistype, n))
+#define TYPE_FIELD_STATIC(thistype, n) FIELD_STATIC (TYPE_FIELD (thistype, n))
+#define TYPE_FIELD_BITPOS(thistype, n) FIELD_BITPOS (TYPE_FIELD (thistype, n))
+#define TYPE_FIELD_STATIC_PHYSNAME(thistype, n) FIELD_STATIC_PHYSNAME (TYPE_FIELD (thistype, n))
+#define TYPE_FIELD_STATIC_PHYSADDR(thistype, n) FIELD_STATIC_PHYSADDR (TYPE_FIELD (thistype, n))
+#define TYPE_FIELD_DWARF_BLOCK(thistype, n) FIELD_DWARF_BLOCK (TYPE_FIELD (thistype, n))
 #define TYPE_FIELD_ARTIFICIAL(thistype, n) FIELD_ARTIFICIAL(TYPE_FIELD(thistype,n))
 #define TYPE_FIELD_BITSIZE(thistype, n) FIELD_BITSIZE(TYPE_FIELD(thistype,n))
 #define TYPE_FIELD_PACKED(thistype, n) (FIELD_BITSIZE(TYPE_FIELD(thistype,n))!=0)
@@ -886,12 +915,6 @@ extern void allocate_cplus_struct_type (
   (TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits == NULL ? 0 \
     : B_TST(TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits, (n)))
 
-#define TYPE_FIELD_STATIC(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].static_kind != 0)
-#define TYPE_FIELD_STATIC_KIND(thistype, n) TYPE_MAIN_TYPE (thistype)->fields[n].static_kind
-#define TYPE_FIELD_STATIC_HAS_ADDR(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].static_kind == 2)
-#define TYPE_FIELD_STATIC_PHYSNAME(thistype, n) FIELD_PHYSNAME(TYPE_FIELD(thistype, n))
-#define TYPE_FIELD_STATIC_PHYSADDR(thistype, n) FIELD_PHYSADDR(TYPE_FIELD(thistype, n))
-
 #define TYPE_FN_FIELDLISTS(thistype) TYPE_CPLUS_SPECIFIC(thistype)->fn_fieldlists
 #define TYPE_FN_FIELDLIST(thistype, n) TYPE_CPLUS_SPECIFIC(thistype)->fn_fieldlists[n]
 #define TYPE_FN_FIELDLIST1(thistype, n) TYPE_CPLUS_SPECIFIC(thistype)->fn_fieldlists[n].fn_fields
--- gdb/mdebugread.c	1 Oct 2008 16:41:27 -0000	1.95
+++ gdb/mdebugread.c	4 Oct 2008 19:44:31 -0000
@@ -1054,11 +1054,10 @@ parse_symbol (SYMR *sh, union aux_ext *a
 		if (tsym.st != stMember)
 		  break;
 
-		FIELD_BITPOS (*f) = tsym.value;
+		SET_FIELD_BITPOS (*f, tsym.value);
 		FIELD_TYPE (*f) = t;
 		FIELD_NAME (*f) = debug_info->ss + cur_fdr->issBase + tsym.iss;
 		FIELD_BITSIZE (*f) = 0;
-		FIELD_STATIC_KIND (*f) = 0;
 
 		enum_sym = ((struct symbol *)
 			    obstack_alloc (&current_objfile->objfile_obstack,
@@ -1241,11 +1240,10 @@ parse_symbol (SYMR *sh, union aux_ext *a
     case stMember:		/* member of struct or union */
       f = &TYPE_FIELDS (top_stack->cur_type)[top_stack->cur_field++];
       FIELD_NAME (*f) = name;
-      FIELD_BITPOS (*f) = sh->value;
+      SET_FIELD_BITPOS (*f, sh->value);
       bitsize = 0;
       FIELD_TYPE (*f) = parse_type (cur_fd, ax, sh->index, &bitsize, bigend, name);
       FIELD_BITSIZE (*f) = bitsize;
-      FIELD_STATIC_KIND (*f) = 0;
       break;
 
     case stIndirect:		/* forward declaration on Irix5 */
@@ -1777,12 +1775,9 @@ upgrade_type (int fd, struct type **tpp,
          ignore the erroneous bitsize from the auxiliary entry safely.
          dbx seems to ignore it too.  */
 
-      /* TYPE_FLAG_TARGET_STUB now takes care of the zero TYPE_LENGTH
-         problem.  */
+      /* TYPE_TARGET_STUB now takes care of the zero TYPE_LENGTH problem.  */
       if (TYPE_LENGTH (*tpp) == 0)
-	{
-	  TYPE_TARGET_STUB (t) = 1;
-	}
+	TYPE_TARGET_STUB (t) = 1;
 
       *tpp = t;
       return 4 + off;
--- gdb/value.c	11 Sep 2008 14:22:33 -0000	1.66
+++ gdb/value.c	4 Oct 2008 19:44:40 -0000
@@ -1236,7 +1236,7 @@ value_static_field (struct type *type, i
 {
   struct value *retval;
 
-  if (TYPE_FIELD_STATIC_HAS_ADDR (type, fieldno))
+  if (TYPE_FIELD_LOC_KIND (type, fieldno) == FIELD_LOC_KIND_PHYSADDR)
     {
       retval = value_at (TYPE_FIELD_TYPE (type, fieldno),
 			 TYPE_FIELD_STATIC_PHYSADDR (type, fieldno));

Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays]

[Joel Brobecker]

> 2008-10-04  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Convert static_kind into loc_kind enum.
[...]

Thanks for the patch. I have a few tiny comments left, but the principle
looks good to me.

> --- gdb/gdbtypes.h	2 Oct 2008 22:06:07 -0000	1.92
> +++ gdb/gdbtypes.h	4 Oct 2008 19:44:27 -0000
> @@ -316,6 +316,16 @@ enum type_instance_flag_value
>  #define TYPE_ADDRESS_CLASS_ALL(t) (TYPE_INSTANCE_FLAGS(t) \
>  				   & TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL)
>  
> +/* Determine which field of the union main_type.fields[x].loc is used.  */
> +
> +enum field_loc_kind
> +  {
> +    FIELD_LOC_KIND_BITPOS,	/* bitpos */

Is it worth forcing the value of FIELD_LOC_KIND_BITPOS to zero?

     FIELD_LOC_KIND_BITPOS = 0,

(this is where my lack of knowledge of C shows up)

> +      /* For variable length arrays.  Passed to dwarf_locexpr_baton_eval.  */
> +      struct dwarf2_locexpr_baton *dwarf_block;

I would prefer a more generic explaination, because this could
be used in any situation where the field location is not known
at compile time.

    /* The field location can be computed by evaluating the following
        DWARF block.  This can be used in Fortran variable-length
        arrays, for instance.  */

> +#define FIELD_STATIC(thisfld)					\
> +  (FIELD_LOC_KIND (thisfld) == FIELD_LOC_KIND_PHYSNAME		\
> +   || FIELD_LOC_KIND (thisfld) == FIELD_LOC_KIND_PHYSADDR)

With the new scheme, this is no longer an accessor macro, but more like
a logical deduction based on field.loc_kind. So I suggest we implement
it as a function instead rather than following the accessor-macro style.
In theory, using the loc discriminant seems a little iffy to me, compared
to having a dedicated flag for that.  But it'll work in practice, at least
for now.  Back to the function, I propose:

    int
    field_is_static (struct field *f)
    {
      /* "static" fields are the fields whose location is not relative
         to the address of the enclosing struct.  It would be nice to
         have a dedicated flag that would be set for static fields when
         the type is being created.  But in practice, checking the field
         loc_kind should give us an accurate answer (at least as long as
         we assume that DWARF block locations are not going to be used
         for static fields).  FIXME?  */
      return (FIELD_LOC_KIND (*f) == FIELD_LOC_KIND_PHYSNAME
              || FIELD_LOC_KIND (*f) == FIELD_LOC_KIND_PHYSADDR);
    }

> +#define TYPE_FIELD_STATIC(thistype, n) FIELD_STATIC (TYPE_FIELD (thistype, n))

If we implement field_is_static, then I propose we delete this macro
entirely.

> --- gdb/dwarf2read.c	30 Sep 2008 16:57:37 -0000	1.285
> +++ gdb/dwarf2read.c	4 Oct 2008 19:44:18 -0000
> @@ -3561,8 +3561,6 @@ dwarf2_add_field (struct field_info *fip

One interesting consequence of your change is that it should help
when we want to deal with fields whose position is described with
a location list.


>        /* Get type of field.  */
>        fp->type = die_type (die, cu);
>  
> -      FIELD_STATIC_KIND (*fp) = 0;
[...]
> -
>        /* Get bit size of field (zero if none).  */
>        attr = dwarf2_attr (die, DW_AT_bit_size, cu);
>        if (attr)
> @@ -3590,10 +3588,10 @@ dwarf2_add_field (struct field_info *fip
>            else
>              byte_offset = decode_locdesc (DW_BLOCK (attr), cu);
>  
> -          FIELD_BITPOS (*fp) = byte_offset * bits_per_byte;
> +          SET_FIELD_BITPOS (*fp, byte_offset * bits_per_byte);
>  	}
>        else
> -	FIELD_BITPOS (*fp) = 0;
> +	SET_FIELD_BITPOS (*fp, 0);

Can we replace the "FIELD_STATIC_KIND = 0" line by "SET_FIELD_BITPOS
(*fp, 0);" and then remove the "else" part below?  I am convinced that
your change is correct, and the kind will always be set because it is
set in both branches of the "if/else", but this way feels safer against
future changes.

-- 
Joel

Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays]

[Jan Kratochvil]

Hi,

you were probably describing it already in your mail but more simply:

SYMBOL_OPS can currently have only two values: dwarf2_loclist_funcs or
dwarf2_locexpr_funcs.  Which is moreover dependent on
ENUM_BITFIELD(address_class).  So the whole ops field has a useful size of
1 bit.  In fact 0 bits as there can be LOC_COMPUTED_LOCLIST and
LOC_COMPUTED_LOCEXPR instead of LOC_COMPUTED.

But I would prefer to keep this part as a different patch.


Thanks,
Jan

Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays]

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 1532 bytes --]

On Mon, 06 Oct 2008 22:00:01 +0200, Joel Brobecker wrote:
> > +enum field_loc_kind
> > +  {
> > +    FIELD_LOC_KIND_BITPOS,	/* bitpos */
> 
> Is it worth forcing the value of FIELD_LOC_KIND_BITPOS to zero?
> 
>      FIELD_LOC_KIND_BITPOS = 0,
> 
> (this is where my lack of knowledge of C shows up)

No, according to ISO C99 enums with no assignments get values 0, 1, 2...


>     int
>     field_is_static (struct field *f)
>     {
>       /* "static" fields are the fields whose location is not relative
>          to the address of the enclosing struct.  It would be nice to
>          have a dedicated flag that would be set for static fields when
>          the type is being created.  But in practice, checking the field
>          loc_kind should give us an accurate answer (at least as long as
>          we assume that DWARF block locations are not going to be used
>          for static fields).  FIXME?  */
>       return (FIELD_LOC_KIND (*f) == FIELD_LOC_KIND_PHYSNAME
>               || FIELD_LOC_KIND (*f) == FIELD_LOC_KIND_PHYSADDR);
>     }

Included.  FIELD_LOC_KIND_DWARF_BLOCK could be even asserted-out as the
current patchset uses FIELD_LOC_KIND_DWARF_BLOCK only for TYPE_CODE_RANGE
fields (low/high/stride) upon whose field_is_static() is never called.


> Can we replace the "FIELD_STATIC_KIND = 0" line by "SET_FIELD_BITPOS
> (*fp, 0);" and then remove the "else" part below?

Done.


Included you as a co-author of this patch.  OK to apply?
Regression-tested only on x86_64-unknown-linux-gnu.


Thanks,
Jan

[-- Attachment #2: fortran9-bit01.patch --]
[-- Type: text/plain, Size: 23278 bytes --]

2008-10-08  Jan Kratochvil  <jan.kratochvil@redhat.com>
	    Joel Brobecker  <brobecker@adacore.com>

	Convert static_kind into loc_kind enum.
	* gdbtypes.h (enum field_loc_kind): New.
	(union field_location): New field dwarf_block.
	(struct field): Rename static_kind as loc_kind.
	(FIELD_STATIC_KIND): Rename to ...
	(FIELD_LOC_KIND): ... here.
	(TYPE_FIELD_STATIC_KIND): Rename to ...
	(TYPE_FIELD_LOC_KIND): ... here and use there now new FIELD_LOC_KIND.
	(TYPE_FIELD_STATIC_HAS_ADDR): Remove.
	(TYPE_FIELD_STATIC): Remove.
	(TYPE_FIELD_BITPOS): Reformat.
	(SET_FIELD_BITPOS): New.
	(FIELD_PHYSADDR): Rename to ...
	(FIELD_STATIC_PHYSADDR): ... here.
	(TYPE_FIELD_STATIC_PHYSADDR): Follow the FIELD_PHYSADDR rename.
	(SET_FIELD_PHYSADDR): Use new FIELD_LOC_KIND.
	(FIELD_PHYSNAME): Rename to ...
	(FIELD_STATIC_PHYSNAME): ... here.
	(TYPE_FIELD_STATIC_PHYSNAME): Follow the FIELD_PHYSNAME rename.
	(SET_FIELD_PHYSNAME): Use new FIELD_LOC_KIND.
	(FIELD_DWARF_BLOCK, TYPE_FIELD_DWARF_BLOCK, SET_FIELD_DWARF_BLOCK): New.
	(field_is_static): New declaration.
	* gdbtypes.c (field_is_static): New function.
	(copy_type_recursive): Update throughout.
	* amd64-tdep.c, c-typeprint.c, coffread.c, cp-valprint.c, dwarf2read.c,
	eval.c, jv-typeprint.c, jv-valprint.c, mdebugread.c, p-typeprint.c,
	p-valprint.c, valops.c, value.c, varobj.c: Update throughout.

--- gdb/amd64-tdep.c	11 Sep 2008 14:23:15 -0000	1.54
+++ gdb/amd64-tdep.c	7 Oct 2008 22:25:44 -0000
@@ -316,7 +316,7 @@ amd64_classify_aggregate (struct type *t
 	  enum amd64_reg_class subclass[2];
 
 	  /* Ignore static fields.  */
-	  if (TYPE_FIELD_STATIC (type, i))
+	  if (field_is_static (&TYPE_FIELD (type, i)))
 	    continue;
 
 	  gdb_assert (pos == 0 || pos == 1);
--- gdb/c-typeprint.c	2 Oct 2008 22:06:07 -0000	1.44
+++ gdb/c-typeprint.c	7 Oct 2008 22:25:46 -0000
@@ -890,14 +890,12 @@ c_type_print_base (struct type *type, st
 		}
 
 	      print_spaces_filtered (level + 4, stream);
-	      if (TYPE_FIELD_STATIC (type, i))
-		{
-		  fprintf_filtered (stream, "static ");
-		}
+	      if (field_is_static (&TYPE_FIELD (type, i)))
+		fprintf_filtered (stream, "static ");
 	      c_print_type (TYPE_FIELD_TYPE (type, i),
 			    TYPE_FIELD_NAME (type, i),
 			    stream, show - 1, level + 4);
-	      if (!TYPE_FIELD_STATIC (type, i)
+	      if (!field_is_static (&TYPE_FIELD (type, i))
 		  && TYPE_FIELD_PACKED (type, i))
 		{
 		  /* It is a bitfield.  This code does not attempt
--- gdb/coffread.c	1 Oct 2008 16:41:27 -0000	1.86
+++ gdb/coffread.c	7 Oct 2008 22:25:47 -0000
@@ -1943,9 +1943,8 @@ coff_read_struct_type (int index, int le
 	    obsavestring (name, strlen (name), &objfile->objfile_obstack);
 	  FIELD_TYPE (list->field) = decode_type (ms, ms->c_type, &sub_aux,
 						  objfile);
-	  FIELD_BITPOS (list->field) = 8 * ms->c_value;
+	  SET_FIELD_BITPOS (list->field, 8 * ms->c_value);
 	  FIELD_BITSIZE (list->field) = 0;
-	  FIELD_STATIC_KIND (list->field) = 0;
 	  nfields++;
 	  break;
 
@@ -1961,9 +1960,8 @@ coff_read_struct_type (int index, int le
 	    obsavestring (name, strlen (name), &objfile->objfile_obstack);
 	  FIELD_TYPE (list->field) = decode_type (ms, ms->c_type, &sub_aux,
 						  objfile);
-	  FIELD_BITPOS (list->field) = ms->c_value;
+	  SET_FIELD_BITPOS (list->field, ms->c_value);
 	  FIELD_BITSIZE (list->field) = sub_aux.x_sym.x_misc.x_lnsz.x_size;
-	  FIELD_STATIC_KIND (list->field) = 0;
 	  nfields++;
 	  break;
 
@@ -2080,11 +2078,10 @@ coff_read_enum_type (int index, int leng
 	  struct symbol *xsym = syms->symbol[j];
 	  SYMBOL_TYPE (xsym) = type;
 	  TYPE_FIELD_NAME (type, n) = SYMBOL_LINKAGE_NAME (xsym);
-	  TYPE_FIELD_BITPOS (type, n) = SYMBOL_VALUE (xsym);
+	  SET_FIELD_BITPOS (TYPE_FIELD (type, n), SYMBOL_VALUE (xsym));
 	  if (SYMBOL_VALUE (xsym) < 0)
 	    unsigned_enum = 0;
 	  TYPE_FIELD_BITSIZE (type, n) = 0;
-	  TYPE_FIELD_STATIC_KIND (type, n) = 0;
 	}
       if (syms == osyms)
 	break;
--- gdb/cp-valprint.c	11 Sep 2008 14:24:27 -0000	1.54
+++ gdb/cp-valprint.c	7 Oct 2008 22:25:47 -0000
@@ -192,7 +192,8 @@ cp_print_value_fields (struct type *type
       for (i = n_baseclasses; i < len; i++)
 	{
 	  /* If requested, skip printing of static fields.  */
-	  if (!static_field_print && TYPE_FIELD_STATIC (type, i))
+	  if (!static_field_print
+	      && field_is_static (&TYPE_FIELD (type, i)))
 	    continue;
 
 	  if (fields_seen)
@@ -225,7 +226,7 @@ cp_print_value_fields (struct type *type
 		fputs_filtered ("\"( ptr \"", stream);
 	      else
 		fputs_filtered ("\"( nodef \"", stream);
-	      if (TYPE_FIELD_STATIC (type, i))
+	      if (field_is_static (&TYPE_FIELD (type, i)))
 		fputs_filtered ("static ", stream);
 	      fprintf_symbol_filtered (stream, TYPE_FIELD_NAME (type, i),
 				       current_language->la_language,
@@ -240,7 +241,7 @@ cp_print_value_fields (struct type *type
 	    {
 	      annotate_field_begin (TYPE_FIELD_TYPE (type, i));
 
-	      if (TYPE_FIELD_STATIC (type, i))
+	      if (field_is_static (&TYPE_FIELD (type, i)))
 		fputs_filtered ("static ", stream);
 	      fprintf_symbol_filtered (stream, TYPE_FIELD_NAME (type, i),
 				       current_language->la_language,
@@ -252,7 +253,8 @@ cp_print_value_fields (struct type *type
 	      annotate_field_value ();
 	    }
 
-	  if (!TYPE_FIELD_STATIC (type, i) && TYPE_FIELD_PACKED (type, i))
+	  if (!field_is_static (&TYPE_FIELD (type, i))
+	      && TYPE_FIELD_PACKED (type, i))
 	    {
 	      struct value *v;
 
@@ -278,7 +280,7 @@ cp_print_value_fields (struct type *type
 		{
 		  fputs_filtered ("<optimized out or zero length>", stream);
 		}
-	      else if (TYPE_FIELD_STATIC (type, i))
+	      else if (field_is_static (&TYPE_FIELD (type, i)))
 		{
 		  struct value *v = value_static_field (type, i);
 		  if (v == NULL)
--- gdb/dwarf2read.c	6 Oct 2008 22:23:17 -0000	1.287
+++ gdb/dwarf2read.c	7 Oct 2008 22:25:55 -0000
@@ -3588,7 +3588,7 @@ dwarf2_add_field (struct field_info *fip
       /* Get type of field.  */
       fp->type = die_type (die, cu);
 
-      FIELD_STATIC_KIND (*fp) = 0;
+      SET_FIELD_BITPOS (*fp, 0);
 
       /* Get bit size of field (zero if none).  */
       attr = dwarf2_attr (die, DW_AT_bit_size, cu);
@@ -3617,10 +3617,8 @@ dwarf2_add_field (struct field_info *fip
           else
             byte_offset = decode_locdesc (DW_BLOCK (attr), cu);
 
-          FIELD_BITPOS (*fp) = byte_offset * bits_per_byte;
+          SET_FIELD_BITPOS (*fp, byte_offset * bits_per_byte);
 	}
-      else
-	FIELD_BITPOS (*fp) = 0;
       attr = dwarf2_attr (die, DW_AT_bit_offset, cu);
       if (attr)
 	{
@@ -3712,10 +3710,9 @@ dwarf2_add_field (struct field_info *fip
       /* C++ base class field.  */
       attr = dwarf2_attr (die, DW_AT_data_member_location, cu);
       if (attr)
-	FIELD_BITPOS (*fp) = (decode_locdesc (DW_BLOCK (attr), cu)
-			      * bits_per_byte);
+	SET_FIELD_BITPOS (*fp, decode_locdesc (DW_BLOCK (attr), cu)
+			       * bits_per_byte);
       FIELD_BITSIZE (*fp) = 0;
-      FIELD_STATIC_KIND (*fp) = 0;
       FIELD_TYPE (*fp) = die_type (die, cu);
       FIELD_NAME (*fp) = type_name_no_tag (fp->type);
       fip->nbaseclasses++;
@@ -4433,9 +4430,8 @@ process_enumeration_scope (struct die_in
 
 		  FIELD_NAME (fields[num_fields]) = SYMBOL_LINKAGE_NAME (sym);
 		  FIELD_TYPE (fields[num_fields]) = NULL;
-		  FIELD_BITPOS (fields[num_fields]) = SYMBOL_VALUE (sym);
+		  SET_FIELD_BITPOS (fields[num_fields], SYMBOL_VALUE (sym));
 		  FIELD_BITSIZE (fields[num_fields]) = 0;
-		  FIELD_STATIC_KIND (fields[num_fields]) = 0;
 
 		  num_fields++;
 		}
--- gdb/eval.c	2 Oct 2008 22:06:07 -0000	1.99
+++ gdb/eval.c	7 Oct 2008 22:25:56 -0000
@@ -319,7 +319,8 @@ evaluate_struct_tuple (struct value *str
 		  fieldno++;
 		  /* Skip static fields.  */
 		  while (fieldno < TYPE_NFIELDS (struct_type)
-			 && TYPE_FIELD_STATIC_KIND (struct_type, fieldno))
+			 && field_is_static (&TYPE_FIELD (struct_type,
+							  fieldno)))
 		    fieldno++;
 		  subfieldno = fieldno;
 		  if (fieldno >= TYPE_NFIELDS (struct_type))
--- gdb/gdbtypes.c	3 Oct 2008 22:00:46 -0000	1.152
+++ gdb/gdbtypes.c	7 Oct 2008 22:26:00 -0000
@@ -2434,6 +2434,20 @@ print_arg_types (struct field *args, int
     }
 }
 
+int
+field_is_static (struct field *f)
+{
+  /* "static" fields are the fields whose location is not relative
+     to the address of the enclosing struct.  It would be nice to
+     have a dedicated flag that would be set for static fields when
+     the type is being created.  But in practice, checking the field
+     loc_kind should give us an accurate answer (at least as long as
+     we assume that DWARF block locations are not going to be used
+     for static fields).  FIXME?  */
+  return (FIELD_LOC_KIND (*f) == FIELD_LOC_KIND_PHYSNAME
+	  || FIELD_LOC_KIND (*f) == FIELD_LOC_KIND_PHYSADDR);
+}
+
 static void
 dump_fn_fieldlists (struct type *type, int spaces)
 {
@@ -2975,18 +2989,23 @@ copy_type_recursive (struct objfile *obj
 	  if (TYPE_FIELD_NAME (type, i))
 	    TYPE_FIELD_NAME (new_type, i) = 
 	      xstrdup (TYPE_FIELD_NAME (type, i));
-	  if (TYPE_FIELD_STATIC_HAS_ADDR (type, i))
-	    SET_FIELD_PHYSADDR (TYPE_FIELD (new_type, i),
-				TYPE_FIELD_STATIC_PHYSADDR (type, i));
-	  else if (TYPE_FIELD_STATIC (type, i))
-	    SET_FIELD_PHYSNAME (TYPE_FIELD (new_type, i),
-				xstrdup (TYPE_FIELD_STATIC_PHYSNAME (type, 
-								     i)));
-	  else
+	  switch (TYPE_FIELD_LOC_KIND (type, i))
 	    {
-	      TYPE_FIELD_BITPOS (new_type, i) = 
-		TYPE_FIELD_BITPOS (type, i);
-	      TYPE_FIELD_STATIC_KIND (new_type, i) = 0;
+	    case FIELD_LOC_KIND_BITPOS:
+	      SET_FIELD_BITPOS (TYPE_FIELD (new_type, i),
+				TYPE_FIELD_BITPOS (type, i));
+	      break;
+	    case FIELD_LOC_KIND_PHYSADDR:
+	      SET_FIELD_PHYSADDR (TYPE_FIELD (new_type, i),
+				  TYPE_FIELD_STATIC_PHYSADDR (type, i));
+	      break;
+	    case FIELD_LOC_KIND_PHYSNAME:
+	      SET_FIELD_PHYSNAME (TYPE_FIELD (new_type, i),
+				  xstrdup (TYPE_FIELD_STATIC_PHYSNAME (type,
+								       i)));
+	      break;
+	    default:
+	      gdb_assert (0);
 	    }
 	}
     }
--- gdb/gdbtypes.h	2 Oct 2008 22:06:07 -0000	1.92
+++ gdb/gdbtypes.h	7 Oct 2008 22:26:00 -0000
@@ -316,6 +316,16 @@ enum type_instance_flag_value
 #define TYPE_ADDRESS_CLASS_ALL(t) (TYPE_INSTANCE_FLAGS(t) \
 				   & TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL)
 
+/* Determine which field of the union main_type.fields[x].loc is used.  */
+
+enum field_loc_kind
+  {
+    FIELD_LOC_KIND_BITPOS,	/* bitpos */
+    FIELD_LOC_KIND_PHYSADDR,	/* physaddr */
+    FIELD_LOC_KIND_PHYSNAME,	/* physname */
+    FIELD_LOC_KIND_DWARF_BLOCK	/* dwarf_block */
+  };
+
 /* This structure is space-critical.
    Its layout has been tweaked to reduce the space used.  */
 
@@ -437,6 +447,12 @@ struct main_type
 
       CORE_ADDR physaddr;
       char *physname;
+
+      /* The field location can be computed by evaluating the following DWARF
+	 block.  This can be used in Fortran variable-length arrays, for
+	 instance.  */
+
+      struct dwarf2_locexpr_baton *dwarf_block;
     }
     loc;
 
@@ -446,11 +462,8 @@ struct main_type
        defined.  */
     unsigned int artificial : 1;
 
-    /* This flag is zero for non-static fields, 1 for fields whose location
-       is specified by the label loc.physname, and 2 for fields whose location
-       is specified by loc.physaddr.  */
-
-    unsigned int static_kind : 2;
+    /* Discriminant for union field_location.  */
+    ENUM_BITFIELD(field_loc_kind) loc_kind : 2;
 
     /* Size of this field, in bits, or zero if not packed.
        For an unpacked field, the field's type's length
@@ -838,20 +851,34 @@ extern void allocate_cplus_struct_type (
 
 #define FIELD_TYPE(thisfld) ((thisfld).type)
 #define FIELD_NAME(thisfld) ((thisfld).name)
+#define FIELD_LOC_KIND(thisfld) ((thisfld).loc_kind)
 #define FIELD_BITPOS(thisfld) ((thisfld).loc.bitpos)
+#define FIELD_STATIC_PHYSNAME(thisfld) ((thisfld).loc.physname)
+#define FIELD_STATIC_PHYSADDR(thisfld) ((thisfld).loc.physaddr)
+#define FIELD_DWARF_BLOCK(thisfld) ((thisfld).loc.dwarf_block)
+#define SET_FIELD_BITPOS(thisfld, bitpos)			\
+  (FIELD_LOC_KIND (thisfld) = FIELD_LOC_KIND_BITPOS,		\
+   FIELD_BITPOS (thisfld) = (bitpos))
+#define SET_FIELD_PHYSNAME(thisfld, name)			\
+  (FIELD_LOC_KIND (thisfld) = FIELD_LOC_KIND_PHYSNAME,		\
+   FIELD_STATIC_PHYSNAME (thisfld) = (name))
+#define SET_FIELD_PHYSADDR(thisfld, addr)			\
+  (FIELD_LOC_KIND (thisfld) = FIELD_LOC_KIND_PHYSADDR,		\
+   FIELD_STATIC_PHYSADDR (thisfld) = (addr))
+#define SET_FIELD_DWARF_BLOCK(thisfld, addr)			\
+  (FIELD_LOC_KIND (thisfld) = FIELD_LOC_KIND_DWARF_BLOCK,	\
+   FIELD_DWARF_BLOCK (thisfld) = (addr))
 #define FIELD_ARTIFICIAL(thisfld) ((thisfld).artificial)
 #define FIELD_BITSIZE(thisfld) ((thisfld).bitsize)
-#define FIELD_STATIC_KIND(thisfld) ((thisfld).static_kind)
-#define FIELD_PHYSNAME(thisfld) ((thisfld).loc.physname)
-#define FIELD_PHYSADDR(thisfld) ((thisfld).loc.physaddr)
-#define SET_FIELD_PHYSNAME(thisfld, name) \
-  ((thisfld).static_kind = 1, FIELD_PHYSNAME(thisfld) = (name))
-#define SET_FIELD_PHYSADDR(thisfld, name) \
-  ((thisfld).static_kind = 2, FIELD_PHYSADDR(thisfld) = (name))
+
 #define TYPE_FIELD(thistype, n) TYPE_MAIN_TYPE(thistype)->fields[n]
 #define TYPE_FIELD_TYPE(thistype, n) FIELD_TYPE(TYPE_FIELD(thistype, n))
 #define TYPE_FIELD_NAME(thistype, n) FIELD_NAME(TYPE_FIELD(thistype, n))
-#define TYPE_FIELD_BITPOS(thistype, n) FIELD_BITPOS(TYPE_FIELD(thistype,n))
+#define TYPE_FIELD_LOC_KIND(thistype, n) FIELD_LOC_KIND (TYPE_FIELD (thistype, n))
+#define TYPE_FIELD_BITPOS(thistype, n) FIELD_BITPOS (TYPE_FIELD (thistype, n))
+#define TYPE_FIELD_STATIC_PHYSNAME(thistype, n) FIELD_STATIC_PHYSNAME (TYPE_FIELD (thistype, n))
+#define TYPE_FIELD_STATIC_PHYSADDR(thistype, n) FIELD_STATIC_PHYSADDR (TYPE_FIELD (thistype, n))
+#define TYPE_FIELD_DWARF_BLOCK(thistype, n) FIELD_DWARF_BLOCK (TYPE_FIELD (thistype, n))
 #define TYPE_FIELD_ARTIFICIAL(thistype, n) FIELD_ARTIFICIAL(TYPE_FIELD(thistype,n))
 #define TYPE_FIELD_BITSIZE(thistype, n) FIELD_BITSIZE(TYPE_FIELD(thistype,n))
 #define TYPE_FIELD_PACKED(thistype, n) (FIELD_BITSIZE(TYPE_FIELD(thistype,n))!=0)
@@ -886,12 +913,6 @@ extern void allocate_cplus_struct_type (
   (TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits == NULL ? 0 \
     : B_TST(TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits, (n)))
 
-#define TYPE_FIELD_STATIC(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].static_kind != 0)
-#define TYPE_FIELD_STATIC_KIND(thistype, n) TYPE_MAIN_TYPE (thistype)->fields[n].static_kind
-#define TYPE_FIELD_STATIC_HAS_ADDR(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].static_kind == 2)
-#define TYPE_FIELD_STATIC_PHYSNAME(thistype, n) FIELD_PHYSNAME(TYPE_FIELD(thistype, n))
-#define TYPE_FIELD_STATIC_PHYSADDR(thistype, n) FIELD_PHYSADDR(TYPE_FIELD(thistype, n))
-
 #define TYPE_FN_FIELDLISTS(thistype) TYPE_CPLUS_SPECIFIC(thistype)->fn_fieldlists
 #define TYPE_FN_FIELDLIST(thistype, n) TYPE_CPLUS_SPECIFIC(thistype)->fn_fieldlists[n]
 #define TYPE_FN_FIELDLIST1(thistype, n) TYPE_CPLUS_SPECIFIC(thistype)->fn_fieldlists[n].fn_fields
@@ -1221,6 +1242,8 @@ extern int rank_one_type (struct type *,
 
 extern void recursive_dump_type (struct type *, int);
 
+extern int field_is_static (struct field *);
+
 /* printcmd.c */
 
 extern void print_scalar_formatted (const void *, struct type *, int, int,
--- gdb/jv-typeprint.c	1 Jan 2008 22:53:11 -0000	1.14
+++ gdb/jv-typeprint.c	7 Oct 2008 22:26:00 -0000
@@ -185,7 +185,7 @@ java_type_print_base (struct type *type,
 		    fprintf_filtered (stream, "public ");
 		}
 
-	      if (TYPE_FIELD_STATIC (type, i))
+	      if (field_is_static (&TYPE_FIELD (type, i)))
 		fprintf_filtered (stream, "static ");
 
 	      java_print_type (TYPE_FIELD_TYPE (type, i),
--- gdb/jv-valprint.c	6 May 2008 21:34:59 -0000	1.32
+++ gdb/jv-valprint.c	7 Oct 2008 22:26:01 -0000
@@ -304,7 +304,7 @@ java_print_value_fields (struct type *ty
       for (i = n_baseclasses; i < len; i++)
 	{
 	  /* If requested, skip printing of static fields.  */
-	  if (TYPE_FIELD_STATIC (type, i))
+	  if (field_is_static (&TYPE_FIELD (type, i)))
 	    {
 	      char *name = TYPE_FIELD_NAME (type, i);
 	      if (!static_field_print)
@@ -342,7 +342,7 @@ java_print_value_fields (struct type *ty
 		fputs_filtered ("\"( ptr \"", stream);
 	      else
 		fputs_filtered ("\"( nodef \"", stream);
-	      if (TYPE_FIELD_STATIC (type, i))
+	      if (field_is_static (&TYPE_FIELD (type, i)))
 		fputs_filtered ("static ", stream);
 	      fprintf_symbol_filtered (stream, TYPE_FIELD_NAME (type, i),
 				       language_cplus,
@@ -357,7 +357,7 @@ java_print_value_fields (struct type *ty
 	    {
 	      annotate_field_begin (TYPE_FIELD_TYPE (type, i));
 
-	      if (TYPE_FIELD_STATIC (type, i))
+	      if (field_is_static (&TYPE_FIELD (type, i)))
 		fputs_filtered ("static ", stream);
 	      fprintf_symbol_filtered (stream, TYPE_FIELD_NAME (type, i),
 				       language_cplus,
@@ -367,7 +367,8 @@ java_print_value_fields (struct type *ty
 	      annotate_field_value ();
 	    }
 
-	  if (!TYPE_FIELD_STATIC (type, i) && TYPE_FIELD_PACKED (type, i))
+	  if (!field_is_static (&TYPE_FIELD (type, i))
+	      && TYPE_FIELD_PACKED (type, i))
 	    {
 	      struct value *v;
 
@@ -392,7 +393,7 @@ java_print_value_fields (struct type *ty
 		{
 		  fputs_filtered ("<optimized out or zero length>", stream);
 		}
-	      else if (TYPE_FIELD_STATIC (type, i))
+	      else if (field_is_static (&TYPE_FIELD (type, i)))
 		{
 		  struct value *v = value_static_field (type, i);
 		  if (v == NULL)
--- gdb/mdebugread.c	1 Oct 2008 16:41:27 -0000	1.95
+++ gdb/mdebugread.c	7 Oct 2008 22:26:05 -0000
@@ -1054,11 +1054,10 @@ parse_symbol (SYMR *sh, union aux_ext *a
 		if (tsym.st != stMember)
 		  break;
 
-		FIELD_BITPOS (*f) = tsym.value;
+		SET_FIELD_BITPOS (*f, tsym.value);
 		FIELD_TYPE (*f) = t;
 		FIELD_NAME (*f) = debug_info->ss + cur_fdr->issBase + tsym.iss;
 		FIELD_BITSIZE (*f) = 0;
-		FIELD_STATIC_KIND (*f) = 0;
 
 		enum_sym = ((struct symbol *)
 			    obstack_alloc (&current_objfile->objfile_obstack,
@@ -1241,11 +1240,10 @@ parse_symbol (SYMR *sh, union aux_ext *a
     case stMember:		/* member of struct or union */
       f = &TYPE_FIELDS (top_stack->cur_type)[top_stack->cur_field++];
       FIELD_NAME (*f) = name;
-      FIELD_BITPOS (*f) = sh->value;
+      SET_FIELD_BITPOS (*f, sh->value);
       bitsize = 0;
       FIELD_TYPE (*f) = parse_type (cur_fd, ax, sh->index, &bitsize, bigend, name);
       FIELD_BITSIZE (*f) = bitsize;
-      FIELD_STATIC_KIND (*f) = 0;
       break;
 
     case stIndirect:		/* forward declaration on Irix5 */
@@ -1777,12 +1775,9 @@ upgrade_type (int fd, struct type **tpp,
          ignore the erroneous bitsize from the auxiliary entry safely.
          dbx seems to ignore it too.  */
 
-      /* TYPE_FLAG_TARGET_STUB now takes care of the zero TYPE_LENGTH
-         problem.  */
+      /* TYPE_TARGET_STUB now takes care of the zero TYPE_LENGTH problem.  */
       if (TYPE_LENGTH (*tpp) == 0)
-	{
-	  TYPE_TARGET_STUB (t) = 1;
-	}
+	TYPE_TARGET_STUB (t) = 1;
 
       *tpp = t;
       return 4 + off;
--- gdb/p-typeprint.c	2 Oct 2008 22:06:07 -0000	1.24
+++ gdb/p-typeprint.c	7 Oct 2008 22:26:07 -0000
@@ -590,14 +590,12 @@ pascal_type_print_base (struct type *typ
 		}
 
 	      print_spaces_filtered (level + 4, stream);
-	      if (TYPE_FIELD_STATIC (type, i))
-		{
-		  fprintf_filtered (stream, "static ");
-		}
+	      if (field_is_static (&TYPE_FIELD (type, i)))
+		fprintf_filtered (stream, "static ");
 	      pascal_print_type (TYPE_FIELD_TYPE (type, i),
 				 TYPE_FIELD_NAME (type, i),
 				 stream, show - 1, level + 4);
-	      if (!TYPE_FIELD_STATIC (type, i)
+	      if (!field_is_static (&TYPE_FIELD (type, i))
 		  && TYPE_FIELD_PACKED (type, i))
 		{
 		  /* It is a bitfield.  This code does not attempt
--- gdb/p-valprint.c	11 Sep 2008 14:17:24 -0000	1.56
+++ gdb/p-valprint.c	7 Oct 2008 22:26:07 -0000
@@ -671,7 +671,8 @@ pascal_object_print_value_fields (struct
       for (i = n_baseclasses; i < len; i++)
 	{
 	  /* If requested, skip printing of static fields.  */
-	  if (!pascal_static_field_print && TYPE_FIELD_STATIC (type, i))
+	  if (!pascal_static_field_print
+	      && field_is_static (&TYPE_FIELD (type, i)))
 	    continue;
 	  if (fields_seen)
 	    fprintf_filtered (stream, ", ");
@@ -703,7 +704,7 @@ pascal_object_print_value_fields (struct
 		fputs_filtered ("\"( ptr \"", stream);
 	      else
 		fputs_filtered ("\"( nodef \"", stream);
-	      if (TYPE_FIELD_STATIC (type, i))
+	      if (field_is_static (&TYPE_FIELD (type, i)))
 		fputs_filtered ("static ", stream);
 	      fprintf_symbol_filtered (stream, TYPE_FIELD_NAME (type, i),
 				       language_cplus,
@@ -718,7 +719,7 @@ pascal_object_print_value_fields (struct
 	    {
 	      annotate_field_begin (TYPE_FIELD_TYPE (type, i));
 
-	      if (TYPE_FIELD_STATIC (type, i))
+	      if (field_is_static (&TYPE_FIELD (type, i)))
 		fputs_filtered ("static ", stream);
 	      fprintf_symbol_filtered (stream, TYPE_FIELD_NAME (type, i),
 				       language_cplus,
@@ -728,7 +729,8 @@ pascal_object_print_value_fields (struct
 	      annotate_field_value ();
 	    }
 
-	  if (!TYPE_FIELD_STATIC (type, i) && TYPE_FIELD_PACKED (type, i))
+	  if (!field_is_static (&TYPE_FIELD (type, i))
+	      && TYPE_FIELD_PACKED (type, i))
 	    {
 	      struct value *v;
 
@@ -753,7 +755,7 @@ pascal_object_print_value_fields (struct
 		{
 		  fputs_filtered ("<optimized out or zero length>", stream);
 		}
-	      else if (TYPE_FIELD_STATIC (type, i))
+	      else if (field_is_static (&TYPE_FIELD (type, i)))
 		{
 		  /* struct value *v = value_static_field (type, i); v4.17 specific */
 		  struct value *v;
--- gdb/valops.c	2 Oct 2008 22:06:07 -0000	1.201
+++ gdb/valops.c	7 Oct 2008 22:26:10 -0000
@@ -1459,7 +1459,7 @@ search_struct_field (char *name, struct 
 	if (t_field_name && (strcmp_iw (t_field_name, name) == 0))
 	  {
 	    struct value *v;
-	    if (TYPE_FIELD_STATIC (type, i))
+	    if (field_is_static (&TYPE_FIELD (type, i)))
 	      {
 		v = value_static_field (type, i);
 		if (v == 0)
@@ -2540,7 +2540,7 @@ value_struct_elt_for_reference (struct t
 
       if (t_field_name && strcmp (t_field_name, name) == 0)
 	{
-	  if (TYPE_FIELD_STATIC (t, i))
+	  if (field_is_static (&TYPE_FIELD (t, i)))
 	    {
 	      v = value_static_field (t, i);
 	      if (v == NULL)
--- gdb/value.c	11 Sep 2008 14:22:33 -0000	1.66
+++ gdb/value.c	7 Oct 2008 22:26:12 -0000
@@ -1236,7 +1236,7 @@ value_static_field (struct type *type, i
 {
   struct value *retval;
 
-  if (TYPE_FIELD_STATIC_HAS_ADDR (type, fieldno))
+  if (TYPE_FIELD_LOC_KIND (type, fieldno) == FIELD_LOC_KIND_PHYSADDR)
     {
       retval = value_at (TYPE_FIELD_TYPE (type, fieldno),
 			 TYPE_FIELD_STATIC_PHYSADDR (type, fieldno));
--- gdb/varobj.c	2 Oct 2008 22:06:07 -0000	1.118
+++ gdb/varobj.c	7 Oct 2008 22:26:13 -0000
@@ -2013,7 +2013,7 @@ value_struct_element_index (struct value
 
   TRY_CATCH (e, RETURN_MASK_ERROR)
     {
-      if (TYPE_FIELD_STATIC (type, type_index))
+      if (field_is_static (&TYPE_FIELD (type, type_index)))
 	result = value_static_field (type, type_index);
       else
 	result = value_primitive_field (value, 0, type_index, type);

type/main_type/field size [Re: [patch] static_kind -> bit0, bit1]

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 929 bytes --]

On Mon, 06 Oct 2008 22:09:28 +0200, Joel Brobecker wrote:
> I think it is kind of ugly, and i tried to think about it for a while,
> but I don't see many possible solutions if we don't want to increase
> the gdbtype struct size.

BTW are there some measurements if sizeof (type / main_type / field) matters?

On Tue, 07 Oct 2008 19:48:53 +0200, Tom Tromey wrote:
> For the lazy psymtab thing I had two related test cases:
> 
> 1. Start OO.o writer, then time 'attach'.
> 2. The same, plus time "thread apply all bt full".

FYI I found (on Firefox) the difference of reading full symbols for this
command `thread apply all bt full' is:
valgrind --tool=massif:
mem_heap_B=393798138
mem_heap_B=395820353
== 2MB == only 0.5% size increase (by the attached patch if it is right)

Just `thread apply all bt full' may not be the typical debugging session,
a more representative application / command set would be useful.


Regards,
Jan

[-- Attachment #2: gdb-readneverfull.patch --]
[-- Type: text/plain, Size: 706 bytes --]

--- gdb/dwarf2read.c	30 Sep 2008 16:57:37 -0000	1.285
+++ gdb/dwarf2read.c	7 Oct 2008 20:50:07 -0000
@@ -2432,6 +2432,7 @@ locate_pdi_sibling (struct partial_die_i
 static void
 dwarf2_psymtab_to_symtab (struct partial_symtab *pst)
 {
+return;
   /* FIXME: This is barely more than a stub.  */
   if (pst != NULL)
     {
--- gdb/symtab.c	1 Oct 2008 17:25:22 -0000	1.199
+++ gdb/symtab.c	7 Oct 2008 20:50:10 -0000
@@ -1527,6 +1527,8 @@ lookup_symbol_aux_psymtabs (int block_in
 				  psymtab_index, domain))
       {
 	s = PSYMTAB_TO_SYMTAB (ps);
+	if (!s)
+	  continue;
 	bv = BLOCKVECTOR (s);
 	block = BLOCKVECTOR_BLOCK (bv, block_index);
 	sym = lookup_block_symbol (block, name, linkage_name, domain);

Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays]

[Joel Brobecker]

> No, according to ISO C99 enums with no assignments get values 0, 1, 2...

Yeah, you're right. I checked C90 as well, since I believe we're still
restricted to using C90 for GDB for now, and it says the same thing.

> Included you as a co-author of this patch.  OK to apply?

That's very kind, but you really didn't need to do that. You should
get sole credit for this work, as you clearly guided me as much as
I tried to guide you.

> 2008-10-08  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 	    Joel Brobecker  <brobecker@adacore.com>
> 
> 	Convert static_kind into loc_kind enum.
> 	* gdbtypes.h (enum field_loc_kind): New.
> 	(union field_location): New field dwarf_block.
> 	(struct field): Rename static_kind as loc_kind.
> 	(FIELD_STATIC_KIND): Rename to ...
> 	(FIELD_LOC_KIND): ... here.
> 	(TYPE_FIELD_STATIC_KIND): Rename to ...
> 	(TYPE_FIELD_LOC_KIND): ... here and use there now new FIELD_LOC_KIND.
> 	(TYPE_FIELD_STATIC_HAS_ADDR): Remove.
> 	(TYPE_FIELD_STATIC): Remove.
> 	(TYPE_FIELD_BITPOS): Reformat.
> 	(SET_FIELD_BITPOS): New.
> 	(FIELD_PHYSADDR): Rename to ...
> 	(FIELD_STATIC_PHYSADDR): ... here.
> 	(TYPE_FIELD_STATIC_PHYSADDR): Follow the FIELD_PHYSADDR rename.
> 	(SET_FIELD_PHYSADDR): Use new FIELD_LOC_KIND.
> 	(FIELD_PHYSNAME): Rename to ...
> 	(FIELD_STATIC_PHYSNAME): ... here.
> 	(TYPE_FIELD_STATIC_PHYSNAME): Follow the FIELD_PHYSNAME rename.
> 	(SET_FIELD_PHYSNAME): Use new FIELD_LOC_KIND.
> 	(FIELD_DWARF_BLOCK, TYPE_FIELD_DWARF_BLOCK, SET_FIELD_DWARF_BLOCK): New.
> 	(field_is_static): New declaration.
> 	* gdbtypes.c (field_is_static): New function.
> 	(copy_type_recursive): Update throughout.
> 	* amd64-tdep.c, c-typeprint.c, coffread.c, cp-valprint.c, dwarf2read.c,
> 	eval.c, jv-typeprint.c, jv-valprint.c, mdebugread.c, p-typeprint.c,
> 	p-valprint.c, valops.c, value.c, varobj.c: Update throughout.

Almost there: Pre-approved with the following nit fixed...

[in copy_type_recursive]
> +	      break;
> +	    default:
> +	      gdb_assert (0);

Can you change that to using an internal_error, instead? For instance,

     internal_error (__FILE__, __LINE__, _("Unexpected ... kind: %d"),
                     TYPE_FIELD_LOC_KIND (type, i));

Something like that.

Thank you,
-- 
Joel

Re: type/main_type/field size [Re: [patch] static_kind -> bit0, bit1]

[Joel Brobecker]

> > I think it is kind of ugly, and i tried to think about it for a while,
> > but I don't see many possible solutions if we don't want to increase
> > the gdbtype struct size.
> 
> BTW are there some measurements if sizeof (type / main_type / field)
> matters?

Once in a while, Tom will post some numbers with each of his size
improvements. He worked pretty hard at reducing the sizes by some
bytes, so I'd like to know what he thinks of the increase before
adding a pointer back (4-8 bytes).

(2MB seems like a small and reasonable increase to me)

-- 
Joel

Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays]

[Jan Kratochvil]

On Wed, 08 Oct 2008 05:27:31 +0200, Joel Brobecker wrote:
> [in copy_type_recursive]
> > +	      break;
> > +	    default:
> > +	      gdb_assert (0);
> 
> Can you change that to using an internal_error, instead? For instance,
> 
>      internal_error (__FILE__, __LINE__, _("Unexpected ... kind: %d"),
>                      TYPE_FIELD_LOC_KIND (type, i));
> 
> Something like that.

Used:
+	      internal_error (__FILE__, __LINE__,
+			      _("Unexpected type field location kind: %d"),
+			      TYPE_FIELD_LOC_KIND (type, i));

And committed:
	http://sourceware.org/ml/gdb-cvs/2008-10/msg00099.html

Going to post the real parts of the patchset rebased on this change and split.


Thanks,
Jan

Re: type/main_type/field size [Re: [patch] static_kind -> bit0, bit1]

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> Once in a while, Tom will post some numbers with each of his size
Joel> improvements. He worked pretty hard at reducing the sizes by some
Joel> bytes, so I'd like to know what he thinks of the increase before
Joel> adding a pointer back (4-8 bytes).

Joel> (2MB seems like a small and reasonable increase to me)

The main_type shrinkage patch was here:

    http://sourceware.org/ml/gdb-patches/2008-08/msg00464.html

According to the note this saved about 1% on "gdb -readnow cc1".
However, this is pretty artificial.  -readnow is not something normal
users do.

So, offhand I would say that type and main_type are not extremely
size-sensitive.

I suppose it is a judgment call as to whether you would rather use
more memory and have the code be clearer, or whether you would rather
save memory and use a bit to indicate (IIUC) a special dwarf-specific
callback.

Tom