RFR: 8313435: Clean up unused default methods code [v2]

[Coleen Phillimore]

On Wed, 2 Aug 2023 07:30:29 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Align parameters.
>
> src/hotspot/share/classfile/defaultMethods.cpp line 1060:
> 
>> 1058: 
>> 1059:   int new_methods_length = klass->methods()->length() + new_methods->length();
>> 1060:   if (new_methods_length > std::numeric_limits<u2>::max()) {
> 
> Again do we need a header file include for this?

I was somewhat surprised that the compilers didn't complain.  I think I should have used USHRT_MAX which is in limits.h and included in globalDefinitions*hpp.

> src/hotspot/share/classfile/defaultMethods.cpp line 1061:
> 
>> 1059:   int new_methods_length = klass->methods()->length() + new_methods->length();
>> 1060:   if (new_methods_length > std::numeric_limits<u2>::max()) {
>> 1061:     THROW_MSG(vmSymbols::java_lang_InternalError(), "default methods error methods overflow");
> 
> That message doesn't quite read right to me - not clear exactly what has overflowed here.

Default methods adds methods to the InstanceKlass::_methods array.  That array can only have u2 number of methods because it's in the JVMLS and also each method has a u2 method idnum (not to be confused with jmethodID).
It's not easy to explain and and I'm not wholly convinced it's possible, because it's filling in vtable slots, which hopefully something has checked that there are < u2 entries beforehand.
Maybe this should just be an assert.

> src/hotspot/share/classfile/defaultMethods.cpp line 1081:
> 
>> 1079:   int new_idx = 0;
>> 1080: 
>> 1081:   for (u2 i = 0; i < new_size; ++i) {
> 
> Is this change for the loop index necessary? Even as an int it is constrained to be <= max(u2) - and `at_put` expects an int index doesn't it?

Later it uses 'i' to be method_idnum, which is constrained to be u2.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/15095#discussion_r1281819681
PR Review Comment: https://git.openjdk.org/jdk/pull/15095#discussion_r1281833207
PR Review Comment: https://git.openjdk.org/jdk/pull/15095#discussion_r1281823672