Re: [PATCH v2 01/11] [PR gdb/14441] gdb: gdbtypes: add definitions for rvalue reference type

[Keith Seitz <keiths at redhat dot com>]

On 01/19/2016 10:53 AM, Artemiy Volkov wrote:
> 2016-01-19  Artemiy Volkov  <artemiyv@acm.org>
> 
>         * gdb/gdbtypes.h (enum type_code): Add TYPE_CODE_RVALUE_REF
>         constant.

Nit: I believe the syntax for this would be:

  (enum type_code) <TYPE_CODE_RVALUE_REF>: New constant.

Also, remove the leading "gdb/" from all gdb/ChangeLog entries. [This
appears in all patches.]

>         (TYPE_REFERENCE): New macro.
>         (struct type): Add rvalue_reference_type field.
>         (TYPE_RVALUE_REFERENCE_TYPE): New macro.
> ---
>  gdb/gdbtypes.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> index e775a1d..52419b4 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -362,6 +364,12 @@ enum type_instance_flag_value
>  #define TYPE_ATOMIC(t) \
>    (TYPE_INSTANCE_FLAGS (t) & TYPE_INSTANCE_FLAG_ATOMIC)
>  
> +/* * C++ lvalue and rvalue references are equivalent in many contexts,
> +   thus create a convenience macro that checks if a type is either of them. */
> +
> +#define TYPE_REFERENCE(t) \
> +  (TYPE_CODE(t) == TYPE_CODE_REF || TYPE_CODE(t) == TYPE_CODE_RVALUE_REF)
> +

Nit: a single whitespace between TYPE_CODE and '(', TYPE_CODE (t) ...
I think this macro could have a more helpful name. It's very close to
TYPE_REFERENCE_TYPE. How about TYPE_IS_REFERENCE (similarly named to
TYPE_IS_OPAQUE)? Then the comment becomes even clearer: "True if this
type represents either an lvalue or rvalue reference type."

>  /* * Instruction-space delimited type.  This is for Harvard architectures
>     which have separate instruction and data address spaces (and perhaps
>     others).
> @@ -767,6 +775,10 @@ struct type
>  
>    struct type *reference_type;
>  
> +  /* * A C++ rvalue reference type added in C++0x. */
> +
> +  struct type *rvalue_reference_type;
> +

Why is this new field necessary? AFAICT, it is used exactly the same way
as the reference_type field above it, and whether a reference type is an
rvalue type is encoded into the type code.

>    /* * Variant chain.  This points to a type that differs from this
>       one only in qualifiers and length.  Currently, the possible
>       qualifiers are const, volatile, code-space, data-space, and
> @@ -1229,6 +1241,7 @@ extern void allocate_gnat_aux_type (struct type *);
>  #define TYPE_TARGET_TYPE(thistype) TYPE_MAIN_TYPE(thistype)->target_type
>  #define TYPE_POINTER_TYPE(thistype) (thistype)->pointer_type
>  #define TYPE_REFERENCE_TYPE(thistype) (thistype)->reference_type
> +#define TYPE_RVALUE_REFERENCE_TYPE(thistype) (thistype)->rvalue_reference_type
>  #define TYPE_CHAIN(thistype) (thistype)->chain
>  /* * Note that if thistype is a TYPEDEF type, you have to call check_typedef.
>     But check_typedef does set the TYPE_LENGTH of the TYPEDEF type,

If struct type.rvalue_reference_type is superfluous, this is unneeded.

Keith