Skip to content

Instantly share code, notes, and snippets.

@ptaoussanis

ptaoussanis/CLJS-866.clj

Last active August 29, 2015 14:07
Show Gist options

  • Select an option

    Learn more about clone URLs
  • Save ptaoussanis/e599719d5fb6828a31b1 to your computer and use it in GitHub Desktop.

Select an option

Learn more about clone URLs
Save ptaoussanis/e599719d5fb6828a31b1 to your computer and use it in GitHub Desktop.
Download ZIP

Revisions

  1. ptaoussanis revised this gist Oct 3, 2014. 1 changed file with 46 additions and 0 deletions.
    46 changes: 46 additions & 0 deletions CLJS-866.patch
    View file
    Original file line number Diff line number Diff line change
    @@ -0,0 +1,46 @@
    From 5f20005531d8280c2ee4b8367c9b6c08065e06b5 Mon Sep 17 00:00:00 2001
    From: Peter Taoussanis <[email protected]>
    Date: Fri, 3 Oct 2014 13:31:38 +0700
    Subject: [PATCH] CLJS-866: Faulty ns macro desugaring

    Fixes:
    * `:refer-macros` sugar producing an invalid expansion.
    * `:include-macros` sugar behaving unpredictably.
    ---
    src/clj/cljs/analyzer.clj | 16 +++++++++-------
    1 file changed, 9 insertions(+), 7 deletions(-)

    diff --git a/src/clj/cljs/analyzer.clj b/src/clj/cljs/analyzer.clj
    index 08f2ad0..56f10ec 100644
    --- a/src/clj/cljs/analyzer.clj
    +++ b/src/clj/cljs/analyzer.clj
    @@ -1131,18 +1131,20 @@
    (map (fn [[k & specs]] [k (into [] specs)]))
    (into {}))
    sugar-keys #{:include-macros :refer-macros}
    + remove-from-spec
    + (fn [pred spec]
    + (if-not (and (sequential? spec) (some pred spec))
    + spec
    + (let [[l r] (split-with (complement pred) spec)]
    + (recur pred (concat l (drop 2 r))))))
    to-macro-specs
    (fn [specs]
    (->> specs
    (filter #(and (sequential? %) (some sugar-keys %)))
    - (map #(->> % (remove #{:include-macros true})
    + (map #(->> % (remove-from-spec #{:include-macros})
    + (remove-from-spec #{:refer})
    (map (fn [x] (if (= x :refer-macros) :refer x)))))))
    - remove-sugar
    - (fn [spec]
    - (if (and (sequential? spec) (some sugar-keys spec))
    - (let [[l & r] (split-with #(not (contains? sugar-keys %)) spec)]
    - (concat l (drop 2 r)))
    - spec))]
    + remove-sugar (partial remove-from-spec sugar-keys)]
    (if-let [require-specs (seq (to-macro-specs require))]
    (map (fn [[k v]] (cons k (map remove-sugar v)))
    (update-in indexed [:require-macros] (fnil into []) require-specs))
    --
    1.9.3 (Apple Git-50)
  2. ptaoussanis created this gist Oct 1, 2014.
    47 changes: 47 additions & 0 deletions CLJS-866.clj
    View file
    Original file line number Diff line number Diff line change
    @@ -0,0 +1,47 @@
    (comment
    ;; Bug report for CLJS-721 (support :include-macros true modifier in :require),
    ;; Ref. http://dev.clojure.org/jira/browse/CLJS-721,
    ;; http://goo.gl/MQ3fWd (GitHub commit de6ee41b3)

    ;; desugar-ns-specs from clojurescript/src/clj/cljs/analyzer.clj
    ;; (`cljs.analyzer` ns)

    (desugar-ns-specs '[(:require [foo.bar :as bar :include-macros true])])
    ;; =>
    ((:require-macros (foo.bar :as bar))
    (:require (foo.bar :as bar))) ; Correct

    (desugar-ns-specs '[(:require [foo.bar :as bar :include-macros true :refer [baz]])])
    ;; =>
    ((:require-macros (foo.bar :as bar :refer [baz]))
    (:require (foo.bar :as bar))) ; Seems off, but what's the intended behaviour?

    (desugar-ns-specs '[(:require [foo.bar :as bar :refer [baz] :include-macros true])])
    ;; =>
    ((:require-macros (foo.bar :as bar :refer [baz]))
    (:require (foo.bar :as bar :refer [baz])))

    ;; Is the position of `:include-macros true` supposed to influence expansion
    ;; like this?

    ;; And is the interaction between `:include-macros true` and `:refer` as
    ;; intended? I would have expected something like this:
    (desugar-ns-specs '[(:require [foo.bar :as bar :include-macros true :refer [baz]])])
    ;; =
    (desugar-ns-specs '[(:require [foo.bar :as bar :refer [baz] :include-macros true])])
    ;; =>
    ((:require-macros (foo.bar :as bar)) ; No :refer
    (:require (foo.bar :as bar :refer [baz])))

    ;;; --------------------------------------------------------------------------

    ;; There's an additional problem with `:refer` + `:refer-macros`:

    (desugar-ns-specs '[(:require [foo.bar :as bar :refer [baz] :refer-macros [qux]])])
    ;; =>
    ((:require-macros (foo.bar :as bar :refer [baz] :refer [qux])) ; Faulty
    (:require (foo.bar :as bar :refer [baz])))

    ;; I believe the correct expansion should be:
    ((:require-macros (foo.bar :as bar :refer [qux]))
    (:require (foo.bar :as bar :refer [baz]))))