RFR[9]: 8133719 java.lang.InternalError in java.lang.invoke.MethodHandleImpl$BindCaller.bindCaller

[shilpi.rast...@oracle.com]

Hi All,

Please review fix for

https://bugs.openjdk.java.net/browse/JDK-8133719
http://cr.openjdk.java.net/~srastogi/8133719/webrev.01/


Thanks,
Shilpi

Re: RFR[9]: 8158510: Add test cases to validate Annotation

[shilpi.rast...@oracle.com]

Thanks Alan and Paul !!

Added the header http://cr.openjdk.java.net/~srastogi/8158510/webrev.04/

Regards,
Shilpi

On 7/5/2016 2:18 PM, Alan Bateman wrote:



On 05/07/2016 09:44, Paul Sandoz wrote:

On 5 Jul 2016, at 08:18, shilpi.rast...@oracle.com wrote:

Hi All,

Please review updated webrev 
http://cr.openjdk.java.net/~srastogi/8158510/webrev.03/



+1

Paul.
I think the copyright headers will need to be fixed up (to use the GPL 
headers) before this is pushed.


-Alan

Re: RFR[9]: 8158510: Add test cases to validate Annotation

[shilpi.rast...@oracle.com]

Hi All,

Please review updated webrev 
http://cr.openjdk.java.net/~srastogi/8158510/webrev.03/


Regards,
Shilpi

On 7/1/2016 5:11 PM, shilpi.rast...@oracle.com wrote:

Thanks Paul !!

Please see http://cr.openjdk.java.net/~srastogi/8158510/webrev.01/

Regards,
Shilpi

On 7/1/2016 3:12 PM, Paul Sandoz wrote:

Hi Shilpi,

There is more going on here than just the test since you have 
modified the annotation processing to throw an ISE for an annotation 
type that contains one or more methods that do not define elements. 
That behaviour might be too restrictive.


This is a grey area and implementation specific but i would tend to 
be cautious and avoid changing the current behaviour. Note that the 
original issue was due to javac adding a synthetic de-sugared method 
for a lambda. We don’t know if other byte code generators might do 
something different.


So my recommendation is to limit the testing to the current set of 
possible failures.


Paul.

On 1 Jul 2016, at 10:15, shilpi.rast...@oracle.com 
<mailto:shilpi.rast...@oracle.com> wrote:


Hi All,

Please review https://bugs.openjdk.java.net/browse/JDK-8158510
*Problem:* How to validate annotation, as javac does not allow us to 
write wrong annotation so how should we test valid annotation at 
runtime?


http://cr.openjdk.java.net/~srastogi/8158510/webrev.00/
*Solution:* To test this i used ASM tool and modified the classfile 
with wrong annotation (not allowed at javac level) and wrote test 
cases.


Thanks,
Shilpi

Re: RFR[9]: 8158510: Add test cases to validate Annotation

[shilpi.rast...@oracle.com]

Thanks Paul !!

Please see http://cr.openjdk.java.net/~srastogi/8158510/webrev.01/

Regards,
Shilpi

On 7/1/2016 3:12 PM, Paul Sandoz wrote:

Hi Shilpi,

There is more going on here than just the test since you have modified 
the annotation processing to throw an ISE for an annotation type that 
contains one or more methods that do not define elements. That 
behaviour might be too restrictive.


This is a grey area and implementation specific but i would tend to be 
cautious and avoid changing the current behaviour. Note that the 
original issue was due to javac adding a synthetic de-sugared method 
for a lambda. We don’t know if other byte code generators might do 
something different.


So my recommendation is to limit the testing to the current set of 
possible failures.


Paul.

On 1 Jul 2016, at 10:15, shilpi.rast...@oracle.com 
<mailto:shilpi.rast...@oracle.com> wrote:


Hi All,

Please review https://bugs.openjdk.java.net/browse/JDK-8158510
*Problem:* How to validate annotation, as javac does not allow us to 
write wrong annotation so how should we test valid annotation at runtime?


http://cr.openjdk.java.net/~srastogi/8158510/webrev.00/
*Solution:* To test this i used ASM tool and modified the classfile 
with wrong annotation (not allowed at javac level) and wrote test cases.


Thanks,
Shilpi

Re: RFR[9]: 8158169: MethodHandles.dropArgumentsToMatch(...)

[shilpi.rast...@oracle.com]

Thanks Michael and Paul for comments!!

Yes we already have negative test cases which throws IAE.
This test i have added to test the scenario where pos is equal to 
newTpes.size() and skip is equal to target.type.parameterList.size(). 
(positive)


Best Regards,
Shilpi
On 6/27/2016 6:42 PM, Paul Sandoz wrote:

Hi,



On 27 Jun 2016, at 14:22, shilpi.rast...@oracle.com wrote:

Hi All,

Please review fix for

https://bugs.openjdk.java.net/browse/JDK-8158169
http://cr.openjdk.java.net/~srastogi/8158169/webrev.00/


The test you added is not testing the constraints you added to the 
documentation? are those constraints already tested?

Paul.

RFR[9]: 8158169: MethodHandles.dropArgumentsToMatch(...)

[shilpi.rast...@oracle.com]

Hi All,

Please review fix for

https://bugs.openjdk.java.net/browse/JDK-8158169
http://cr.openjdk.java.net/~srastogi/8158169/webrev.00/

Thanks,
Shilpi

Fwd: RFR[9] 8039955: jdk/lambda/LambdaTranslationTest1 - java.lang.AssertionError:

[shilpi.rast...@oracle.com]

Gentle reminder!


 Forwarded Message 
Subject: 	RFR[9] 8039955: jdk/lambda/LambdaTranslationTest1 - 
java.lang.AssertionError:

Date:   Tue, 7 Jun 2016 14:58:47 +0530
From:   shilpi.rast...@oracle.com <shilpi.rast...@oracle.com>
To: core-libs-dev <core-libs-dev@openjdk.java.net>



Hi All,

Please review fix for
https://bugs.openjdk.java.net/browse/JDK-8039955
http://cr.openjdk.java.net/~srastogi/8039955/webrev.00/

Thanks,
Shilpi

Re: RFR[9]: 8147585: Annotations with lambda expressions has parameters result in wrong behavior.

[shilpi.rast...@oracle.com]

Thanks Joseph!!
Please see http://cr.openjdk.java.net/~srastogi/8147585/webrev.05/

Regards,
Shilpi

On 6/7/2016 6:00 AM, Joseph D. Darcy wrote:

Hello,

The test

  45 void testAnnotationWithLambda() {
  46 Method[] methods = 
AnnotationWithLambda.MethodsWithAnnotations.class.getDeclaredMethods();

  47 for (Method method : methods) {
  48 assertEquals(1, method.getDeclaredAnnotations().length);
  49 }
  50 }

is very non-specific in what it is testing. A better test would 
include something like


method.isAnnotationPresent(@LambdaWithParameter) &&
method.isAnnotationPresent(@LambdaWithouParameter)

if both annotations were put on the same method.

Cheers,

-Joe

On 6/6/2016 12:20 AM, shilpi.rast...@oracle.com wrote:

Gentle reminder!!

On 6/3/2016 11:40 AM, shilpi.rast...@oracle.com wrote:

Thanks Vladimir and Joel!!
Please see http://cr.openjdk.java.net/~srastogi/8147585/webrev.04/
Added restriction for synthetic methods as well.

On 6/2/2016 5:04 PM, Vladimir Ivanov wrote:



On 6/2/16 12:02 AM, Joel Borggrén-Franck wrote:

Hi,

I think this was caught by the verifier before 8 since you 
couldn't have
concrete or private methods in an interface. Now you can (though 
not in

Java for private ones).

My spider sense tells me there might be something lurking here 
(though
it was a while since this was in my L1 cache). It is not likely, 
but I'm
not 100% sure that it is impossible to make javac produce a bridge 
when

compiling an annotation type for example, so why not remove synthetic
methods as well?


I'm not an expert in annotation processing, but it bothers me as 
well, since current behavior looks too Java-centric. There are 
valid (in JVMS sense) class files which are not produced by javac 
or from java source file.


What is expected behavior in such case? It is unfortunate when 
non-Java languages have to obey to Java rules. It reminds me a 
problem with Class.getSimpleName() (see JDK-8057919 [1]) we fixed 
some time ago.

Best regards,
Vladimir Ivanov

[1] https://bugs.openjdk.java.net/browse/JDK-8057919



Spending some time with ASM to do a bunch of tests not compilable in
java might be useful, there should also be some frameworks in 
langtools

to produce invalid classfiles IIRC.
  Raised https://bugs.openjdk.java.net/browse/JDK-8158510 to include 
new test cases.


Regards,
Shilpi


cheers
/Joel

On Tue, 31 May 2016 at 17:49 shilpi.rast...@oracle.com
<mailto:shilpi.rast...@oracle.com> <shilpi.rast...@oracle.com
<mailto:shilpi.rast...@oracle.com>> wrote:

Thanks Paul!!
Please see 
http://cr.openjdk.java.net/~srastogi/8147585/webrev.03/


Thanks,
Shilpi

On 5/31/2016 7:57 PM, Paul Sandoz wrote:
> >"Returns an array containing Method objects reflecting all the
declared methods of the class or interface represented by this 
Class
object, including public, protected, default (package) access, 
and

private methods, but excluding inherited methods."

RFR[9] 8039955: jdk/lambda/LambdaTranslationTest1 - java.lang.AssertionError:

[shilpi.rast...@oracle.com]

Hi All,

Please review fix for
https://bugs.openjdk.java.net/browse/JDK-8039955
http://cr.openjdk.java.net/~srastogi/8039955/webrev.00/

Thanks,
Shilpi

Re: RFR[9]: 8147585: Annotations with lambda expressions has parameters result in wrong behavior.

[shilpi.rast...@oracle.com]

Gentle reminder!!

On 6/3/2016 11:40 AM, shilpi.rast...@oracle.com wrote:

Thanks Vladimir and Joel!!
Please see http://cr.openjdk.java.net/~srastogi/8147585/webrev.04/
Added restriction for synthetic methods as well.

On 6/2/2016 5:04 PM, Vladimir Ivanov wrote:



On 6/2/16 12:02 AM, Joel Borggrén-Franck wrote:

Hi,

I think this was caught by the verifier before 8 since you couldn't 
have

concrete or private methods in an interface. Now you can (though not in
Java for private ones).

My spider sense tells me there might be something lurking here (though
it was a while since this was in my L1 cache). It is not likely, but 
I'm

not 100% sure that it is impossible to make javac produce a bridge when
compiling an annotation type for example, so why not remove synthetic
methods as well?


I'm not an expert in annotation processing, but it bothers me as 
well, since current behavior looks too Java-centric. There are valid 
(in JVMS sense) class files which are not produced by javac or from 
java source file.


What is expected behavior in such case? It is unfortunate when 
non-Java languages have to obey to Java rules. It reminds me a 
problem with Class.getSimpleName() (see JDK-8057919 [1]) we fixed 
some time ago.

Best regards,
Vladimir Ivanov

[1] https://bugs.openjdk.java.net/browse/JDK-8057919



Spending some time with ASM to do a bunch of tests not compilable in
java might be useful, there should also be some frameworks in langtools
to produce invalid classfiles IIRC.
  Raised https://bugs.openjdk.java.net/browse/JDK-8158510 to include 
new test cases.


Regards,
Shilpi


cheers
/Joel

On Tue, 31 May 2016 at 17:49 shilpi.rast...@oracle.com
<mailto:shilpi.rast...@oracle.com> <shilpi.rast...@oracle.com
<mailto:shilpi.rast...@oracle.com>> wrote:

Thanks Paul!!
Please see http://cr.openjdk.java.net/~srastogi/8147585/webrev.03/

Thanks,
Shilpi

On 5/31/2016 7:57 PM, Paul Sandoz wrote:
> >"Returns an array containing Method objects reflecting all the
declared methods of the class or interface represented by this 
Class

object, including public, protected, default (package) access, and
private methods, but excluding inherited methods."

Re: RFR[9]: 8147585: Annotations with lambda expressions has parameters result in wrong behavior.

[shilpi.rast...@oracle.com]

Thanks Vladimir and Joel!!
Please see http://cr.openjdk.java.net/~srastogi/8147585/webrev.04/
Added restriction for synthetic methods as well.

On 6/2/2016 5:04 PM, Vladimir Ivanov wrote:



On 6/2/16 12:02 AM, Joel Borggrén-Franck wrote:

Hi,

I think this was caught by the verifier before 8 since you couldn't have
concrete or private methods in an interface. Now you can (though not in
Java for private ones).

My spider sense tells me there might be something lurking here (though
it was a while since this was in my L1 cache). It is not likely, but I'm
not 100% sure that it is impossible to make javac produce a bridge when
compiling an annotation type for example, so why not remove synthetic
methods as well?


I'm not an expert in annotation processing, but it bothers me as well, 
since current behavior looks too Java-centric. There are valid (in 
JVMS sense) class files which are not produced by javac or from java 
source file.


What is expected behavior in such case? It is unfortunate when 
non-Java languages have to obey to Java rules. It reminds me a problem 
with Class.getSimpleName() (see JDK-8057919 [1]) we fixed some time ago.

Best regards,
Vladimir Ivanov

[1] https://bugs.openjdk.java.net/browse/JDK-8057919



Spending some time with ASM to do a bunch of tests not compilable in
java might be useful, there should also be some frameworks in langtools
to produce invalid classfiles IIRC.
  Raised https://bugs.openjdk.java.net/browse/JDK-8158510 to include 
new test cases.


Regards,
Shilpi


cheers
/Joel

On Tue, 31 May 2016 at 17:49 shilpi.rast...@oracle.com
<mailto:shilpi.rast...@oracle.com> <shilpi.rast...@oracle.com
<mailto:shilpi.rast...@oracle.com>> wrote:

Thanks Paul!!
Please see http://cr.openjdk.java.net/~srastogi/8147585/webrev.03/

Thanks,
Shilpi

On 5/31/2016 7:57 PM, Paul Sandoz wrote:
> >"Returns an array containing Method objects reflecting all the
declared methods of the class or interface represented by this Class
object, including public, protected, default (package) access, and
private methods, but excluding inherited methods."

RFR[9]:MethodHandles.dropArgumentsToMatch(...) non-documented IAE

[shilpi.rast...@oracle.com]

Hi All,

Please review fix for
https://bugs.openjdk.java.net/browse/JDK-8158171
http://cr.openjdk.java.net/~srastogi/8158171/webrev.00/


Thanks,
Shilpi

Re: RFR[9]: 8147585: Annotations with lambda expressions has parameters result in wrong behavior.

[shilpi.rast...@oracle.com]

Thanks Paul!!
Please see http://cr.openjdk.java.net/~srastogi/8147585/webrev.03/

Thanks,
Shilpi

On 5/31/2016 7:57 PM, Paul Sandoz wrote:

>"Returns an array containing Method objects reflecting all the declared methods of 
the class or interface represented by this Class object, including public, protected, 
default (package) access, and private methods, but excluding inherited methods."

Re: RFR[9]: 8147585: Annotations with lambda expressions has parameters result in wrong behavior.

[shilpi.rast...@oracle.com]

Hi All,

Please see updated webrev 
http://cr.openjdk.java.net/~srastogi/8147585/webrev.02/


On 5/31/2016 2:21 PM, Paul Sandoz wrote:

On 31 May 2016, at 10:35, shilpi.rast...@oracle.com wrote:

Thanks Paul for comments.

Please see http://cr.openjdk.java.net/~srastogi/8147585/webrev.01/

Now processing only public abstract methods of interface.


Thanks. It would be good to get some got feedback from those wiser than I in 
this regard.

Have you looked at the existing annotation-based tests to see if they test edge 
cases e.g. annotation classes generated with incorrect methods? that might give 
us some clues.
  I saw existing annotation-based test, valid modifier for annotations, 
valid method for annotation tests we are checking in javac code.
  default, static, private modifier we are restricting at compile time 
so we can not add test cases for this  (compilation will fail).


So only scenario i can add is for synthetic methods. ( according to my 
assumption)

In AnnotationType.java

public Method[] run() {
 // Initialize memberTypes and defaultValues
 return annotationClass.getDeclaredMethods();
  }
});

As here, calling getDeclaredMethods() on annotationclass and 
getDeclaredMethods() doc says


https://docs.oracle.com/javase/8/docs/api/java/lang/Class.html#getDeclaredMethods--
"Returns an array containing Method objects reflecting all the declared 
methods of the class or interface represented by this Class object, 
including public, protected, default (package) access, and private 
methods, but excluding inherited methods."


Doc did not mention anything about synthetic methods so i am not sure 
this is expected behavior or not.
If yes, Could you please suggest how to add testcases to test synthetic 
method?

Shall I use ASM?

Regards,
Shilpi



Testing wise:

- you can avoid the filtering of the test method by placing the annotations in 
another auxiliary class (my preference is to nest rather than using auxiliary 
classes, up to you)

- there is a couple of minor style inconsistencies e.g. a space here "@ interface 
LambdaWithoutParameter"

- 30  * @run testng/othervm -ea -esa AnnotationWithLambda

You can just do:

   @run testng AnnotationWithLambda

?

Thanks,
Paul.





Thanks,
Shilpi

On 5/30/2016 6:35 PM, Paul Sandoz wrote:

Hi Shilpi,

You have found the right place but i am not sure your fix is entirely correct.

(Tip: if you use  -Xlog:exceptions=info you can observe the IAE exception when 
the annotation is processed)

In your test you have:

@Target(value = ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
@ interface LambdaWithParameter {
 Consumer f1 = a -> {
 System.out.println("lambda has parameter");
 };
}

@Target(value = ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
@ interface LambdaWithoutParameter {
 Runnable r = () -> System.out.println("lambda without parameter");
}


Both of those annotations will have static synthetic methods generated by the 
compiler that the indy resolves and links to (look at the javap output). The 
former will have a method with one parameter.


The code in sun/reflect/annotation/AnnotationType.java:

for (Method method : methods) {
 if (method.getParameterTypes().length != 0)
 throw new IllegalArgumentException(method + " has params");

has thrown the IAE since 2004, but it’s not clear why it was added as opposed 
to something more general (see below).


The correct fix appears to be to skip over any non-abstract/non-public methods. 
Thus only public abstract methods get processed.

Your current fix now processes synthetic methods with parameters, in addition 
to those which were already processed such as synthetic methods without 
parameters, or even private methods that could have been generated by some 
tool. I dunno how much say the verifier has in all this, perhaps little or no 
say.

Thus non-public/non-abstract methods could add inconsistent information to the 
data structures of AnnotationType. Perhaps this is mostly harmless?

Perhaps Joel (CC’ed) can she some more light on this?

Paul.



On 30 May 2016, at 08:00, shilpi.rast...@oracle.com wrote:

Hi All,

Please review fix for https://bugs.openjdk.java.net/browse/JDK-8147585
http://cr.openjdk.java.net/~srastogi/8147585/webrev.00/

Problem: Annotation with Lamda has parameters results into wrong behavior ( not 
considered as valid annotation) because
According to JLS "By virtue of the AnnotationTypeElementDeclaration production, a 
method declaration in an annotation type declaration cannot have formal parameters, type 
parameters, or a throws clause. The following production from §4.3 is shown here for 
convenience:"
(Ref: https://docs.oracle.com/javase/specs/jls/se8/html/jls-9.html#jls-9.6.1)


Solution: We should skip synthetic methods during Annotation parsing. According to JLS 
"An annotation type has no elements other than those defined by the methods 

Re: RFR[9]: 8147585: Annotations with lambda expressions has parameters result in wrong behavior.

[shilpi.rast...@oracle.com]

Thanks Paul for comments.

Please see http://cr.openjdk.java.net/~srastogi/8147585/webrev.01/

Now processing only public abstract methods of interface.

Thanks,
Shilpi

On 5/30/2016 6:35 PM, Paul Sandoz wrote:

Hi Shilpi,

You have found the right place but i am not sure your fix is entirely correct.

(Tip: if you use  -Xlog:exceptions=info you can observe the IAE exception when 
the annotation is processed)

In your test you have:

@Target(value = ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
@ interface LambdaWithParameter {
 Consumer f1 = a -> {
 System.out.println("lambda has parameter");
 };
}

@Target(value = ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
@ interface LambdaWithoutParameter {
 Runnable r = () -> System.out.println("lambda without parameter");
}


Both of those annotations will have static synthetic methods generated by the 
compiler that the indy resolves and links to (look at the javap output). The 
former will have a method with one parameter.


The code in sun/reflect/annotation/AnnotationType.java:

for (Method method : methods) {
 if (method.getParameterTypes().length != 0)
 throw new IllegalArgumentException(method + " has params");

has thrown the IAE since 2004, but it’s not clear why it was added as opposed 
to something more general (see below).


The correct fix appears to be to skip over any non-abstract/non-public methods. 
Thus only public abstract methods get processed.

Your current fix now processes synthetic methods with parameters, in addition 
to those which were already processed such as synthetic methods without 
parameters, or even private methods that could have been generated by some 
tool. I dunno how much say the verifier has in all this, perhaps little or no 
say.

Thus non-public/non-abstract methods could add inconsistent information to the 
data structures of AnnotationType. Perhaps this is mostly harmless?

Perhaps Joel (CC’ed) can she some more light on this?

Paul.



On 30 May 2016, at 08:00, shilpi.rast...@oracle.com wrote:

Hi All,

Please review fix for https://bugs.openjdk.java.net/browse/JDK-8147585
http://cr.openjdk.java.net/~srastogi/8147585/webrev.00/

Problem: Annotation with Lamda has parameters results into wrong behavior ( not 
considered as valid annotation) because
According to JLS "By virtue of the AnnotationTypeElementDeclaration production, a 
method declaration in an annotation type declaration cannot have formal parameters, type 
parameters, or a throws clause. The following production from §4.3 is shown here for 
convenience:"
(Ref: https://docs.oracle.com/javase/specs/jls/se8/html/jls-9.html#jls-9.6.1)


Solution: We should skip synthetic methods during Annotation parsing. According to JLS 
"An annotation type has no elements other than those defined by the methods it 
explicitly declares."
(Ref https://docs.oracle.com/javase/specs/jls/se8/html/.html#jls-9jls-9.6.1)


Thanks,
Shilpi

Re: RFR[9]: 8156868 MethodHandles.zero(Class) doc issues

[shilpi.rast...@oracle.com]

Thanks Paul!!

Please see http://cr.openjdk.java.net/~srastogi/8156868/webrev.02/

On 5/23/2016 9:08 PM, Paul Sandoz wrote:

On 23 May 2016, at 08:27, shilpi.rast...@oracle.com wrote:

Hi All,

Please review small doc changes-

http://cr.openjdk.java.net/~srastogi/8156868/webrev.01/

Fixed following doc issues in same patch

https://bugs.openjdk.java.net/browse/JDK-8138599
http://cr.openjdk.java.net/~srastogi/8156868/webrev.01/src/java.base/share/classes/java/lang/invoke/MemberName.java.sdiff.html


I think that’s an correct educated guess at completing the trailing sentence.



https://bugs.openjdk.java.net/browse/JDK-8138597
http://cr.openjdk.java.net/~srastogi/8156868/webrev.01/src/java.base/share/classes/java/lang/invoke/MethodHandle.java.sdiff.html

https://bugs.openjdk.java.net/browse/JDK-8156868
http://cr.openjdk.java.net/~srastogi/8156868/webrev.01/src/java.base/share/classes/java/lang/invoke/MethodHandles.java.sdiff.html


+1



https://bugs.openjdk.java.net/browse/JDK-8075283
http://cr.openjdk.java.net/~srastogi/8156868/webrev.01/src/java.base/share/classes/jdk/internal/misc/Unsafe.java.sdiff.html


Can you also update the same method on sun.misc.Unsafe in the jdk.unsupported 
module?

http://cr.openjdk.java.net/~srastogi/8156868/webrev.02/src/jdk.unsupported/share/classes/sun/misc/Unsafe.java.sdiff.html

  Regards,
 Shilpi


Thanks,
Paul.

RFR[9]: 8156868 MethodHandles.zero(Class) doc issues

[shilpi.rast...@oracle.com]

Hi All,

Please review small doc changes-

http://cr.openjdk.java.net/~srastogi/8156868/webrev.01/

Fixed following doc issues in same patch

https://bugs.openjdk.java.net/browse/JDK-8138599
http://cr.openjdk.java.net/~srastogi/8156868/webrev.01/src/java.base/share/classes/java/lang/invoke/MemberName.java.sdiff.html

https://bugs.openjdk.java.net/browse/JDK-8138597
http://cr.openjdk.java.net/~srastogi/8156868/webrev.01/src/java.base/share/classes/java/lang/invoke/MethodHandle.java.sdiff.html

https://bugs.openjdk.java.net/browse/JDK-8156868
http://cr.openjdk.java.net/~srastogi/8156868/webrev.01/src/java.base/share/classes/java/lang/invoke/MethodHandles.java.sdiff.html

https://bugs.openjdk.java.net/browse/JDK-8075283
http://cr.openjdk.java.net/~srastogi/8156868/webrev.01/src/java.base/share/classes/jdk/internal/misc/Unsafe.java.sdiff.html


Thanks,
Shilpi

Re: RFR[9]:java/lang/invoke/VarargsArrayTest.java miss othervm for main/bootclasspath mode

[shilpi.rast...@oracle.com]

On 5/17/2016 10:28 PM, Vladimir Ivanov wrote:

  Could someone please explain why it is not required for this test as i
saw we are updating other tests (
http://hg.openjdk.java.net/jdk9/hs/hotspot/rev/0e2a2be60453 ) ?

The test was fixed in Jigsaw repo [1] in a different way.

Before the fix, the test class should be put on boot class path (hence 
main/bootclasspath), since MethodHandles.Lookup.IMPL_LOOKUP was used.


After the fix, MethodHandleHelper is part of java.base module and the 
test fetches the lookup from it:


 * @compile/module=java.base java/lang/invoke/MethodHandleHelper.java
 * @run main/othervm -esa CustomizedLambdaFormTest
...
MethodHandle mh = 
MethodHandleHelper.IMPL_LOOKUP.findVirtual(String.class, "concat",


So, main/othervm is enough and there's no need in 
main/bootclasspath/othervm anymore.


Hope it helps.

 Thank You Vladimir!


Best regards,
Vladimir Ivanov

[1] 
http://hg.openjdk.java.net/jdk9/dev/jdk/diff/b2a69d66dc65/test/java/lang/invoke/CustomizedLambdaFormTest.java

Re: RFR[9]:java/lang/invoke/VarargsArrayTest.java miss othervm for main/bootclasspath mode

[shilpi.rast...@oracle.com]

On 5/17/2016 7:44 PM, Alan Bateman wrote:


On 17/05/2016 14:34, Martin Buchholz wrote:

In jdk8 the /othervm is missing.
http://mail.openjdk.java.net/pipermail/jtreg-use/2016-May/000478.html


Ah, I assumed we were discussing the version in jdk9/dev.

BTW: My interest here is to make sure we didn't break anything with 
the module system refresh two weeks ago. We are currently joined at 
the hip to jtreg and so had to force an update to jtreg as part of the 
update. We updated many tests as part of this and I don't recall any 
tests failing when we pushed.


  Could someone please explain why it is not required for this test as 
i saw we are updating other tests ( 
http://hg.openjdk.java.net/jdk9/hs/hotspot/rev/0e2a2be60453 ) ?


  Regards,
  Shilpi


-Alan

Re: RFR[9]:java/lang/invoke/VarargsArrayTest.java miss othervm for main/bootclasspath mode

[shilpi.rast...@oracle.com]

On 5/17/2016 6:00 PM, Aleksey Shipilev wrote:

On 05/17/2016 03:21 PM, shilpi.rast...@oracle.com wrote:

Gentle Reminder!

https://bugs.openjdk.java.net/browse/JDK-8155791
http://cr.openjdk.java.net/~srastogi/8155791/webrev.00/

Parse Exception: [-esa]: vm option(s) found, need to specify /othervm

Still not getting this. The message tells to specify /othervm.

But the original code in CustomizedLambdaFormTest already has
"/othervm", and the fix adds "/bootclasspath". This contradicts both the
comment and the synopsis for the issue.
  jtreg 4.2b02 requires main/bootclasspath mode to specify othervm if 
any command-line flags are used.

  So  just added

* @run main/bootclasspath/othervm -esa CustomizedLambdaFormTest

Regards,
Shilpi

Thanks,
-Aleksey

Re: RFR[9]:java/lang/invoke/VarargsArrayTest.java miss othervm for main/bootclasspath mode

[shilpi.rast...@oracle.com]

Gentle Reminder!

https://bugs.openjdk.java.net/browse/JDK-8155791
http://cr.openjdk.java.net/~srastogi/8155791/webrev.00/



On 5/12/2016 3:06 PM, Michael Haupt wrote:

Hi Shilpi,

thanks for the clarification. OK by me then - please note this is a lower-case 
review.

Best,

Michael


Am 11.05.2016 um 15:55 schrieb shilpi.rast...@oracle.com:

Thank You Michael for comments!

It was fixed for VarargsArrayTest.java  but not fixed for 
java/lang/invoke/CustomizedLambdaFormTest.java.


Boris Molodenkov 
<https://bugs.openjdk.java.net/secure/ViewProfile.jspa?name=bmoloden>added a 
comment -2016-05-02 02:37-Restricted toConfidential
One more test
RULE "java/lang/invoke/CustomizedLambdaFormTest.java" StatusError Parse 
Exception: [-esa]: vm option(s) found, need to specify /othervm

Thanks,
Shilpi

On 5/11/2016 6:47 PM, Michael Haupt wrote:

Hi Shilpi,

not a review - the bug mentions that this issue is fixed in Jake already and 
that the bug should be closed once the fix from Jake propagates to dev. What is 
the status of that?

Best,

Michael


Am 11.05.2016 um 13:24 schrieb shilpi.rast...@oracle.com:

Hi All,

Please review one small fix for

https://bugs.openjdk.java.net/browse/JDK-8155791
http://cr.openjdk.java.net/~srastogi/8155791/webrev.00/

Thanks,
Shilpi

Re: RFR[9]:Fix java/lang/invoke/MethodHandleImpl's use of Unsafe.defineAnonymousClass()

[shilpi.rast...@oracle.com]

Thanks Vladimir and Remi.

Please review updated webrev 
http://cr.openjdk.java.net/~srastogi/8149574/webrev/


Regards,
Shilpi

On 5/13/2016 7:31 PM, Vladimir Ivanov wrote:

Good point, Remi. I agree that it's redundant.

Best regards,
Vladimir Ivanov

On 5/13/16 4:51 PM, Remi Forax wrote:

Also instead of
  MethodVisitor mv = cw.visitMethod(ACC_PRIVATE | ACC_STATIC, 
"invoke_V",

"(Ljava/lang/invoke/MethodHandle;[Ljava/lang/Object;)Ljava/lang/Object;",
  null, new String[] { "java/lang/Throwable" });

because the code is never read by a java compiler (as javac), you 
don't need to specify the exception

(checked exceptions are a Java the language artifact)
  MethodVisitor mv = cw.visitMethod(ACC_PRIVATE | ACC_STATIC, 
"invoke_V",

"(Ljava/lang/invoke/MethodHandle;[Ljava/lang/Object;)Ljava/lang/Object;",
  null, null);

my 2 cents,
Rémi

- Mail original -

De: "Vladimir Ivanov" <vladimir.x.iva...@oracle.com>
À: "shilpi rastogi" <shilpi.rast...@oracle.com>
Cc: core-libs-dev@openjdk.java.net
Envoyé: Vendredi 13 Mai 2016 15:41:33
Objet: Re: RFR[9]:Fix java/lang/invoke/MethodHandleImpl's use of
Unsafe.defineAnonymousClass()


MethodHandle vamh = prepareForInvoker(MH_checkCallerClass);
 Object ok = bccInvoker.invokeExact(vamh, new 
Object[]{hostClass, bcc});

+   assert Boolean.TRUE.equals(ok) : ok;

What I meant is to convert the whole test (inside try-catch block) into
an assert.


+UNSAFE.ensureClassInitialized(bcc);

Do people see any reason to force invoker class init? I'd prefer to see
it goes away.

Also, as a second thought, generateInvokerTemplate() looks clearer than
invokerTemplateGenerator().

Something like the following:
   http://cr.openjdk.java.net/~vlivanov/8149574/webrev.00/

Additional cleanup: checkCallerClass() should return injected invoker
class when invoked using a method handle, so no need in 2nd argument.

Best regards,
Vladimir Ivanov

On 5/13/16 11:56 AM, shilpi.rast...@oracle.com wrote:

Thanks Paul!

Please review http://cr.openjdk.java.net/~srastogi/8149574/webrev10.0/

Regards,
Shilpi

On 5/13/2016 2:09 PM, Paul Sandoz wrote:

assert Boolean.TRUE.equals(ok) : ok;

Re: RFR[9]:Fix java/lang/invoke/MethodHandleImpl's use of Unsafe.defineAnonymousClass()

[shilpi.rast...@oracle.com]

Thanks Paul!

Please review http://cr.openjdk.java.net/~srastogi/8149574/webrev10.0/

Regards,
Shilpi

On 5/13/2016 2:09 PM, Paul Sandoz wrote:

assert Boolean.TRUE.equals(ok) : ok;

Re: RFR[9]:Fix java/lang/invoke/MethodHandleImpl's use of Unsafe.defineAnonymousClass()

[shilpi.rast...@oracle.com]

Thank you Vladimir for comments.

Please review updated webrev 
http://cr.openjdk.java.net/~srastogi/8149574/webrev.09/


Regards,
Shilpi

On 5/12/2016 6:12 PM, Vladimir Ivanov wrote:

rankly speaking, I'd prefer [2] to be co

Re: RFR[9]:Fix java/lang/invoke/MethodHandleImpl's use of Unsafe.defineAnonymousClass()

[shilpi.rast...@oracle.com]

Thank You Paul for comments.
Please review updated webrev 
http://cr.openjdk.java.net/~srastogi/8149574/webrev.08/


Regards,
Shilpi

On 5/12/2016 3:41 PM, Paul Sandoz wrote:

On 11 May 2016, at 18:31, shilpi.rast...@oracle.com wrote:

Hi All,

Please review the updated webrev- 
http://cr.openjdk.java.net/~srastogi/8149574/webrev.07/



1219 FieldVisitor fv;
1220 MethodVisitor mv;
1221 AnnotationVisitor av0;

Field “fv is not used. Since “av0” is only used once, might as well declare it 
at line #1246.

Can you break up the long lines at #1242 & #1252 ?


My inclination is to turn the anon static block into a method returning byte[] 
and then do:

/*

*/
private static final byte[] T_BYTES = generateT();
private static byte[] generateT() {
…
}


One concern, more so because of my ignorance, is why can the default package be 
used. Does anyone know?

Paul.


Changed the anonymous class package with no package name.

Regards,
Shilpi

Re: RFR[9]:Fix java/lang/invoke/MethodHandleImpl's use of Unsafe.defineAnonymousClass()

[shilpi.rast...@oracle.com]

Hi All,

Please review the updated webrev- 
http://cr.openjdk.java.net/~srastogi/8149574/webrev.07/


Changed the anonymous class package with no package name.

Regards,
Shilpi

On 5/11/2016 8:22 PM, Paul Sandoz wrote:

Hi Shilpi,

What makes me slightly queasy about the constant pool patching of T is it’s 
quite fragile and it produces a class with a slightly inconsistent state 
regarding the InnerClasses structures. It’s probably mostly harmless but still 
bothers me.

One solution is to combine ASM with constant pool patching. Use ASM to generate 
the class bytes, and query the constant pool in the ClassWriter, then remember 
the class bytes, the pool size, and index to patch (a cursory glance at 
ClassWriter suggests this is possible).

But i think Michael raises an important point about simplification using the 
default package, if that is possible. However, if the default package is to be 
used ASM is still required to generate class bytes and define the class 
anonymously within the scope of the host class. Thus in this approach the 
addition of patching should not add much more complexity and i think would be 
more in sync with that of the host class.

Paul.


On 11 May 2016, at 13:24, shilpi.rast...@oracle.com wrote:

Hi All,

Please review the following-

https://bugs.openjdk.java.net/browse/JDK-8149574

Solution: Changed anonymous class package name with the package name of its 
host class.

Two approaches to solve this-
1.  Parse .class and get the class name index form constant pool and patch it 
with new name
http://cr.openjdk.java.net/~srastogi/8149574/webrev.05/

2. Create class with new name (With ASM)
http://cr.openjdk.java.net/~srastogi/8149574/webrev.06/

Which approach is better?

Thanks,
Shilpi

Re: RFR[9]:java/lang/invoke/VarargsArrayTest.java miss othervm for main/bootclasspath mode

[shilpi.rast...@oracle.com]

Thank You Michael for comments!

It was fixed for VarargsArrayTest.java  but not fixed for 
java/lang/invoke/CustomizedLambdaFormTest.java.



Boris Molodenkov 
<https://bugs.openjdk.java.net/secure/ViewProfile.jspa?name=bmoloden>added 
a comment -2016-05-02 02:37-Restricted toConfidential

One more test
RULE "java/lang/invoke/CustomizedLambdaFormTest.java" StatusError Parse 
Exception: [-esa]: vm option(s) found, need to specify /othervm


Thanks,
Shilpi

On 5/11/2016 6:47 PM, Michael Haupt wrote:

Hi Shilpi,

not a review - the bug mentions that this issue is fixed in Jake already and 
that the bug should be closed once the fix from Jake propagates to dev. What is 
the status of that?

Best,

Michael


Am 11.05.2016 um 13:24 schrieb shilpi.rast...@oracle.com:

Hi All,

Please review one small fix for

https://bugs.openjdk.java.net/browse/JDK-8155791
http://cr.openjdk.java.net/~srastogi/8155791/webrev.00/

Thanks,
Shilpi

RFR[9]:Fix java/lang/invoke/MethodHandleImpl's use of Unsafe.defineAnonymousClass()

[shilpi.rast...@oracle.com]

Hi All,

Please review the following-

https://bugs.openjdk.java.net/browse/JDK-8149574

Solution: Changed anonymous class package name with the package name of 
its host class.


Two approaches to solve this-
1.  Parse .class and get the class name index form constant pool and 
patch it with new name

http://cr.openjdk.java.net/~srastogi/8149574/webrev.05/

2. Create class with new name (With ASM)
http://cr.openjdk.java.net/~srastogi/8149574/webrev.06/

Which approach is better?

Thanks,
Shilpi

RFR[9]:java/lang/invoke/VarargsArrayTest.java miss othervm for main/bootclasspath mode

[shilpi.rast...@oracle.com]

Hi All,

Please review one small fix for

https://bugs.openjdk.java.net/browse/JDK-8155791
http://cr.openjdk.java.net/~srastogi/8155791/webrev.00/

Thanks,
Shilpi

Fwd: RFR 8150829: Enhanced drop-args, identity and default constant, varargs adjustment

[shilpi.rast...@oracle.com]

Gentle Reminder!

 Forwarded Message 
Subject: 	RFR 8150829: Enhanced drop-args, identity and default 
constant, varargs adjustment

Date:   Tue, 22 Mar 2016 17:11:00 +0530
From:   shilpi.rast...@oracle.com <shilpi.rast...@oracle.com>
To: 	core-libs-dev@openjdk.java.net, John Rose <john.r.r...@oracle.com>, 
Vladimir Ivanov <vladimir.x.iva...@oracle.com>, paul.san...@oracle.com




Hi All,

Please review the following-

https://bugs.openjdk.java.net/browse/JDK-8150829
http://cr.openjdk.java.net/~srastogi/8150829/webrev.04


Thanks,
Shilpi

RFR 8150829: Enhanced drop-args, identity and default constant, varargs adjustment

[shilpi.rast...@oracle.com]

Hi All,

Please review the following-

https://bugs.openjdk.java.net/browse/JDK-8150829
http://cr.openjdk.java.net/~srastogi/8150829/webrev.04


Thanks,
Shilpi

Re: RFR 8144931: Assert class signatures are correct and refer to valid classes

[shilpi.rast...@oracle.com]

Hi All,

Please see the updated webrev
http://cr.openjdk.java.net/~srastogi/8144931/webrev.03/

Regards,
Shilpi

On 2/18/2016 8:33 PM, Paul Sandoz wrote:

On 18 Feb 2016, at 15:55, Vladimir Ivanov  wrote:


P.S.: You could improve the assertion with the Type class, so it would also fail on otherwise 
broken descriptor strings (missing "L" or missing ";"):

I like how it shapes out with Type. Thanks, Uwe!


Me too!

Paul.


Best regards,
Vladimir Ivanov


 static boolean checkClassName(String cn) {
Type tp = Type.getType(cn);
// additional sanity so only valid "L;" descriptors work
if (tp.getSort() != Type.OBJECT) {
  return false;
}
try {
Class c = Class.forName(tp.getClassName(), false, null);
return true;
} catch (ClassNotFoundException e) {
return false;
}
}

-
Uwe Schindler
uschind...@apache.org
ASF Member, Apache Lucene PMC / Committer
Bremen, Germany
http://lucene.apache.org/

Re: RFR 8144931: Assert class signatures are correct and refer to valid classes

[shilpi.rast...@oracle.com]

Thank You Vladimir!

I have done the changes. Please review the updated patch-

http://cr.openjdk.java.net/~srastogi/8144931/webrev.02/

Regards,
Shilpi

On 2/18/2016 1:58 PM, Vladimir Ivanov wrote:

Shilpi,

_CLASS suffix looks redundant and you can abbreviate LAMBDA_FORM to LF:
  LF_HIDDEN_SIG
  LF_COMPILED_SIG
  FORCEINLINE_SIG
  DONTINLINE_SIG

Otherwise, looks fine.

Best regards,
Vladimir Ivanov

On 2/17/16 5:47 PM, shilpi rastogi wrote:

Hi All,

Please review fix for the following bug-

https://bugs.openjdk.java.net/browse/JDK-8144931
http://cr.openjdk.java.net/~srastogi/8144931/webrev.01/


Thanks,
Shilpi