ClojureScript

REPL def symbol init collision

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: 1.7.145
  • Fix Version/s: 1.9.293
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code and Test

Description

In a REPL, if you try def where the init is a local matching the symbol being defined, then analysis fails.

(let [x 1]
  (def x x))

This can be verified in script/noderepljs and you can see it is some bad interaction with REPL var emission because if :def-emits-var false is added to the script, things work.

Activity

Hide
Txus Bach added a comment - - edited

Confirmed that evaluating this breaks:

(require 'clojure.tools.reader
         'cljs.analyzer
         'cljs.compiler)

(let [env '{:ns {:name cljs.user}
            :def-emits-var true
            :locals {}}]
  (->> "(let [x 1] (def x 3))"
       clojure.tools.reader/read-string
       (cljs.analyzer/analyze env)
       cljs.compiler/emit-str))

(When emitting, not when just analyzing.)

Removing `:def-emits-var true` makes the bug disappear.

Looking into how to fix it – any clues are welcome, it's my first time around the codebase

Show
Txus Bach added a comment - - edited Confirmed that evaluating this breaks:
(require 'clojure.tools.reader
         'cljs.analyzer
         'cljs.compiler)

(let [env '{:ns {:name cljs.user}
            :def-emits-var true
            :locals {}}]
  (->> "(let [x 1] (def x 3))"
       clojure.tools.reader/read-string
       (cljs.analyzer/analyze env)
       cljs.compiler/emit-str))
(When emitting, not when just analyzing.) Removing `:def-emits-var true` makes the bug disappear. Looking into how to fix it – any clues are welcome, it's my first time around the codebase
Hide
Txus Bach added a comment -

Seems that var-ast is returning nil, because resolve-var doesn't return a map with an :ns key. Not sure if this code should work, but it returns nil:

(let [env {:ns {:name 'cljs.user},
 :def-emits-var true,
 :locals
 {'x
  {:init
   {:op :constant,
    :env
    {:ns {:name 'cljs.user},
     :def-emits-var true,
     :locals {},
     :line nil,
     :column nil,
     :context :expr},
    :form 1,
    :tag 'number},
   :name 'x,
   :binding-form? true,
   :op :var,
   :env {:line nil, :column nil},
   :column nil,
   :line nil,
   :info {:name 'x, :shadow nil},
   :tag 'number,
   :shadow nil,
   :local true}}}]
  (ana/resolve-var env 'x))

Will continue tomorrow. It's so much fun!

Show
Txus Bach added a comment - Seems that var-ast is returning nil, because resolve-var doesn't return a map with an :ns key. Not sure if this code should work, but it returns nil:
(let [env {:ns {:name 'cljs.user},
 :def-emits-var true,
 :locals
 {'x
  {:init
   {:op :constant,
    :env
    {:ns {:name 'cljs.user},
     :def-emits-var true,
     :locals {},
     :line nil,
     :column nil,
     :context :expr},
    :form 1,
    :tag 'number},
   :name 'x,
   :binding-form? true,
   :op :var,
   :env {:line nil, :column nil},
   :column nil,
   :line nil,
   :info {:name 'x, :shadow nil},
   :tag 'number,
   :shadow nil,
   :local true}}}]
  (ana/resolve-var env 'x))
Will continue tomorrow. It's so much fun!
Hide
António Nuno Monteiro added a comment -

Attached patch with fix and test.

Show
António Nuno Monteiro added a comment - Attached patch with fix and test.
Hide
Mike Fikes added a comment -

I tried António's patch with various expressions in the REPL and couldn't find a way to break it.

Show
Mike Fikes added a comment - I tried António's patch with various expressions in the REPL and couldn't find a way to break it.
Hide
David Nolen added a comment - - edited

Would like to address the fact that `((let [x 1] (defn x [] x))` does not appear to work in the REPL. I'm looking into it.

Show
David Nolen added a comment - - edited Would like to address the fact that `((let [x 1] (defn x [] x))` does not appear to work in the REPL. I'm looking into it.
Hide
António Nuno Monteiro added a comment -

Yeah, that's weird. This is what I tested with and works:

cljs.user=> (let [x 1] (defn x [] x))
#'cljs.user/x
cljs.user=> (x)
1
Show
António Nuno Monteiro added a comment - Yeah, that's weird. This is what I tested with and works:
cljs.user=> (let [x 1] (defn x [] x))
#'cljs.user/x
cljs.user=> (x)
1
Hide
António Nuno Monteiro added a comment -

Attached a new patch that addresses the recently raised issues. Also includes tests for them.

Show
António Nuno Monteiro added a comment - Attached a new patch that addresses the recently raised issues. Also includes tests for them.

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: