Re: [PATCH v2 03/11] [PR gdb/14441] gdb: valops: add ability to return rvalue reference values from value_ref()

[Keith Seitz <keiths@redhat.com>]

Just "nits" in this review.

On 01/19/2016 10:53 AM, Artemiy Volkov wrote:
> 2016-01-19  Artemiy Volkov  <artemiyv@acm.org>
> 
>         * gdb/ada-lang.c (ada_evaluate_subexp): Adhere to the new
>         value_ref() interface.
>         * gdb/c-valprint.c (c_value_print): Likewise.
>         * gdb/infcall.c (value_arg_coerce): Likewise.
>         * gdb/python/py-value.c (valpy_reference_value): Likewise.
>         * gdb/valops.c (value_cast): Likewise.
>         (value_reinterpret_cast): Likewise.
>         (value_dynamic_cast): Likewise.
>         (value_ref): Parameterize by kind of return value reference type.
>         (typecmp): Likewise.

Another place that could be simplified, removing "likewise."

>         * gdb/value.h: New interface.

This isn't a particularly useful comment. ChangeLogs explain what changed:

	* gdb/value.h (value_ref): Add new parameter "refcode".

> diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c
> index 62552ec..e210e99 100644
> --- a/gdb/c-valprint.c
> +++ b/gdb/c-valprint.c
> @@ -602,10 +602,13 @@ c_value_print (struct value *val, struct ui_file *stream,
>        else if (options->objectprint
>  	       && (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_STRUCT))
>  	{
> -	  int is_ref = TYPE_CODE (type) == TYPE_CODE_REF;
> +         int is_ref = TYPE_REFERENCE (type);
> +         enum type_code refcode = TYPE_CODE_UNDEF;

Indentation?

>  
> -	  if (is_ref)
> +	  if (is_ref) {
>  	    val = value_addr (val);
> +           refcode = TYPE_CODE (type);
> +         }
>  
>  	  /* Pointer to class, check real type of object.  */
>  	  fprintf_filtered (stream, "(");
> diff --git a/gdb/valops.c b/gdb/valops.c
> index 1aafb5a..cc01689 100644
> --- a/gdb/valops.c
> +++ b/gdb/valops.c
> @@ -828,7 +829,7 @@ value_dynamic_cast (struct type *type, struct value *arg)
>  			       value_address (tem), tem,
>  			       rtti_type, &result) == 1)
>      return value_cast (type,
> -		       is_ref ? value_ref (result) : value_addr (result));
> +		       is_ref ? value_ref (result, TYPE_CODE (resolved_type)) : value_addr (result));
>  
>    if (TYPE_CODE (resolved_type) == TYPE_CODE_PTR)
>      return value_zero (type, not_lval);

The two changed lines in the above hunk are too long.

> @@ -1499,16 +1500,19 @@ value_addr (struct value *arg1)
>     contents.  */
>  
>  struct value *
> -value_ref (struct value *arg1)
> +value_ref (struct value *arg1, enum type_code refcode)
>  {
>    struct value *arg2;
>    struct type *type = check_typedef (value_type (arg1));
>  
> -  if (TYPE_CODE (type) == TYPE_CODE_REF)
> +  gdb_assert (refcode == TYPE_CODE_REF || refcode == TYPE_CODE_RVALUE_REF);
> +
> +  if ((TYPE_CODE (type) == TYPE_CODE_REF || TYPE_CODE (type) == TYPE_CODE_RVALUE_REF)
> +      && TYPE_CODE (type) == refcode)
>      return arg1;
>  

The "if ((TYPE_CODE...)" line is too long.

>    arg2 = value_addr (arg1);
> -  deprecated_set_value_type (arg2, lookup_lvalue_reference_type (type));
> +  deprecated_set_value_type (arg2, lookup_reference_type (type, refcode));
>    return arg2;
>  }
>  

Keith