Hi Alex,
Can you put "expected statement printed by jdb" in a static final since
it is used in 2 places? Otherwise it looks good. I don't need to see an
updated webrev.
thanks,
Chris
On 9/20/18 10:24 AM, Alex Menkov wrote:
Looks like JDK-4660756 fixed line number, but didn't fix source.
I filled JDK-8210927 for this.
New webrev:
http://cr.openjdk.java.net/~amenkov/sh2java/step4/webrev.02/
changes (vs ver .01):
- updated comment in StringConvertTest.java about arrays;
- RedefineTTYLineNumber was updated to verify line numbers,
verification of the source at breakpoint is added, but commented out
(should be uncommented by fix for JDK-8210927)
--alex
On 09/14/2018 18:09, Chris Plummer wrote:
Hmm. I thought that's what the original bug was addressing.
27 * @summary TTY: Need to clear source cache after doing a
redefine class
Chris
On 9/14/18 4:37 PM, Alex Menkov wrote:
Looks like only line numbers are reported correctly, but the content
of the line content if not correct :)
[jdb] Breakpoint hit: "thread=main", RedefineTTYLineNumberTarg.A(),
line=47 bci=0
[jdb] 47 System.out.println("in A, about to call B");
[jdb]
[jdb] main[1]
[jdb] Breakpoint hit: "thread=main", RedefineTTYLineNumberTarg.A(),
line=46 bci=0
[jdb] 46 public void A() {
[jdb]
[jdb] main[1]
"public void A()" is a line 46 in the original file
--alex
On 09/14/2018 15:32, Chris Plummer wrote:
I think checking the output after the second breakpoint would be a
good idea. However, rather than checking the line number, maybe
just check the contents of the line, which should be included in
the breakpoint event output.
Chris
On 9/14/18 3:23 PM, Alex Menkov wrote:
Hi Chris,
The file history does not contain any info about line number
dependency.
I'll remove "11 before, 10 afterward" comment.
Actually the test is not clear to me.
Accordingly the test description jdb report obsolete line number
in the case, but the test does not verify its correctness, but
just checks _debuggee_ (not jdb) output for absence of "Internal
exception".
The original bug is ancient, so it's hard to say if the test is
correct or not.
I can add extra testing - extract reported line numbers (by using
regexp "line=(\d+)\b") and verify that 2st breakpoint is reported
with the expected line number (1 less than line of the 1st
breakpoint).
Thoughts?
--alex
On 09/14/2018 14:27, Chris Plummer wrote:
Hi Alex,
Just one issue I see. For RedefineTTYLineNumber.java, the
original test used to have this comment, which your removed:
52 // line number sensitive!!! Next line must be line 10.
It's not clear to me why this test was ever line number
sensitive, and whether you removed this sensitivity, or it just
never existed. In any case, you left in the following comment,
which maybe should also be removed:
47 System.out.println("in A, about to call B"); // 11
before, 10 afterward
Also, the println output from A() does not seem to match what the
test is doing. There is no call to B():
46 public void A() {
47 System.out.println("in A, about to call B"); // 11
before, 10 afterward
48 System.out.println("out from B");
49 }
Maybe that's some bit rot. My understanding of the point of the
test is while at the breakpoint at the start of A(), a redefine
is done that deletes a line above this point, and jdi needs to
make the appropriate adjustment of the current breakpoint line
number. So calling B() does not play a roll in this, but perhaps
it did a one point but the call was removed.
Also, I don't see any indication of line number sensitivity here,
but once again, maybe this is a bit rot issue and at one point it
was line number sensitive.
thanks,
Chris
On 9/14/18 12:59 PM, Alex Menkov wrote:
Hi all,
please review fix for
https://bugs.openjdk.java.net/browse/JDK-8210760
webrev:
http://cr.openjdk.java.net/~amenkov/sh2java/step4/webrev.01/
--alex
