Re: [PATCH v4 05/13] vla: update type from newly created value

[Joel Brobecker <brobecker@adacore.com>]

> Constructing a value based on a type and address might change the type
> of the newly constructed value. Thus re-fetch type via value_type to ensure
> we have the correct type at hand.
> 
> 2013-10-18  Sanimir Agovic  <sanimir.agovic@intel.com>
>             Keven Boell  <keven.boell@intel.com>
> 
> 	* ada-lang.c (ada_value_primitive_packed_val): Re-fetch type from value.
> 	(ada_template_to_fixed_record_type_1): Likewise.
> 	(ada_to_fixed_type_1): Likewise.
> 	* cp-valprint.c (cp_print_value_fields_rtti): Likewise.
> 	(cp_print_value): Likewise.
> 	* d-valprint.c (dynamic_array_type): Likewise.
> 	* jv-valprint.c (java_value_print): Likewise.
> 	* valops.c (value_ind): Likewise.
> 	* value.c (coerce_ref): Likewise.

This patch makes me a little nervous, but unfortunately, the only option
I have to help with that is going to be a little labor-intensive, so
may not be practical. I'll leave it to you and the maintainers who have
been reviewing your patches so far.

I see that the type re-fetch was not added systematically, but only in
some of the locations where value_at/value_at_lazy/
value_from_contents_and_address are being called?  Was it in order
to fix some regressions revealed by the testsuite?  If that's the case,
I think this patch should be merged with the patch that causes the
regressions, just to make sure that each patch individually causes
no regression. This also helps when using the bisect feature of git.

It looks like it's very easy to miss a case where we should re-fetch
the type, which is what makes me slightly nervous. It's also putting
the onus on the user to remember that value_[...] may return a value
whose type is different from the one he has. So, now the labor-intensive
suggestion: How about adding a type parameter to those 3 functions,
to force the user to think about it and give him a chance to DTRT from
the start? This parameter would be allowed to be NULL in the cases were
we know we don't need to re-fecth. Another advantage I see from
this approach is that vendor code-bases would stop building as soon as
your change is imported, directing their attention to this change and
the implicit question, for each location, that needs to be answered.

I did a quick grep, and counted about 70 locations that would need
to be adjusted if we were to go that route.

-- 
Joel