Skip to content

Commit d7329a2

Browse files
authored
case should only evaluate the matching clause expr (#700)
Hi, can you please consider a patch to evaluate only the expression that matches the clause. It fixes #699 . The issue was that `~(apply hash-map pairs)` evaluated all the clauses expressions. With this update, the clauses are turned into `cond` clauses, taking extra case to quote any symbol bindings. I tried to keep as match of the old code as possible, I could not think of any other way of doing so. It seems as if this requires a special form to as not to evaluate the clauses (e.g. `cond`). The drawback with this approach is that we went from O(1) to find the right expr to return it's value, to O(n). Feel free to update or discard in case there is an obvious better solution. I've included a test to confirm the same. Thanks, (the corresponding def in clojure-1.11 is [quite involved](https://github.com/clojure/clojure/blob/ce55092f2b2f5481d25cff6205470c1335760ef6/src/clj/clojure/core.clj#L6748)) --------- Co-authored-by: ikappaki <[email protected]>
1 parent f6bebc5 commit d7329a2

File tree

3 files changed

+12
-3
lines changed

3 files changed

+12
-3
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
55
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
66

77
## [Unreleased]
8+
* Fix issue with `case` evaluating all of its clauses expressions (#699).
89

910
## [v0.1.0a2]
1011
### Added

src/basilisp/core.lpy

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3401,7 +3401,8 @@
34013401

34023402
The clauses are pairs of a matching value and a return value. The matching values are
34033403
not evaluated and must be compile-time constants. Symbols will not be resolved. Lists
3404-
may be passed to match multiple compile time values to a single return value."
3404+
may be passed to match multiple compile time values to a single return value. The
3405+
dispatch is done in constant time."
34053406
[expr & clauses]
34063407
(when (< (count clauses) 2)
34073408
(throw (ex-info "case expression must have at least one clause"
@@ -3412,7 +3413,7 @@
34123413
pairs (mapcat identity
34133414
(mapcat (fn [pair]
34143415
(let [binding (first pair)
3415-
expr (second pair)]
3416+
expr `(fn [] ~(second pair))]
34163417
(if (list? binding)
34173418
(map #(vector (list 'quote %) expr) binding)
34183419
[[(list 'quote binding) expr]])))
@@ -3427,7 +3428,7 @@
34273428
~(if (= default :basilisp.core.case/no-default)
34283429
`(throw (python/ValueError (str "No case clause matches " ~expr-gs)))
34293430
`~default)
3430-
(get m# ~expr-gs)))))
3431+
((get m# ~expr-gs))))))
34313432

34323433
(defmacro condp
34333434
"Take a predicate and an expression and a series of clauses, call ``(pred test expr)``

tests/basilisp/test_core_macros.lpy

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,13 @@
326326
(is (= "also no" (case 'c :a "no" (b c d) "also no" "duh")))
327327
(is (= "also no" (case 'd :a "no" (b c d) "also no" "duh")))
328328
(is (= "duh" (case 'e :a "no" (b c d) "also no" "duh")))
329+
(let [out* (atom [])]
330+
(is (= 2 (case :2
331+
:1 (do (swap! out* conj 1) 1)
332+
:2 (do (swap! out* conj 2) 2)
333+
:3 (do (swap! out* conj 3) 3)
334+
(do (swap! out* conj 4) 4))))
335+
(is (= [2] @out*)))
329336
(is (thrown? python/ValueError
330337
(case "da" :b "nope" :c "mega nope" :a "yes"))))
331338

0 commit comments

Comments
 (0)