Re: [PATCH v5 6/7] compile: New 'compile print'

[Pedro Alves <palves@redhat.com>]

A couple very minor nits below, and this is good to go.

On 05/13/2015 09:17 PM, Jan Kratochvil wrote:

> +  if (TYPE_CODE (gdb_type) != TYPE_CODE_PTR)
> +    error (_("Invalid type code %d of symbol \"%s\" "
> +	     "in compiled module \"%s\"."),
> +	   TYPE_CODE (gdb_type_from_ptr), COMPILE_I_EXPR_VAL,
> +	   objfile_name (objfile));
> +  
> +  switch (TYPE_CODE (gdb_type_from_ptr))
> +    {
> +    case TYPE_CODE_ARRAY:
> +      retval = gdb_type_from_ptr;
> +      gdb_type_from_ptr = TYPE_TARGET_TYPE (gdb_type_from_ptr);
> +      break;
> +    case TYPE_CODE_FUNC:
> +      retval = gdb_type_from_ptr;

AFAIC, retval is always gdb_type_from_ptr, and could be
moved out of the switch.

> +      break;
> +    default:
> +      error (_("Invalid type code %d of symbol \"%s\" "
> +	       "in compiled module \"%s\"."),
> +	     TYPE_CODE (gdb_type_from_ptr), COMPILE_I_EXPR_PTR_TYPE,
> +	     objfile_name (objfile));
> +    }

> diff --git a/gdb/compile/compile-object-load.h b/gdb/compile/compile-object-load.h
> index f5e887d..3e0e45b 100644
> --- a/gdb/compile/compile-object-load.h
> +++ b/gdb/compile/compile-object-load.h
> @@ -37,6 +37,14 @@ struct compile_module
>  
>    /* User data for SCOPE in use.  */
>    void *scope_data;
> +
> +  /* Inferior parameter out value address or NULL if the inferior function does not
> +     have one.  */

Line too long, I think.  Also, "NULL if the inferior function does not
have one" looks like a copy/paste, given this is not a pointer
variable.   I'd swap around the fields, and write:

  /* Inferior parameter out value type or NULL if the inferior function does not
     have one.  */
  struct type *out_value_type;

  /* If the inferior function has an out value, this is its address.  */
  CORE_ADDR out_value_addr;

> +  CORE_ADDR out_value_addr;
> +
> +  /* Inferior parameter out value type or NULL if the inferior function does not
> +     have one.  */
> +  struct type *out_value_type;
>  };
>  

Thanks,
Pedro Alves