Clojure

starting scope of let bindings seems incorrect from jdi perspective

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: Release 1.2
  • Fix Version/s: Release 1.3
  • Component/s: None
  • Labels:
    None
  • Patch:
    New
  • Approval:
    Ok

Description

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.)

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
George Jahad added a comment -

here is the diff against the current clojure master branch.

Show
George Jahad added a comment - here is the diff against the current clojure master branch.
George Jahad made changes -
Field Original Value New Value
Attachment letBindings-master.diff [ 10100 ]
Rich Hickey made changes -
Fix Version/s Release.Next [ 10038 ]
Approval Vetted
George Jahad made changes -
Attachment letBindings-master.diff [ 10100 ]
George Jahad made changes -
Attachment letBindings1-2.diff [ 10099 ]
Hide
Permalink
George Jahad added a comment -

re-added patch in proper format

Show
George Jahad added a comment - re-added patch in proper format
George Jahad made changes -
Attachment clj-734.diff [ 10105 ]
Rich Hickey made changes -
Approval Vetted Test
Hide
Permalink
George Jahad added a comment -

To test: start clojure with a jdi port, (like 8050 below)
java -agentlib:jdwp=transport=dt_socket,address=8050,server=y,suspend=n -cp /Users/georgejahad/incoming/clojure-1.3.0-alpha4/clojure.jar clojure.main

Then start jdb and attach to that port like so:
jdb -attach 8050

Set a breakpoint on pmap from jdb:
stop in clojure.core$pmap.invoke

Invoke pmap from the clojure repl:

(pmap inc (range 6))

From jdb, step over the first line of pmap, (that binds the local n):
next

From jdb, try to print the just bound local:
print n

Without the patch, you'll see:
com.sun.tools.example.debug.expr.ParseException: Name unknown: n
n = null

With it, you should see n equal to the correct value:
n = 4

Show
George Jahad added a comment - To test: start clojure with a jdi port, (like 8050 below) java -agentlib:jdwp=transport=dt_socket,address=8050,server=y,suspend=n -cp /Users/georgejahad/incoming/clojure-1.3.0-alpha4/clojure.jar clojure.main Then start jdb and attach to that port like so: jdb -attach 8050 Set a breakpoint on pmap from jdb: stop in clojure.core$pmap.invoke Invoke pmap from the clojure repl: (pmap inc (range 6)) From jdb, step over the first line of pmap, (that binds the local n): next From jdb, try to print the just bound local: print n Without the patch, you'll see: com.sun.tools.example.debug.expr.ParseException: Name unknown: n n = null With it, you should see n equal to the correct value: n = 4
Hide
Permalink
Stuart Halloway added a comment -

Rich: does this need to be rewritten so that BindingInit is a value, or is the mutation of startScope ok?

Patch works against master.

Show
Stuart Halloway added a comment - Rich: does this need to be rewritten so that BindingInit is a value, or is the mutation of startScope ok? Patch works against master.
Stuart Halloway made changes -
Approval Test Screened
Hide
Permalink
Rich Hickey added a comment -

Yes, it is weird to mutate bindinginit with emit-related info. Better to keep a map of bindinginit->label inside doEmit, please.

Show
Rich Hickey added a comment - Yes, it is weird to mutate bindinginit with emit-related info. Better to keep a map of bindinginit->label inside doEmit, please.
Rich Hickey made changes -
Approval Screened Incomplete
Waiting On stu
Stuart Halloway made changes -
Waiting On stu george
George Jahad made changes -
Attachment clj-734.diff [ 10105 ]
Hide
Permalink
George Jahad added a comment -

fixed to use HashMap of labels

Show
George Jahad added a comment - fixed to use HashMap of labels
George Jahad made changes -
Attachment clj-734.diff [ 10124 ]
Hide
Permalink
Stuart Halloway added a comment -

Feb 26 patch is good

Show
Stuart Halloway added a comment - Feb 26 patch is good
Stuart Halloway made changes -
Approval Incomplete Screened
Waiting On george
Rich Hickey made changes -
Approval Screened Ok
Stuart Halloway made changes -
Status Open [ 1 ] Closed [ 6 ]
Resolution Completed [ 1 ]

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4#649-r158309) | Report a problem
Powered by a free Atlassian JIRA open source license for Clojure. Try JIRA - bug tracking software for your team.