Request for review: JDK-8004728 Hotspot support for parameter reflection

[Eric McCorkle]

On 01/05/13 01:39, John Rose wrote:

> Cool stuff!  One quick comment on this loop in the CFP:
> 
> +  // Copy method parameters
> +  if (method_parameters_length > 0) {
> +    MethodParametersElement* elem = m->constMethod()->method_parameters_start();
> +    for(int i = 0; i < method_parameters_length; i++) {
> +      elem[i].name_cp_index =
> +	Bytes::get_Java_u2(method_parameters_data);
> +      method_parameters_data += 2;
> +      elem[i].flags = Bytes::get_Java_u4(method_parameters_data);
> +      method_parameters_data += 4;
> +    }
> +  }
> 
> This uses a C style (or C++ STL style) bumped pointer (*elemptr++).
> 
> In the Hotspot source base is is more common, and preferable, to use the array/index style (array[index++]).
> 

I agree with this in principle; however, I'd like to leave this as is
for now (though with the suggested variable name change made), if that's
ok, for the following reasons:

1) I actually copied this way of doing this from another place in CFP (I
forget exactly where)

2) MethodParameters data is heterogenous (ie composed of u2's and u4's
interspersed), and bumping and casting a pointer is really the only way
to read such data portably[0]

3) I know for certain that the code I have now works, and it is very
easy to introduce subtle errors in this kind of code by revising it.

4) There is talk of completely reworking CFP at some point after SE8.



[0]: As far as I'm aware, neither C nor C++ make any solid guarantees
regarding the offsets of structure fields (I yield to any C++ language
attorneys that say otherwise), and given that the u2
parameter_name_index comes before the u4 modifiers field (which is
likely to result in a misaligned modifiers field, which compilers may
attempt to correct), I'm not terribly comfortable trusting a compiler to
generate the right offsets.