[v3,8/9] Use enum accessibility in types and member functions

  This changes nested types and member functions to use the new
'accessibility' enum, rather than separate private/protected flags.
This is done for consistency, but it also lets us simplify some other
code in the next patch.

Acked-By: Simon Marchi <simon.marchi@efficios.com>
Reviewed-by: Keith Seitz <keiths@redhat.com>
---
 gdb/dwarf2/read.c |  8 ++++----
 gdb/gdbtypes.h    | 27 ++++++++++++++-------------
 gdb/stabsread.c   |  4 ++--
 3 files changed, 20 insertions(+), 19 deletions(-)
  

On 11/3/23 17:09, Tom Tromey wrote:
> This changes nested types and member functions to use the new
> 'accessibility' enum, rather than separate private/protected flags.
> This is done for consistency, but it also lets us simplify some other
> code in the next patch.
> 

FWIW, this breaks the build for me with system gcc 7.5.0:
...
/data/vries/gdb/src/gdb/gdbtypes.h:721:51: error: 
‘field::m_accessibility’ is too small to hold all values of ‘enum class 
accessibility’ [-Werror]
    ENUM_BITFIELD (accessibility) m_accessibility : 2;
                                                    ^
/data/vries/gdb/src/gdb/gdbtypes.h:1615:49: error: 
‘fn_field::accessibility’ is too small to hold all values of ‘enum class 
accessibility’ [-Werror]
    ENUM_BITFIELD (accessibility) accessibility : 2;
                                                  ^
/data/vries/gdb/src/gdb/gdbtypes.h:1661:49: error: 
‘decl_field::accessibility’ is too small to hold all values of ‘enum 
class accessibility’ [-Werror]
    ENUM_BITFIELD (accessibility) accessibility : 2;
                                                  ^
...

I can fix this by using --disable-werror.

Also builds fine when I switch to gcc 9.3.1 with --enable-werror.

Thanks,
- Tom

> Acked-By: Simon Marchi <simon.marchi@efficios.com>
> Reviewed-by: Keith Seitz <keiths@redhat.com>
> ---
>   gdb/dwarf2/read.c |  8 ++++----
>   gdb/gdbtypes.h    | 27 ++++++++++++++-------------
>   gdb/stabsread.c   |  4 ++--
>   3 files changed, 20 insertions(+), 19 deletions(-)
> 
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index e52ca5b5b83..0d8c24ad5b1 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -11714,10 +11714,10 @@ dwarf2_add_type_defn (struct field_info *fip, struct die_info *die,
>         /* The assumed value if neither private nor protected.  */
>         break;
>       case DW_ACCESS_private:
> -      fp.is_private = 1;
> +      fp.accessibility = accessibility::PRIVATE;
>         break;
>       case DW_ACCESS_protected:
> -      fp.is_protected = 1;
> +      fp.accessibility = accessibility::PROTECTED;
>         break;
>       }
>   
> @@ -12076,10 +12076,10 @@ dwarf2_add_member_fn (struct field_info *fip, struct die_info *die,
>     switch (accessibility)
>       {
>       case DW_ACCESS_private:
> -      fnp->is_private = 1;
> +      fnp->accessibility = accessibility::PRIVATE;
>         break;
>       case DW_ACCESS_protected:
> -      fnp->is_protected = 1;
> +      fnp->accessibility = accessibility::PROTECTED;
>         break;
>       }
>   
> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> index bec69bdc3d1..d3c7a206b34 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -1611,8 +1611,8 @@ struct fn_field
>   
>     unsigned int is_const:1;
>     unsigned int is_volatile:1;
> -  unsigned int is_private:1;
> -  unsigned int is_protected:1;
> +  /* Accessibility of the field.  */
> +  ENUM_BITFIELD (accessibility) accessibility : 2;
>     unsigned int is_artificial:1;
>   
>     /* * A stub method only has some fields valid (but they are enough
> @@ -1657,11 +1657,8 @@ struct decl_field
>   
>     struct type *type;
>   
> -  /* * True if this field was declared protected, false otherwise.  */
> -  unsigned int is_protected : 1;
> -
> -  /* * True if this field was declared private, false otherwise.  */
> -  unsigned int is_private : 1;
> +  /* Accessibility of the field.  */
> +  ENUM_BITFIELD (accessibility) accessibility : 2;
>   };
>   
>   /* * C++ language-specific information for TYPE_CODE_STRUCT and
> @@ -1984,8 +1981,10 @@ extern void set_type_vptr_basetype (struct type *, struct type *);
>   #define TYPE_FN_FIELD_ARGS(thisfn, n) (((thisfn)[n].type)->fields ())
>   #define TYPE_FN_FIELD_CONST(thisfn, n) ((thisfn)[n].is_const)
>   #define TYPE_FN_FIELD_VOLATILE(thisfn, n) ((thisfn)[n].is_volatile)
> -#define TYPE_FN_FIELD_PRIVATE(thisfn, n) ((thisfn)[n].is_private)
> -#define TYPE_FN_FIELD_PROTECTED(thisfn, n) ((thisfn)[n].is_protected)
> +#define TYPE_FN_FIELD_PRIVATE(thisfn, n) \
> +  ((thisfn)[n].accessibility == accessibility::PRIVATE)
> +#define TYPE_FN_FIELD_PROTECTED(thisfn, n) \
> +  ((thisfn)[n].accessibility == accessibility::PROTECTED)
>   #define TYPE_FN_FIELD_ARTIFICIAL(thisfn, n) ((thisfn)[n].is_artificial)
>   #define TYPE_FN_FIELD_STUB(thisfn, n) ((thisfn)[n].is_stub)
>   #define TYPE_FN_FIELD_CONSTRUCTOR(thisfn, n) ((thisfn)[n].is_constructor)
> @@ -2008,9 +2007,9 @@ extern void set_type_vptr_basetype (struct type *, struct type *);
>   #define TYPE_TYPEDEF_FIELD_COUNT(thistype) \
>     TYPE_CPLUS_SPECIFIC (thistype)->typedef_field_count
>   #define TYPE_TYPEDEF_FIELD_PROTECTED(thistype, n) \
> -  TYPE_TYPEDEF_FIELD (thistype, n).is_protected
> +  (TYPE_TYPEDEF_FIELD (thistype, n).accessibility == accessibility::PROTECTED)
>   #define TYPE_TYPEDEF_FIELD_PRIVATE(thistype, n)        \
> -  TYPE_TYPEDEF_FIELD (thistype, n).is_private
> +  (TYPE_TYPEDEF_FIELD (thistype, n).accessibility == accessibility::PRIVATE)
>   
>   #define TYPE_NESTED_TYPES_ARRAY(thistype)	\
>     TYPE_CPLUS_SPECIFIC (thistype)->nested_types
> @@ -2023,9 +2022,11 @@ extern void set_type_vptr_basetype (struct type *, struct type *);
>   #define TYPE_NESTED_TYPES_COUNT(thistype) \
>     TYPE_CPLUS_SPECIFIC (thistype)->nested_types_count
>   #define TYPE_NESTED_TYPES_FIELD_PROTECTED(thistype, n) \
> -  TYPE_NESTED_TYPES_FIELD (thistype, n).is_protected
> +  (TYPE_NESTED_TYPES_FIELD (thistype, n).accessibility \
> +   == accessibility::PROTECTED)
>   #define TYPE_NESTED_TYPES_FIELD_PRIVATE(thistype, n)	\
> -  TYPE_NESTED_TYPES_FIELD (thistype, n).is_private
> +  (TYPE_NESTED_TYPES_FIELD (thistype, n).accessibility \
> +   == accessibility::PRIVATE)
>   
>   #define TYPE_IS_OPAQUE(thistype) \
>     ((((thistype)->code () == TYPE_CODE_STRUCT) \
> diff --git a/gdb/stabsread.c b/gdb/stabsread.c
> index 334371c2e46..fbf11701337 100644
> --- a/gdb/stabsread.c
> +++ b/gdb/stabsread.c
> @@ -2372,10 +2372,10 @@ read_member_functions (struct stab_field_info *fip, const char **pp,
>   	  switch (*(*pp)++)
>   	    {
>   	    case VISIBILITY_PRIVATE:
> -	      new_sublist->fn_field.is_private = 1;
> +	      new_sublist->fn_field.accessibility = accessibility::PRIVATE;
>   	      break;
>   	    case VISIBILITY_PROTECTED:
> -	      new_sublist->fn_field.is_protected = 1;
> +	      new_sublist->fn_field.accessibility = accessibility::PROTECTED;
>   	      break;
>   	    }
>   
>
  

Tom> FWIW, this breaks the build for me with system gcc 7.5.0:

Mark mentioned something like this yesterday, but didn't say which
patch...  it's a GCC bug.

However, I will push a gdb fix momentarily.

Tom
  

Tom> However, I will push a gdb fix momentarily.

Actually, since I can't test it, I'll just send the patch.  I'll CC you.
Please try it out.

Tom