Skip to content

Instantly share code, notes, and snippets.

@ptaoussanis
Last active August 29, 2015 14:07
  • Star 0 You must be signed in to star a gist
  • Fork 1 You must be signed in to fork a gist
Star You must be signed in to star a gist
Save ptaoussanis/e599719d5fb6828a31b1 to your computer and use it in GitHub Desktop.
Faulty ns macro desugaring
(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]))))
From 5f20005531d8280c2ee4b8367c9b6c08065e06b5 Mon Sep 17 00:00:00 2001
From: Peter Taoussanis <ptaoussanis@taoensso.com>
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment