ClojureScript

Analyzer incorrectly merges metadata when redefining vars

Details

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

Description

I'm experimenting with using the analyzer for some more sophisticated macros, including a CPS transform and control constructs. During interactive development, I discovered that the analyzer is incorrectly merging metadata on vars when redefining them. This patch changes redef's to replace, rather than merge, existing var metadata.

The patch does not include a test, since the tests don't currently muck with the analyzer directly. Here's some code you can play with in your repl:

(require '[cljs.analyzer :as ana])
(require '[cljs.compiler :as comp])

(def env (ana/empty-env))

(defn show-foo [form]
  (ana/analyze env form)
  (-> @ana/namespaces (get 'cljs.user) :defs (get 'x) :foo))

(show-foo '(def ^{:foo 1} x 1))

(show-foo '(def ^{:foo 2} x 1))

(show-foo '(def x 1))  ; before patch, this returns 2. After patch, nil.
  1. redef-meta.patch
    26/Aug/12 6:22 PM
    3 kB
    Brandon Bloom
  2. redef-meta-2.patch
    31/Aug/12 11:22 AM
    2 kB
    Brandon Bloom

Activity

Hide
Brandon Bloom added a comment -

Also, the patch makes the behavior match JVM Clojure:

user=> (def ^:foo x)
#'user/x
user=> (:foo (meta #'x))
true
user=> (def x)
#'user/x
user=> (:foo (meta #'x))
nil

Show
Brandon Bloom added a comment - Also, the patch makes the behavior match JVM Clojure: user=> (def ^:foo x) #'user/x user=> (:foo (meta #'x)) true user=> (def x) #'user/x user=> (:foo (meta #'x)) nil
David Nolen made changes -
Field Original Value New Value
Description I'm experimenting with using the analyzer for some more sophisticated macros, including a CPS transform and control constructs. During interactive development, I discovered that the analyzer is incorrectly merging metadata on vars when redefining them. This patch changes redef's to replace, rather than merge, existing var metadata.


The patch does not include a test, since the tests don't currently muck with the analyzer directly. Here's some code you can play with in your repl:


(require '[cljs.analyzer :as ana])
(require '[cljs.compiler :as comp])

(def env (ana/empty-env))

(defn show-foo [form]
  (ana/analyze env form)
  (-> @ana/namespaces (get 'cljs.user) :defs (get 'x) :foo))

(show-foo '(def ^{:foo 1} x 1))

(show-foo '(def ^{:foo 2} x 1))

(show-foo '(def x 1)) ; before patch, this returns 2. After patch, nil.
I'm experimenting with using the analyzer for some more sophisticated macros, including a CPS transform and control constructs. During interactive development, I discovered that the analyzer is incorrectly merging metadata on vars when redefining them. This patch changes redef's to replace, rather than merge, existing var metadata.


The patch does not include a test, since the tests don't currently muck with the analyzer directly. Here's some code you can play with in your repl:

{code}
(require '[cljs.analyzer :as ana])
(require '[cljs.compiler :as comp])

(def env (ana/empty-env))

(defn show-foo [form]
  (ana/analyze env form)
  (-> @ana/namespaces (get 'cljs.user) :defs (get 'x) :foo))

(show-foo '(def ^{:foo 1} x 1))

(show-foo '(def ^{:foo 2} x 1))

(show-foo '(def x 1)) ; before patch, this returns 2. After patch, nil.
{code}
Hide
David Nolen added a comment -

Can we get a patch without whitespace changes? Thanks!

Show
David Nolen added a comment - Can we get a patch without whitespace changes? Thanks!
Hide
Brandon Bloom added a comment -

Updated patch: no longer changes indent/whitespace and resolves conflict with change that happened to master since original patch.

Show
Brandon Bloom added a comment - Updated patch: no longer changes indent/whitespace and resolves conflict with change that happened to master since original patch.
Brandon Bloom made changes -
Attachment redef-meta-2.patch [ 11464 ]
David Nolen made changes -
Resolution Completed [ 1 ]
Status Open [ 1 ] Resolved [ 5 ]
David Nolen made changes -
Status Resolved [ 5 ] Closed [ 6 ]

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: