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

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Fri, 15 May 2015 19:04:03 +0200, Pedro Alves wrote:
> 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.

It was written this way as it has semantical logic - how the type is
determined depends on TYPE_CODE_* of that object.  Just accidentally in this
case it is the same.  I do not do performance micro-optimizations which
compiler will do anyway.  But I have changed it.


> > +  /* Inferior parameter out value address or NULL if the inferior function does not
> > +     have one.  */
> 
> Line too long, I think.

Yes, fixed.  (Not discussing the 72 vs. 78 or 80 or which width of comments
rule.)


> Also, "NULL if the inferior function does not have one" looks like
> a copy/paste, given this is not a pointer variable.

It is "inferior NULL", so I find that "NULL" valid there.


> 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;

Done although I have added line "Otherwise it is zero." there:

+  /* 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.
+     Otherwise it is zero.  */
+  CORE_ADDR out_value_addr;


Without that line 'out_value_addr' would be unspecified if out_value_type is
NULL which does not correspond to the current code.


Checked in:
	36de76f9cc2eea0bd5f1b7ce74ef60e1aa0b27c2


Jan