RFR: JDK-8211027 jdb "eval" should perform substitutability test when applying == to inline types

Roger Riggs Roger.Riggs at oracle.com
Tue Oct 15 15:31:11 UTC 2019


Hi,

Be careful about spreading around multiple implementations of 
isSubsitiutable.
Perhaps add a comment refering to the 'official' implementation.

Roger


On 10/15/19 11:24 AM, David Simms wrote:
>
> Ah, thanks for the context ! Go ahead and push
>
> /D
>
> On 15/10/19 2:29 PM, Frederic Parain wrote:
>> Thank you for reviewing this.
>>
>> Spelling fixed, and a few minor additional fixes in the new web rev:
>>
>> http://cr.openjdk.java.net/~fparain/jdi_substitutability/webrev.01/
>>
>> ValueBootstrapMethods.isSubstitutable() cannot be used in this 
>> particular
>> case. The code is executed in the debugger, not in the target VM. The
>> isSubstitutable() method doesn’t operate directly on instances of inline
>> types, but on instances of ObjectReferenceImpl which represent objects
>> in the target VM.
>>
>>   152     private boolean isSubstitutable(ObjectReferenceImpl other) {
>>   153         if (referenceType() != other.referenceType()) return 
>> false;
>>   154         List<Field> fields = referenceType().fields();
>>   155         for (Field f : fields) {
>>   156             if (f.isStatic()) {
>>   157                 fields.remove(f);
>>   158             }
>>   159         }
>>   160         Map<Field,Value> thisFields = getValues(fields);
>>   161         Map<Field,Value> otherFields = other.getValues(fields);
>>   162         for (Field f : fields) {
>>   163             if (!thisFields.get(f).equals(otherFields.get(f))) 
>> return false;
>>   164         }
>>   165         return true;
>>   166     }
>>
>>
>> In this code, the type Field is not java.lang.reflect.Field but the
>> interface com.sun.jdi.Field.
>>
>> Regards,
>>
>> Fred
>>
>>
>>> On Oct 15, 2019, at 04:53, David Simms <david.simms at oracle.com> wrote:
>>>
>>>
>>> Look good, with two minor comments:
>>>
>>> * Spelling "isSubtituable()" => "isSubstitutable()"
>>> * An alternative is to use ValueBootstrapMethods.isSubstitutable()
>>>    rather than re-implement ?
>>>
>>> Cheers
>>>
>>> /Mr. Simms
>>>
>>>
>>>
>>> On 11/10/19 10:21 PM, Frederic Parain wrote:
>>>> Please review this change in JDI to implement the substitutability 
>>>> test
>>>> when jdb “eval” applies the ‘==‘ operator to inline types.
>>>>
>>>> CR:
>>>> https://bugs.openjdk.java.net/browse/JDK-8211027
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~fparain/jdi_substitutability/webrev.00/index.html 
>>>>
>>>>
>>>> Regards,
>>>>
>>>> Fred
>>>>
>



More information about the valhalla-dev mailing list