[3/7] Remove byte vectors from cplus_struct_type

Message ID 20230921-field-bits-v1-3-201285360900@adacore.com
State New
Headers
Series Remove char-based bitfield macros |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed

Commit Message

Tom Tromey Sept. 21, 2023, 6:01 p.m. UTC
  This removes some byte vectors from cplus_struct_type, moving the
information into bitfields in holes in struct field.
---
 gdb/dwarf2/read.c |  86 +++++++-------------------------------
 gdb/gdbtypes.c    |  53 -----------------------
 gdb/gdbtypes.h    | 118 ++++++++++++++++++++++++---------------------------
 gdb/stabsread.c   | 123 +++++++++++++++++++-----------------------------------
 4 files changed, 113 insertions(+), 267 deletions(-)
  

Comments

Lancelot SIX Sept. 25, 2023, 10:32 p.m. UTC | #1
Hi Tom

On Thu, Sep 21, 2023 at 12:01:30PM -0600, Tom Tromey via Gdb-patches wrote:
> This removes some byte vectors from cplus_struct_type, moving the
> information into bitfields in holes in struct field.
> ---
>  gdb/dwarf2/read.c |  86 +++++++-------------------------------
>  gdb/gdbtypes.c    |  53 -----------------------
>  gdb/gdbtypes.h    | 118 ++++++++++++++++++++++++---------------------------
>  gdb/stabsread.c   | 123 +++++++++++++++++++-----------------------------------
>  4 files changed, 113 insertions(+), 267 deletions(-)
> 
> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> index 505c8ba12b5..c72512b8204 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -668,6 +668,46 @@ struct field
>      m_loc.dwarf_block = dwarf_block;
>    }
>  
> +  /* True if this field is 'private'.  */
> +  bool is_private () const
> +  { return m_private; }
> +
> +  /* Set the field's "private" flag.  */
> +  void set_private ()
> +  {
> +    /* Can't have both.  */
> +    gdb_assert (!m_protected);
> +    m_private = true;
> +  }
> +
> +  /* True if this field is 'protected'.  */
> +  bool is_protected () const
> +  { return m_protected; }
> +
> +  /* Set the field's "protected" flag.  */
> +  void set_protected ()
> +  {
> +    /* Can't have both.  */
> +    gdb_assert (!m_private);
> +    m_protected = true;
> +  }
> +
> +  /* True if this field is 'virtual'.  */
> +  bool is_virtual () const
> +  { return m_virtual; }
> +
> +  /* Set the field's "virtual" flag.  */
> +  void set_virtual ()
> +  { m_virtual = true; }
> +
> +  /* True if this field is 'ignored'.  */
> +  bool is_ignored () const
> +  { return m_ignored; }
> +
> +  /* Set the field's "ignored" flag.  */
> +  void set_ignored ()
> +  { m_ignored = true; }
> +
>    union field_location m_loc;
>  
>    /* * For a function or member type, this is 1 if the argument is
> @@ -677,6 +717,15 @@ struct field
>  
>    unsigned int m_artificial : 1;
>  
> +  /* Whether the field is 'private'.  */
> +  bool m_private : 1;
> +  /* Whether the field is 'protected'.  */
> +  bool m_protected : 1;

Is there a reason I miss to not use an enum to describe the visibility?
This would avoid having to verify that m_private and m_protected are
mutually exclusive.

Is seems to me that something similar to

    debug_visibility m_visibility : 2;

(or a custom enum) should work.

> +  /* Whether the field is 'virtual'.  */
> +  bool m_virtual : 1;
> +  /* Whether the field is 'ignored'.  */
> +  bool m_ignored : 1;
> +
>    /* * Discriminant for union field_location.  */
>  
>    ENUM_BITFIELD(field_loc_kind) m_loc_kind : 3;
> diff --git a/gdb/stabsread.c b/gdb/stabsread.c
> index 7402a26a401..9a84e5f3f80 100644
> --- a/gdb/stabsread.c
> +++ b/gdb/stabsread.c
> @@ -3108,11 +3115,15 @@ read_baseclasses (struct stab_field_info *fip, const char **pp,
>  	}
>        ++(*pp);
>  
> -      newobj->visibility = *(*pp)++;
> -      switch (newobj->visibility)
> +      int visibility = *(*pp)++;
> +      switch (visibility)
>  	{
>  	case VISIBILITY_PRIVATE:
> +	  newobj->field.set_private ();
> +	  break;
>  	case VISIBILITY_PROTECTED:
> +	  newobj->field.set_private ();

I think this should be:

  newobj->field.set_protected ();

> +	  break;
>  	case VISIBILITY_PUBLIC:
>  	  break;
>  	default:

Best,
Lancelot.
  
Tom Tromey Oct. 27, 2023, 2:20 p.m. UTC | #2
>>>>> "Lancelot" == Lancelot SIX <lsix@lancelotsix.com> writes:

>> +  /* Whether the field is 'private'.  */
>> +  bool m_private : 1;
>> +  /* Whether the field is 'protected'.  */
>> +  bool m_protected : 1;

Lancelot> Is there a reason I miss to not use an enum to describe the visibility?
Lancelot> This would avoid having to verify that m_private and m_protected are
Lancelot> mutually exclusive.

I changed this in v2.

>> case VISIBILITY_PROTECTED:
>> +	  newobj->field.set_private ();

Lancelot> I think this should be:

newobj-> field.set_protected ();

I fixed this too.

Tom
  

Patch

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 5bbc8e24cf9..2f7c4c5483f 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -670,8 +670,6 @@  struct variant_part_builder
 
 struct nextfield
 {
-  int accessibility = 0;
-  int virtuality = 0;
   /* Variant parts need to find the discriminant, which is a DIE
      reference.  We track the section offset of each field to make
      this link.  */
@@ -694,9 +692,6 @@  struct field_info
   std::vector<struct nextfield> fields;
   std::vector<struct nextfield> baseclasses;
 
-  /* Set if the accessibility of one of the fields is not public.  */
-  bool non_public_fields = false;
-
   /* Member function fieldlist array, contains name of possibly overloaded
      member function, number of overloaded member functions and a pointer
      to the head of the member function field chain.  */
@@ -11644,15 +11639,23 @@  dwarf2_add_field (struct field_info *fip, struct die_info *die,
 
   new_field->offset = die->sect_off;
 
-  new_field->accessibility = dwarf2_access_attribute (die, cu);
-  if (new_field->accessibility != DW_ACCESS_public)
-    fip->non_public_fields = true;
+  switch (dwarf2_access_attribute (die, cu))
+    {
+    case DW_ACCESS_public:
+      break;
+    case DW_ACCESS_private:
+      new_field->field.set_private ();
+      break;
+    case DW_ACCESS_protected:
+      new_field->field.set_protected ();
+      break;
+    default:
+      gdb_assert_not_reached ("invalid accessibility");
+    }
 
   attr = dwarf2_attr (die, DW_AT_virtuality, cu);
-  if (attr != nullptr)
-    new_field->virtuality = attr->as_virtuality ();
-  else
-    new_field->virtuality = DW_VIRTUALITY_none;
+  if (attr != nullptr && attr->as_virtuality ())
+    new_field->field.set_virtual ();
 
   fp = &new_field->field;
 
@@ -11746,8 +11749,7 @@  dwarf2_add_field (struct field_info *fip, struct die_info *die,
       if (dwarf2_attr (die, DW_AT_artificial, cu))
 	{
 	  fp->set_is_artificial (true);
-	  new_field->accessibility = DW_ACCESS_private;
-	  fip->non_public_fields = true;
+	  fp->set_private ();
 	}
     }
   else if (die->tag == DW_TAG_member || die->tag == DW_TAG_variable)
@@ -12061,30 +12063,9 @@  dwarf2_attach_fields_to_type (struct field_info *fip, struct type *type,
      and create blank accessibility bitfields if necessary.  */
   type->alloc_fields (nfields);
 
-  if (fip->non_public_fields && cu->lang () != language_ada)
-    {
-      ALLOCATE_CPLUS_STRUCT_TYPE (type);
-
-      TYPE_FIELD_PRIVATE_BITS (type) =
-	(B_TYPE *) TYPE_ZALLOC (type, B_BYTES (nfields));
-
-      TYPE_FIELD_PROTECTED_BITS (type) =
-	(B_TYPE *) TYPE_ZALLOC (type, B_BYTES (nfields));
-
-      TYPE_FIELD_IGNORE_BITS (type) =
-	(B_TYPE *) TYPE_ZALLOC (type, B_BYTES (nfields));
-    }
-
-  /* If the type has baseclasses, allocate and clear a bit vector for
-     TYPE_FIELD_VIRTUAL_BITS.  */
   if (!fip->baseclasses.empty () && cu->lang () != language_ada)
     {
-      int num_bytes = B_BYTES (fip->baseclasses.size ());
-      unsigned char *pointer;
-
       ALLOCATE_CPLUS_STRUCT_TYPE (type);
-      pointer = (unsigned char *) TYPE_ZALLOC (type, num_bytes);
-      TYPE_FIELD_VIRTUAL_BITS (type) = pointer;
       TYPE_N_BASECLASSES (type) = fip->baseclasses.size ();
     }
 
@@ -12099,41 +12080,6 @@  dwarf2_attach_fields_to_type (struct field_info *fip, struct type *type,
 	   : fip->fields[i - fip->baseclasses.size ()]);
 
       type->field (i) = field.field;
-      switch (field.accessibility)
-	{
-	case DW_ACCESS_private:
-	  if (cu->lang () != language_ada)
-	    SET_TYPE_FIELD_PRIVATE (type, i);
-	  break;
-
-	case DW_ACCESS_protected:
-	  if (cu->lang () != language_ada)
-	    SET_TYPE_FIELD_PROTECTED (type, i);
-	  break;
-
-	case DW_ACCESS_public:
-	  break;
-
-	default:
-	  /* Unknown accessibility.  Complain and treat it as public.  */
-	  {
-	    complaint (_("unsupported accessibility %d"),
-		       field.accessibility);
-	  }
-	  break;
-	}
-      if (i < fip->baseclasses.size ())
-	{
-	  switch (field.virtuality)
-	    {
-	    case DW_VIRTUALITY_virtual:
-	    case DW_VIRTUALITY_pure_virtual:
-	      if (cu->lang () == language_ada)
-		error (_("unexpected virtuality in component of Ada type"));
-	      SET_TYPE_FIELD_VIRTUAL (type, i);
-	      break;
-	    }
-	}
     }
 }
 
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 1aabb38fa5f..55d12218b4e 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -4883,25 +4883,6 @@  rank_one_type (struct type *parm, struct type *arg, struct value *value)
 
 /* End of functions for overload resolution.  */
 
-/* Routines to pretty-print types.  */
-
-static void
-print_bit_vector (B_TYPE *bits, int nbits)
-{
-  int bitno;
-
-  for (bitno = 0; bitno < nbits; bitno++)
-    {
-      if ((bitno % 8) == 0)
-	{
-	  gdb_puts (" ");
-	}
-      if (B_TST (bits, bitno))
-	gdb_printf (("1"));
-      else
-	gdb_printf (("0"));
-    }
-}
 
 /* Note the first arg should be the "this" pointer, we may not want to
    include it since we may get into a infinitely recursive
@@ -5004,40 +4985,6 @@  print_cplus_stuff (struct type *type, int spaces)
 	      TYPE_N_BASECLASSES (type));
   gdb_printf ("%*snfn_fields %d\n", spaces, "",
 	      TYPE_NFN_FIELDS (type));
-  if (TYPE_N_BASECLASSES (type) > 0)
-    {
-      gdb_printf
-	("%*svirtual_field_bits (%d bits at *%s)",
-	 spaces, "", TYPE_N_BASECLASSES (type),
-	 host_address_to_string (TYPE_FIELD_VIRTUAL_BITS (type)));
-
-      print_bit_vector (TYPE_FIELD_VIRTUAL_BITS (type),
-			TYPE_N_BASECLASSES (type));
-      gdb_puts ("\n");
-    }
-  if (type->num_fields () > 0)
-    {
-      if (TYPE_FIELD_PRIVATE_BITS (type) != NULL)
-	{
-	  gdb_printf
-	    ("%*sprivate_field_bits (%d bits at *%s)",
-	     spaces, "", type->num_fields (),
-	     host_address_to_string (TYPE_FIELD_PRIVATE_BITS (type)));
-	  print_bit_vector (TYPE_FIELD_PRIVATE_BITS (type),
-			    type->num_fields ());
-	  gdb_puts ("\n");
-	}
-      if (TYPE_FIELD_PROTECTED_BITS (type) != NULL)
-	{
-	  gdb_printf
-	    ("%*sprotected_field_bits (%d bits at *%s",
-	     spaces, "", type->num_fields (),
-	     host_address_to_string (TYPE_FIELD_PROTECTED_BITS (type)));
-	  print_bit_vector (TYPE_FIELD_PROTECTED_BITS (type),
-			    type->num_fields ());
-	  gdb_puts ("\n");
-	}
-    }
   if (TYPE_NFN_FIELDS (type) > 0)
     {
       dump_fn_fieldlists (type, spaces);
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 505c8ba12b5..c72512b8204 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -668,6 +668,46 @@  struct field
     m_loc.dwarf_block = dwarf_block;
   }
 
+  /* True if this field is 'private'.  */
+  bool is_private () const
+  { return m_private; }
+
+  /* Set the field's "private" flag.  */
+  void set_private ()
+  {
+    /* Can't have both.  */
+    gdb_assert (!m_protected);
+    m_private = true;
+  }
+
+  /* True if this field is 'protected'.  */
+  bool is_protected () const
+  { return m_protected; }
+
+  /* Set the field's "protected" flag.  */
+  void set_protected ()
+  {
+    /* Can't have both.  */
+    gdb_assert (!m_private);
+    m_protected = true;
+  }
+
+  /* True if this field is 'virtual'.  */
+  bool is_virtual () const
+  { return m_virtual; }
+
+  /* Set the field's "virtual" flag.  */
+  void set_virtual ()
+  { m_virtual = true; }
+
+  /* True if this field is 'ignored'.  */
+  bool is_ignored () const
+  { return m_ignored; }
+
+  /* Set the field's "ignored" flag.  */
+  void set_ignored ()
+  { m_ignored = true; }
+
   union field_location m_loc;
 
   /* * For a function or member type, this is 1 if the argument is
@@ -677,6 +717,15 @@  struct field
 
   unsigned int m_artificial : 1;
 
+  /* Whether the field is 'private'.  */
+  bool m_private : 1;
+  /* Whether the field is 'protected'.  */
+  bool m_protected : 1;
+  /* Whether the field is 'virtual'.  */
+  bool m_virtual : 1;
+  /* Whether the field is 'ignored'.  */
+  bool m_ignored : 1;
+
   /* * Discriminant for union field_location.  */
 
   ENUM_BITFIELD(field_loc_kind) m_loc_kind : 3;
@@ -1672,42 +1721,6 @@  struct cplus_struct_type
 
     struct type *vptr_basetype;
 
-    /* * For derived classes, the number of base classes is given by
-       n_baseclasses and virtual_field_bits is a bit vector containing
-       one bit per base class.  If the base class is virtual, the
-       corresponding bit will be set.
-       I.E, given:
-
-       class A{};
-       class B{};
-       class C : public B, public virtual A {};
-
-       B is a baseclass of C; A is a virtual baseclass for C.
-       This is a C++ 2.0 language feature.  */
-
-    B_TYPE *virtual_field_bits;
-
-    /* * For classes with private fields, the number of fields is
-       given by nfields and private_field_bits is a bit vector
-       containing one bit per field.
-
-       If the field is private, the corresponding bit will be set.  */
-
-    B_TYPE *private_field_bits;
-
-    /* * For classes with protected fields, the number of fields is
-       given by nfields and protected_field_bits is a bit vector
-       containing one bit per field.
-
-       If the field is private, the corresponding bit will be set.  */
-
-    B_TYPE *protected_field_bits;
-
-    /* * For classes with fields to be ignored, either this is
-       optimized out or this field has length 0.  */
-
-    B_TYPE *ignore_field_bits;
-
     /* * For classes, structures, and unions, a description of each
        field, which consists of an overloaded name, followed by the
        types of arguments that the method expects, and then the name
@@ -1952,37 +1965,16 @@  extern void set_type_vptr_basetype (struct type *, struct type *);
 #define TYPE_CPLUS_DYNAMIC(thistype) TYPE_CPLUS_SPECIFIC (thistype)->is_dynamic
 
 #define BASETYPE_VIA_VIRTUAL(thistype, index) \
-  (TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits == NULL ? 0 \
-    : B_TST(TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits, (index)))
-
-#define TYPE_FIELD_PRIVATE_BITS(thistype) \
-  TYPE_CPLUS_SPECIFIC(thistype)->private_field_bits
-#define TYPE_FIELD_PROTECTED_BITS(thistype) \
-  TYPE_CPLUS_SPECIFIC(thistype)->protected_field_bits
-#define TYPE_FIELD_IGNORE_BITS(thistype) \
-  TYPE_CPLUS_SPECIFIC(thistype)->ignore_field_bits
-#define TYPE_FIELD_VIRTUAL_BITS(thistype) \
-  TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits
-#define SET_TYPE_FIELD_PRIVATE(thistype, n) \
-  B_SET (TYPE_CPLUS_SPECIFIC(thistype)->private_field_bits, (n))
-#define SET_TYPE_FIELD_PROTECTED(thistype, n) \
-  B_SET (TYPE_CPLUS_SPECIFIC(thistype)->protected_field_bits, (n))
-#define SET_TYPE_FIELD_IGNORE(thistype, n) \
-  B_SET (TYPE_CPLUS_SPECIFIC(thistype)->ignore_field_bits, (n))
-#define SET_TYPE_FIELD_VIRTUAL(thistype, n) \
-  B_SET (TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits, (n))
+  ((thistype)->field (index).is_virtual ())
+
 #define TYPE_FIELD_PRIVATE(thistype, n) \
-  (TYPE_CPLUS_SPECIFIC(thistype)->private_field_bits == NULL ? 0 \
-    : B_TST(TYPE_CPLUS_SPECIFIC(thistype)->private_field_bits, (n)))
+  ((thistype)->field (n).is_private ())
 #define TYPE_FIELD_PROTECTED(thistype, n) \
-  (TYPE_CPLUS_SPECIFIC(thistype)->protected_field_bits == NULL ? 0 \
-    : B_TST(TYPE_CPLUS_SPECIFIC(thistype)->protected_field_bits, (n)))
+  ((thistype)->field (n).is_protected ())
 #define TYPE_FIELD_IGNORE(thistype, n) \
-  (TYPE_CPLUS_SPECIFIC(thistype)->ignore_field_bits == NULL ? 0 \
-    : B_TST(TYPE_CPLUS_SPECIFIC(thistype)->ignore_field_bits, (n)))
+  ((thistype)->field (n).is_ignored ())
 #define TYPE_FIELD_VIRTUAL(thistype, n) \
-  (TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits == NULL ? 0 \
-    : B_TST(TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits, (n)))
+  ((thistype)->field (n).is_virtual ())
 
 #define TYPE_FN_FIELDLISTS(thistype) TYPE_CPLUS_SPECIFIC(thistype)->fn_fieldlists
 #define TYPE_FN_FIELDLIST(thistype, n) TYPE_CPLUS_SPECIFIC(thistype)->fn_fieldlists[n]
diff --git a/gdb/stabsread.c b/gdb/stabsread.c
index 7402a26a401..9a84e5f3f80 100644
--- a/gdb/stabsread.c
+++ b/gdb/stabsread.c
@@ -65,11 +65,6 @@  struct stabs_nextfield
 {
   struct stabs_nextfield *next;
 
-  /* This is the raw visibility from the stab.  It is not checked
-     for being one of the visibilities we recognize, so code which
-     examines this field better be able to deal.  */
-  int visibility;
-
   struct field field;
 };
 
@@ -2789,7 +2784,7 @@  read_cpp_abbrev (struct stab_field_info *fip, const char **pp,
       }
       /* This field is unpacked.  */
       fip->list->field.set_bitsize (0);
-      fip->list->visibility = VISIBILITY_PRIVATE;
+      fip->list->field.set_private ();
     }
   else
     {
@@ -2814,15 +2809,42 @@  read_one_struct_field (struct stab_field_info *fip, const char **pp,
   *pp = p + 1;
 
   /* This means we have a visibility for a field coming.  */
+  int visibility;
   if (**pp == '/')
     {
       (*pp)++;
-      fip->list->visibility = *(*pp)++;
+      visibility = *(*pp)++;
     }
   else
     {
       /* normal dbx-style format, no explicit visibility */
-      fip->list->visibility = VISIBILITY_PUBLIC;
+      visibility = VISIBILITY_PUBLIC;
+    }
+
+  switch (visibility)
+    {
+    case VISIBILITY_PRIVATE:
+      fip->list->field.set_private ();
+      break;
+
+    case VISIBILITY_PROTECTED:
+      fip->list->field.set_protected ();
+      break;
+
+    case VISIBILITY_IGNORE:
+      fip->list->field.set_ignored ();
+      break;
+
+    case VISIBILITY_PUBLIC:
+      break;
+
+    default:
+      /* Unknown visibility.  Complain and treat it as public.  */
+      {
+	complaint (_("Unknown visibility `%c' for field"),
+		   visibility);
+      }
+      break;
     }
 
   fip->list->field.set_type (read_type (pp, objfile));
@@ -2892,7 +2914,7 @@  read_one_struct_field (struct stab_field_info *fip, const char **pp,
 	 for dbx compatibility.  */
 
       /* Ignore this field.  */
-      fip->list->visibility = VISIBILITY_IGNORE;
+      fip->list->field.set_ignored ();
     }
   else
     {
@@ -3066,21 +3088,6 @@  read_baseclasses (struct stab_field_info *fip, const char **pp,
       return 0;
   }
 
-#if 0
-  /* Some stupid compilers have trouble with the following, so break
-     it up into simpler expressions.  */
-  TYPE_FIELD_VIRTUAL_BITS (type) = (B_TYPE *)
-    TYPE_ZALLOC (type, B_BYTES (TYPE_N_BASECLASSES (type)));
-#else
-  {
-    int num_bytes = B_BYTES (TYPE_N_BASECLASSES (type));
-    char *pointer;
-
-    pointer = (char *) TYPE_ZALLOC (type, num_bytes);
-    TYPE_FIELD_VIRTUAL_BITS (type) = (B_TYPE *) pointer;
-  }
-#endif /* 0 */
-
   for (i = 0; i < TYPE_N_BASECLASSES (type); i++)
     {
       newobj = OBSTACK_ZALLOC (&fip->obstack, struct stabs_nextfield);
@@ -3097,7 +3104,7 @@  read_baseclasses (struct stab_field_info *fip, const char **pp,
 	  /* Nothing to do.  */
 	  break;
 	case '1':
-	  SET_TYPE_FIELD_VIRTUAL (type, i);
+	  newobj->field.set_virtual ();
 	  break;
 	default:
 	  /* Unknown character.  Complain and treat it as non-virtual.  */
@@ -3108,11 +3115,15 @@  read_baseclasses (struct stab_field_info *fip, const char **pp,
 	}
       ++(*pp);
 
-      newobj->visibility = *(*pp)++;
-      switch (newobj->visibility)
+      int visibility = *(*pp)++;
+      switch (visibility)
 	{
 	case VISIBILITY_PRIVATE:
+	  newobj->field.set_private ();
+	  break;
 	case VISIBILITY_PROTECTED:
+	  newobj->field.set_private ();
+	  break;
 	case VISIBILITY_PUBLIC:
 	  break;
 	default:
@@ -3120,8 +3131,7 @@  read_baseclasses (struct stab_field_info *fip, const char **pp,
 	     public.  */
 	  {
 	    complaint (_("Unknown visibility `%c' for baseclass"),
-		       newobj->visibility);
-	    newobj->visibility = VISIBILITY_PUBLIC;
+		       visibility);
 	  }
 	}
 
@@ -3268,43 +3278,19 @@  attach_fields_to_type (struct stab_field_info *fip, struct type *type,
 		       struct objfile *objfile)
 {
   int nfields = 0;
-  int non_public_fields = 0;
   struct stabs_nextfield *scan;
 
-  /* Count up the number of fields that we have, as well as taking note of
-     whether or not there are any non-public fields, which requires us to
-     allocate and build the private_field_bits and protected_field_bits
-     bitfields.  */
+  /* Count up the number of fields that we have.  */
 
   for (scan = fip->list; scan != NULL; scan = scan->next)
-    {
-      nfields++;
-      if (scan->visibility != VISIBILITY_PUBLIC)
-	{
-	  non_public_fields++;
-	}
-    }
+    nfields++;
 
   /* Now we know how many fields there are, and whether or not there are any
      non-public fields.  Record the field count, allocate space for the
-     array of fields, and create blank visibility bitfields if necessary.  */
+     array of fields.  */
 
   type->alloc_fields (nfields);
 
-  if (non_public_fields)
-    {
-      ALLOCATE_CPLUS_STRUCT_TYPE (type);
-
-      TYPE_FIELD_PRIVATE_BITS (type) =
-	(B_TYPE *) TYPE_ZALLOC (type, B_BYTES (nfields));
-
-      TYPE_FIELD_PROTECTED_BITS (type) =
-	(B_TYPE *) TYPE_ZALLOC (type, B_BYTES (nfields));
-
-      TYPE_FIELD_IGNORE_BITS (type) =
-	(B_TYPE *) TYPE_ZALLOC (type, B_BYTES (nfields));
-    }
-
   /* Copy the saved-up fields into the field vector.  Start from the
      head of the list, adding to the tail of the field array, so that
      they end up in the same order in the array in which they were
@@ -3313,31 +3299,6 @@  attach_fields_to_type (struct stab_field_info *fip, struct type *type,
   while (nfields-- > 0)
     {
       type->field (nfields) = fip->list->field;
-      switch (fip->list->visibility)
-	{
-	case VISIBILITY_PRIVATE:
-	  SET_TYPE_FIELD_PRIVATE (type, nfields);
-	  break;
-
-	case VISIBILITY_PROTECTED:
-	  SET_TYPE_FIELD_PROTECTED (type, nfields);
-	  break;
-
-	case VISIBILITY_IGNORE:
-	  SET_TYPE_FIELD_IGNORE (type, nfields);
-	  break;
-
-	case VISIBILITY_PUBLIC:
-	  break;
-
-	default:
-	  /* Unknown visibility.  Complain and treat it as public.  */
-	  {
-	    complaint (_("Unknown visibility `%c' for field"),
-		       fip->list->visibility);
-	  }
-	  break;
-	}
       fip->list = fip->list->next;
     }
   return 1;