On Tue, 2014-07-01 at 10:04 -0600, Tom Tromey wrote:
> Mark> +   the cv-qualifiers.  This is mimics section 6.7.3 of the C99
> Mark> +   specification.  */
> 
> "This is" -> "This".

Fixed.

> Mark> +static struct type *
> Mark> +add_array_cv_type (struct die_info *die, struct dwarf2_cu *cu,
> Mark> +		   struct type *base_type, int cnst, int voltl)
> 
> gdb usually puts a blank line between the intro comment and the function
> head.

Fixed.

> Mark> +  while (TYPE_CODE (TYPE_TARGET_TYPE (inner_array)) == TYPE_CODE_ARRAY)
> Mark> +    {
> Mark> +      TYPE_TARGET_TYPE (inner_array) =
> Mark> +	copy_type (TYPE_TARGET_TYPE (inner_array));
> Mark> +      inner_array = TYPE_TARGET_TYPE (inner_array);
> Mark> +    }
> 
> I think this may be overly eager in the case where the underlying type
> already has the needed qualifications.

But we have to look anyway. This is just the existing code, moved into
its own function. What do you suggest should be changed?

> Mark> +  cnst |= TYPE_CONST (el_type);
> Mark> +  voltl |= TYPE_VOLATILE (el_type);
> Mark> +  TYPE_TARGET_TYPE (inner_array) = make_cv_type (cnst, voltl, el_type, NULL);
> 
> make_cv_type preserves the already existing qualifiers so you don't need
> to track "cnst" and "voltl".  You can just pass in the ones you want to
> add.

The original code already did it this way. And I don't think it is true
that make_cv_type preserves existing const or volatile qualifiers. The
first thing make_cv_type does is mask away the old const and volatile
flags. Without explicitly preserving the existing flags 2 of the new
testcases, the const volatile arrays tests, fail (note one of the
qualifiers is missing):

ptype vindictive
type = const char [2]
(gdb) FAIL: gdb.base/volatile.exp: ptype vindictive
ptype vegetation
type = const unsigned char [2]
(gdb) FAIL: gdb.base/volatile.exp: ptype vegetation

> Mark> +  /* In case the volatile qualifier is applied to an array type, the
> Mark> +     element type is so qualified, not the array type (section 6.7.3
> Mark> +     of C99).  */
> Mark> +  if (TYPE_CODE (base_type) == TYPE_CODE_ARRAY)
> Mark> +    return add_array_cv_type (die, cu, base_type, 0, 1);
> 
> I wonder about typedefs to array type.
> Calling check_typedef here is probably not so great but I assume we can
> just ignore incomplete types.

I am not sure I understand precisely what you are wondering about. Do
you have an example? Then I can add a testcase for it. This is just
precisely as is done for the const case. So the issue you are thinking
about is probably the same for both cases and might already be existing.

Thanks,

Mark