> On Nov 20, 2015, at 1:59 AM, serguei.spit...@oracle.com wrote: > > The 'int bci' is not used above.
This is weird. Daniel caught that and I took that line out already. http://cr.openjdk.java.net/~mchung/jdk9/jep259/webrev.03-delta/hotspot/src/share/vm/classfile/javaClasses.cpp.sdiff.html Anyway this webrev is the up-to-date one including fixing the nits you pointed out. http://cr.openjdk.java.net/~mchung/jdk9/jep259/webrev.04 > > Minor: The calls to set_version() and set_bci() are the same for both > alternatives > and can be done just once before the if-else statement as below. > With such refactoring it is clear what the common part is. > Moved. > > webrev.03/hotspot/src/share/vm/prims/jvm.cpp > > Minor: Need spaces around the '+'. > webrev.03/hotspot/src/share/vm/prims/stackwalk.cpp I am not sure if that’s the convention applied consistently in VM. I fixed it anyway. > > Minor: Indent at the line 115 must be +2. I don’t see any indentation/formatting issue there. > 360 for (int n=0; n < skip_frames && !vfst.at_end(); vfst.next(), n++) { > I prefer to keep n=0 and there are other places using that convention. > 451 int count = frame_count+start_index; > Minor: Need spaces around the '=' and '+’. Fixed. Thanks Mandy