On 11/20/15 08:39, Mandy Chung wrote:
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.
Thanks.
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.
webrev.04/hotspot/src/share/vm/prims/jvm.cpp
One place left with inconsistent formatting:
597 int limit = start_index+frame_count;
Minor: Indent at the line 115 must be +2.
I don’t see any indentation/formatting issue there.
I see it fixed in new webrev. :)
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.
Ok.
451 int count = frame_count+start_index;
Minor: Need spaces around the '=' and '+’.
Fixed.
Thank you for making the changes! I do not need another webrev. Amazing
work in general! Thanks, Serguei
Thanks
Mandy
