Hi Daniil,
we have some tests checking the number of jdwp packets and they've
detected a regression here:
com.jetbrains.jdi.ObjectReferenceImpl#validateAssignment now always
requests referenceType (requiring one more jdwp packet if value is not
yet cached),
previously it happened only for arrays.
Simple fix like this solves the issue:
===================================================================
--- src/main/java/com/jetbrains/jdi/ObjectReferenceImpl.java (revision
d96cab0ec2eeb791cceb7884341c56496cc026b9)
+++ src/main/java/com/jetbrains/jdi/ObjectReferenceImpl.java (revision
10a78a742d17338778e0226990ad0029a71cbf50)
@@ -647,11 +647,10 @@
*/
JNITypeParser destSig = new
JNITypeParser(destination.signature());
- JNITypeParser sourceSig = new JNITypeParser(type().signature());
if (destSig.isPrimitive()) {
throw new InvalidTypeException("Can't assign object value
to primitive");
}
- if (destSig.isArray() && !sourceSig.isArray()) {
+ if (destSig.isArray() && !new
JNITypeParser(type().signature()).isArray()) {
throw new InvalidTypeException("Can't assign non-array
value to an array");
}
if (destSig.isVoid()) {
If possible - please push the fix.
Thanks,
Egor
On 19-May-20 19:32, Daniil Titov wrote:
Hi Chris and Serguei,
Thank you for reviewing this change.
Best regards,
Daniil
On 5/18/20, 12:41 PM, "Chris Plummer" <chris.plum...@oracle.com> wrote:
Looks good.
thanks,
Chris
On 5/14/20 1:33 PM, Daniil Titov wrote:
> Hi Serguei and Chris,
>
> Please review a new version of the change [1] that addresses your
comments.
>
> Testing: Mach5 tier1-tier5 tests successfully passed.
>
> Regarding CR for the JDWP spec issues related to missing type
information in some of the protocol packet descriptions [3], as Chris has just
noticed
> we really don't need it, so I withdrew this CR.
>
> [1] http://cr.openjdk.java.net/~dtitov/8241080/webrev.02
> [2] https://bugs.openjdk.java.net/browse/JDK-8241080
> [3] https://bugs.openjdk.java.net/browse/JDK-8245057
>
> Thank you,
> Daniil
>
>
> From: "serguei.spit...@oracle.com" <serguei.spit...@oracle.com>
> Date: Monday, May 11, 2020 at 11:53 AM
> To: Daniil Titov <daniil.x.ti...@oracle.com>, serviceability-dev
<serviceability-dev@openjdk.java.net>
> Subject: Re: RFR: 8241080: Consolidate signature parsing code in
serviceability tools
>
> Hi Daniil,
>
> It looks pretty good in general.
> A couple of nits below.
>
>
http://cr.openjdk.java.net/~dtitov/8241080/webrev.01/src/jdk.jdwp.agent/share/native/libjdwp/invoker.c.udiff.html
> + void *cursor;
> + jbyte argumentTag;
> + jint argIndex = 0;
> + jvalue *argument = request->arguments;;
> . . .
> void *cursor;
> jint argIndex = 0;
> + jbyte argumentTag;
> jvalue *argument = request->arguments;
>
> It is better if the local variables 'cursor' and 'argumentTag' get
initialized.
> There is double semicolon: "arguments;;"
>
>
>
http://cr.openjdk.java.net/~dtitov/8241080/webrev.01/src/jdk.jdwp.agent/share/native/libjdwp/signature.h.html
> 43 static inline jbyte basicType(const char *signature) {
>
> It'd be nice to rename it to basicTypeTag() to get it unified with other
functions below.
>
> It is more safe to run tier5 as well.
>
> Thanks,
> Serguei
>
>
> On 5/9/20 09:29, Daniil Titov wrote:
> Please review a change[1] that centralizes the signature processing in
serviceability tools to make it capable of being easily extensible in the future.
>
> Testing: Mach5 tier1-tier3 tests successfully passed.
>
> [1] http://cr.openjdk.java.net/~dtitov/8241080/webrev.01
> [2] https://bugs.openjdk.java.net/browse/JDK-8241080
>
> Thank you,
> Daniil
>
>
>
>
>
>
--
Egor Ushakov
Software Developer
JetBrains
http://www.jetbrains.com
The Drive to Develop
