fix(destructure): apply :or on absence, not on falsiness - #29
Open
BuddhiLW wants to merge 1 commit into
Open
Conversation
Clojure applies a destructuring default only when the key is ABSENT.
cljel emitted (or (get m :x) 5), so {:keys [x] :or {x 5}} over {:x nil}
bound 5 instead of nil. All four binding forms - :keys, :strs, :syms and
explicit - now reach get's 3-arity.
The runtime half matters too: clel-get only distinguished absence from
falsiness for hash tables. Its list and alist branches used the same
`or`, so a 3-arity call would have kept the bug for alist-backed maps.
Both now check presence.
A second bug fell out of the same line. The default was read with
(get or-map sym), which cannot tell "no default" from "the default is
nil or false", so :or {x false} was dropped entirely. Now contains?.
The four binding forms each carried their own copy of the default
logic; they now share one `lookup`, so the next change to default
semantics happens once.
Proven at the execution rung: both new ERT tests fail against the
previous runtime and pass against this one, and a compiled .cljel using
all three binding forms was run in batch Emacs.
Closes 20260710102141-320e86ab.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes CLJEL-COMPILER map-destructuring-
:or(20260710102141-320e86ab).Fourth in the stack (#26 → #27 → #28 → this); bases retarget as each merges.
The bug
Clojure applies a destructuring default only when the key is absent. cljel
emitted
(or (get m :x) 5), so a present falsy value was overridden:All four binding forms were affected:
:keys,:strs,:symsand explicit.The runtime half
Repointing the emitter at
get's 3-arity is not enough on its own.clel-getonly distinguished absence from falsiness for hash tables — its list and
alist branches used the same
or, so the bug would have survived foralist-backed maps. Both branches now check presence, and index lookups tell a
nil element from an out-of-range index.
A second bug on the same line
The default was read with
(get or-map sym), which cannot tell "no default"from "the default is nil or false".
:or {x false}was therefore droppedentirely. It now uses
contains?.Structure
The four binding forms each carried their own copy of the default logic —
four places to get this right, and the reason one line had two bugs. They now
share one
lookup, so the next change to default semantics happens once.Evidence
Both new ERT tests fail against the previous runtime and pass against this
one. A compiled
.cljelexercising all three binding forms was run in batchEmacs:
610 Clojure tests, 3071 assertions, 0 failures. 30 ERT tests, 0 unexpected.