[CLJ-734] starting scope of let bindings seems incorrect from jdi perspective Created: 30/Jan/11 Updated: 11/Mar/11 Resolved: 11/Mar/11
|Affects Version/s:||Release 1.2|
|Fix Version/s:||Release 1.3|
It appears like the clojure compiler doesn't get the starting scope of let bindings right from the java debug interface perspective.
The compiler sets the starting scope of a local to the let/loop body, rather than to immediately after the local is bound. So when you are stepping through a let statement in a debugger, you can't eval the already defined bindings until you get to the let body.
Because it is a compiler problem, you see the symptoms with any jdi based debugger, (I've confirmed this on both jdb and cdt.)
The fix is pretty straightforward. Just gen.mark() a label after each binding is emitted and use that in the gen.visitLocalVariable() call instead of the loopLabel. I've attached a patch for clojure 1.2 branch. (If there is interest I'll create one for clojure master.)
|Comment by George Jahad [ 30/Jan/11 7:15 PM ]|
here is the diff against the current clojure master branch.
|Comment by George Jahad [ 30/Jan/11 10:25 PM ]|
re-added patch in proper format
|Comment by George Jahad [ 31/Jan/11 12:41 PM ]|
To test: start clojure with a jdi port, (like 8050 below)
Then start jdb and attach to that port like so:
Set a breakpoint on pmap from jdb:
Invoke pmap from the clojure repl:
(pmap inc (range 6))
From jdb, step over the first line of pmap, (that binds the local n):
From jdb, try to print the just bound local:
Without the patch, you'll see:
With it, you should see n equal to the correct value:
|Comment by Stuart Halloway [ 22/Feb/11 5:29 PM ]|
Rich: does this need to be rewritten so that BindingInit is a value, or is the mutation of startScope ok?
Patch works against master.
|Comment by Rich Hickey [ 25/Feb/11 9:40 AM ]|
Yes, it is weird to mutate bindinginit with emit-related info. Better to keep a map of bindinginit->label inside doEmit, please.
|Comment by George Jahad [ 26/Feb/11 12:39 PM ]|
fixed to use HashMap of labels
|Comment by Stuart Halloway [ 04/Mar/11 9:21 PM ]|
Feb 26 patch is good