RFR: 8221503: vmTestbase/nsk/jdb/eval/eval001/eval001.java fails with: com.sun.jdi.InvalidTypeException: Can't assign double[][][] to double[][][] [v2]

[Fairoz Matte]

On Thu, 29 Apr 2021 04:31:35 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

>> Hi Chris, 
>> 
>> Thanks for looking into this, 
>> 
>>>Do we even need findComponentType() any more? Isn't ReferenceTypeImpl.findType() sufficient.
>> 
>> We still need findComponentType(), 
>> Difference between findType() and findComponentType() is that, findComponentType() tries to get the list of ReferenceType from the "vm.classesByName". In case list is empty, it explicitly throws ClassNotLoadedException.
>> This exception check is required in validateAssignment(ValueContainer destination) call from ObjectReferenceImpl.java.
>> 
>> https://github.com/openjdk/jdk/blob/master/src/jdk.jdi/share/classes/com/sun/tools/jdi/ObjectReferenceImpl.java#L579
>> 
>> 
>> 
>> I have modified the method a bit to handle that scenario
>> 
>> 
>> diff --git a/src/jdk.jdi/share/classes/com/sun/tools/jdi/ArrayTypeImpl.java b/src/jdk.jdi/share/classes/com/sun/tools/jdi/ArrayTypeImpl.java
>> index e544c81ae3e..54deba43894 100644
>> --- a/src/jdk.jdi/share/classes/com/sun/tools/jdi/ArrayTypeImpl.java
>> +++ b/src/jdk.jdi/share/classes/com/sun/tools/jdi/ArrayTypeImpl.java
>> @@ -95,20 +95,13 @@ public class ArrayTypeImpl extends ReferenceTypeImpl
>>          JNITypeParser sig = new JNITypeParser(signature);
>>          if (sig.isReference()) {
>>              // It's a reference type
>> -            JNITypeParser parser = new JNITypeParser(componentSignature());
>> -            List<ReferenceType> list = vm.classesByName(parser.typeName());
>> -            Iterator<ReferenceType> iter = list.iterator();
>> -            while (iter.hasNext()) {
>> -                ReferenceType type = iter.next();
>> -                ClassLoaderReference cl = type.classLoader();
>> -                if ((cl == null)?
>> -                         (classLoader() == null) :
>> -                         (cl.equals(classLoader()))) {
>> -                    return type;
>> -                }
>> +           try {
>> +             Type componentType = findType(componentSignature());
>> +             return componentType;
>> +              // Component class has not yet been loaded
>> +           } catch (ClassNotLoadedException ex) {
>> +               throw new ClassNotLoadedException(componentTypeName());
>>              }
>> -            // Component class has not yet been loaded
>> -            throw new ClassNotLoadedException(componentTypeName());
>>          } else {
>>              // It's a primitive type
>>              return vm.primitiveTypeMirror(sig.jdwpTag());```
>> 
>> Thanks,
>
>> > Do we even need findComponentType() any more? Isn't ReferenceTypeImpl.findType() sufficient.
>> 
>> We still need findComponentType(),
>> Difference between findType() and findComponentType() is that, findComponentType() tries to get the list of ReferenceType from the "vm.classesByName". In case list is empty, it explicitly throws ClassNotLoadedException.
>> This exception check is required in validateAssignment(ValueContainer destination) call from ObjectReferenceImpl.java.
> 
> I'm not sure what you mean by this. After your changes, this is all `findComponentType()` does:
> 
> 
>     Type findComponentType(String signature) throws ClassNotLoadedException {
>         return findType(signature);
>     }
> 
> 
> And `findType()` has the exact same signature, including the `throws`:
> 
> `    Type findType(String signature) throws ClassNotLoadedException {`
> 
> 
> So my suggestion is to get rid of `findComponentType()` and just have current users call `findType()` instead.

Thanks Chris, for the review comments.
I have modified the suggested changes, kindly review.

Thanks,

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

PR: https://git.openjdk.java.net/jdk/pull/3658