JosephFerano/flan
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

or hands back its deciding operand instead of a bare bool, review pass on item 7

Browse Source 
The dyn if truthiness review turned up that or's answer position, unlike
and's, still traps on a non-bool dyn value: or's short-circuit sentinel
sat in the then arm of its own if, the one check_if types first, so that
sentinel decided the whole expression's type and a later non-bool dyn
answer hit the strict bool boundary and unboxed itself into a trap
rather than surviving as itself. (or nil "x") — the canonical Clojure
(or x default) idiom — crashed instead of answering "x", identical on
all three backends.

or now binds its test to a temp and answers the temp itself, exactly the
way Clojure's own or macro expands: (or a b) becomes (let [t a] (if t t
b)), not (if a true b). The temp evaluates a once and lets the answer be
a without writing it a second time as the then arm; it is the temp's own
type check_if sees first, so or hands back the actual truthy operand the
same way and always has. Verified real output, unchanged, on LLVM, -O0
and --x86, and the survey program now exercises the case its own header
used to exclude for being unsafe: a non-bool value stopping or and being
handed back as-is.

check_truthy also gets three corrections a closer look found. Its own
[loc] used to come from the enclosing if/while/not rather than from the
condition itself, so the rt call and cast it builds carried the wrong
column in an --x86 disassembly or the dev inspector whenever the
condition was not the form's first token; it now takes loc from the
scrutinee's own AST node, confirmed against a real --x86 dump. A comment
now names the precondition its exception-swallowing retry rests on: none
of check.ml's save-restore sites (barrier, in_frames, in_defer, loops,
scope) are exception-safe, which is harmless only because the retry
always either succeeds cleanly or re-raises and aborts the compile
before ctx is read again — and would stop being harmless the day some
want-sensitive elaboration on this path could succeed differently on
retry. And a bare keyword condition, which used to be checked with
want:Bool from the start and refused by the keyword arm's enum-or-refuse
case, now resolves as the dyn keyword instead and is unconditionally
truthy — a deliberate loss of that diagnostic, the author's call, pinned
in test_flan.ml so it does not regress by accident.

The two typed-refusal messages captured before this pass (a float
literal condition, an i32 while condition) are unchanged, checked again
against the same baseline. test_flan.ml's parser test for or's shape is
updated to match the new let-bound desugaring.
This commit is contained in:
Joseph Ferano 2026-09-20 09:56:34 +07:00
parent d7070501e4
commit ad0f1fbd6a
5 changed files with 130 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

69
lib/check.ml
View File

@ -2032,7 +2032,7 @@ let rec check ctx ?want (e : Ast.expr) : Tast.expr =
top of every trip — but it stays outside [in_loop], because a [break]
in a condition still means the enclosing loop and a [defer] there is
still the outer block's. *)
let c = check_truthy ctx loc c in
let c = check_truthy ctx c in
let body = in_loop ctx ?label (fun () ->
scoped ctx (fun () -> map_lr (fun b -> check ctx b) body))
in
@ -3177,9 +3177,10 @@ and check_recur ctx ~tail loc args =
(* Every boolean-position test in the language funnels through here: [if]'s
own condition, [while]'s, and [not]'s argument. ([when] and [cond] are
sugar built out of [Ast.If] in parse.ml, so they get this for free
without a separate case. [and] and [or] are sugar too, and their *tests*
get this for free the same way — see [shortcircuit] in parse.ml for why
[or]'s answer position does not.)
without a separate case. [and] and [or] are sugar too, and both their
tests and their answers get this for free the same way — see
[shortcircuit] in parse.ml for how [or] carries its deciding operand
through, the same way [and] always has.)
A dyn scrutinee is tested for truthiness, Clojure's rule: nil and false
are the only falsey values, and everything else — 0, "", an empty vec, an
@ -3199,8 +3200,60 @@ and check_recur ctx ~tail loc args =
found None". Asking the first way first is what makes the dyn case work,
so the refusal path — success or exception both — asks the second way
again, after the fact, purely to get the sentence a typed if has always
given. *)
and check_truthy ctx loc c =
given.
[loc] comes from [c] itself, not from the caller's [if]/[while]/[not] —
the built-in rt call and cast have to sit at the condition's own position
or an --x86 disassembly and the dev inspector point at the wrong column
when the two differ (an [if] whose test is not its first token).
The [exception Loc.Error _] arm swallows a rejection from arbitrary depth
inside [c] and re-runs the whole of [check ctx c] a second time, which is
sound only because every one of [ctx]'s save-restore sites — [barrier],
and the [in_frames]/[in_defer]/[loops]/[scope] plumbing [check] itself
uses — is not exception-safe: a failure mid-walk can leave one of those
pushed without its pop. Today that is harmless, because the second call
always either succeeds outright or raises again and this function's own
caller then aborts the compile — nothing downstream ever reads [ctx]
again on that path. It would stop being harmless the day some later
want-sensitive elaboration on this path can *succeed* by yielding a
concrete [Bool] on retry rather than failing a second time: then the
first, swallowed pass's half-restored state and any name or slot it
registered before raising would both still be live.
That same retry-on-failure is also, deliberately, not made cheaper by
only retrying at the leaf that actually needs a nicer message (an int or
float literal, or [None]) and re-raising everywhere else: the shorter
path was tried and shelved, because "everywhere else" is not safe to
generalise past [not]'s one level of nesting — a condition that is
itself a compound form carrying its own literals arbitrarily deep (an
[if] or [let] standing where a condition is expected) would need [want]
threaded through exactly as far as this function's own second call
already threads it, and stopping short changes which of *those*
literals gets the nicer message, not just the speed. The cost that
buys is real: nested [not] on a program that does not type-check re-runs
this whole function once per level of nesting inside the level above it,
which is exponential in how deep the nesting goes — moot for a program
that compiles, since neither retry ever fires, and moot for ordinary
nesting depths, but visible within a second or so around twenty levels
of a [not] wrapped in a [not] wrapped in .... The dev daemon is the one
caller that could feel this, recompiling a half-typed form on every
edit; nobody has hit it in practice and it is not fixed here.
Keywords are a separate, deliberate loss rather than a bug: a bare
[:kw] used to be checked here with [want:Types.Bool] from the start, so
it hit the keyword arm's [Some other] case and refused by name — "is an
enum member where an enum is expected and a dyn keyword elsewhere, but
bool is expected here". Checking it here with no expectation first, as
every other scrutinee now is, resolves it as the dyn keyword instead
(there being no enum in play), and dyn keywords are unconditionally
truthy — so [(if :kw a b)] now takes [a], where it used to refuse
outright. The author's call: lispy truthiness wins wherever it can, so
this refusal is given up on purpose and not special-cased back in;
test_flan.ml pins the new answer down so it is not lost again by
accident. *)
and check_truthy ctx c =
let loc = c.Ast.loc in
match check ctx c with
| c0 when c0.Tast.ty = Types.Dyn ->
widen loc Types.Bool (rt loc (Types.Int Types.I32) "flan_dyn_truthy" [ c0 ])
@ -3209,7 +3262,7 @@ and check_truthy ctx loc c =
| exception Loc.Error _ -> check ctx ~want:Types.Bool c
and check_if ctx ?(tail = false) ?want loc c t e =
let c = check_truthy ctx loc c in
let c = check_truthy ctx c in
(* Both arms are the tail, and a one-armed [if] counts: [(when c (recur ...))]
is how nearly every loop is written, and the branch is still the last
thing the body does. *)
@ -4249,7 +4302,7 @@ and named_call ctx ~want loc name args =
arity loc name 1 args;
(* Same truthiness as [if]: a dyn argument is negated on nil/false vs.
everything else, not narrowed to a strict bool first. *)
prim Tast.Not Types.Bool [ check_truthy ctx loc (List.hd args) ]
prim Tast.Not Types.Bool [ check_truthy ctx (List.hd args) ]
(* Bitwise operators are integers-only, and the shift count has the same type
as the value shifted — there is no implicit widening anywhere else either. *)
| "bit-and" | "bit-or" | "bit-xor" ->

35
lib/parse.ml
View File

@ -882,17 +882,24 @@ and cond f (args : Form.t list) : Ast.expr =
if args = [] then Loc.fail f.loc "cond needs at least one clause" else go args
(* Every test here is an [if]'s condition, so a dyn operand is truthy-tested
(check.ml's check_truthy) exactly the way a bare [if]'s is — that much is
symmetric between [and] and [or]. What is not symmetric is the *answer*:
[and]'s "false" sentinel sits in the else arm, so check_if picks the real
branch's type first and boxes "false" to match it, which is what lets
[and] hand back the actual last dyn value, Clojure-style, rather than a
bare bool. [or]'s "true" sentinel sits in the then arm instead — the one
check_if types *first* — so it is what decides the whole expression's
type when nothing upstream already demanded one, and a later non-bool dyn
answer then hits the strict bool boundary instead of surviving as itself.
Reordering [or] to match [and] is a real fix and a separate decision; not
this item's to make. *)
(check.ml's check_truthy) exactly the way a bare [if]'s is, for both
[and] and [or]. The *answer* used to be asymmetric between them: [and]'s
"false" sentinel sits in the else arm, so check_if picks the real
branch's type first and boxes "false" to match it, which lets [and] hand
back the actual last dyn value, Clojure-style. [or] used to put its
"true" sentinel in the then arm instead — the one check_if types first —
so that sentinel decided the whole expression's type, and a later
non-bool dyn answer hit the strict bool boundary instead of surviving as
itself: (or nil "x") traps rather than answering "x", exactly the
canonical (or x default) idiom Clojure is reached for.
[or] now binds its test to a temp and asks the temp itself, the way
Clojure's own or expands: [(or a b)] is [(let [t a] (if t t b))], not
[(if a true b)]. The temp is what lets the answer be [a] itself without
writing [a] a second time as the then arm — [(if a a b)] would evaluate
it twice, once for the test and again for the answer — and it is the
temp's own type check_if sees first, so [or] hands back the actual
truthy value the same way [and] hands back its own. *)
and shortcircuit f (args : Form.t list) ~is_and : Ast.expr =
let mk e = { Ast.e; loc = f.loc } in
let rec go = function
@ -900,7 +907,11 @@ and shortcircuit f (args : Form.t list) ~is_and : Ast.expr =
| [ last ] -> expr last
| x :: rest ->
if is_and then mk (Ast.If (expr x, go rest, Some (mk (Ast.Var "false"))))
else mk (Ast.If (expr x, mk (Ast.Var "true"), Some (go rest)))
else
let t = fresh_temp () in
let tvar = mk (Ast.Var t) in
let bind = { Ast.bname = t; bty = None; bval = expr x; bloc = f.loc } in
mk (Ast.Let ([ bind ], [ mk (Ast.If (tvar, tvar, Some (go rest))) ]))
in
go args

28
test/programs/dyn-if-truthy.flan
View File

@ -10,13 +10,15 @@
;;;; own syntax rather than by inspecting the desugaring. [and]'s *answer*
;;;; carries a non-bool dyn value through too, Clojure-style, because its
;;;; short-circuit sentinel is the else arm and the real value's type wins.
;;;; [or]'s sentinel is the then arm instead, so it is the one that decides
;;;; the whole expression's type when nothing else does, and [or]'s answer
;;;; stays a strict bool -- a pre-existing asymmetry (parse.ml, shortcircuit)
;;;; this item leaves alone; only [or]'s tests are exercised here, not a
;;;; non-bool value in its answer position. [not] and [while] are not [if]
;;;; in disguise, so check_truthy is called at their own sites by hand, and
;;;; get their own coverage too.
;;;; [or] used to put its sentinel in the then arm instead, so a non-bool
;;;; dyn answer hit the strict bool boundary and traps -- (or nil "x"), the
;;;; canonical Clojure (or x default) idiom, used to crash. [or] now binds
;;;; its test to a temp and answers the temp itself, Clojure's own
;;;; expansion, so its answer carries a non-bool dyn value through exactly
;;;; the way [and]'s does; both are exercised below, including the case
;;;; that used to be excluded here for being unsafe. [not] and [while] are
;;;; not [if] in disguise, so check_truthy is called at their own sites by
;;;; hand, and get their own coverage too.
;;;;
;;;; A typed if keeps needing a strict bool -- that refusal, and its message,
;;;; is a checker test in test_flan.ml, not a row here, since a program that
@ -66,14 +68,18 @@
;; and/or: also if in disguise, so each test along the chain is
;; truthy-tested the same way if's own is -- 0 and "" do not stop and,
;; only nil and false do; 0 does stop or, the way any truthy value does.
;; and's answer is the last truthy operand itself (:kw here), Clojure's
;; and. or's answer stays a plain bool -- "true" below is or's own
;; sentinel, not the operand that made it truthy; that asymmetry is
;; parse.ml's, not this item's to close.
;; Both hand back the actual operand that decided them, Clojure-style --
;; and's answer is the last truthy operand itself (:kw here); or's is the
;; first truthy one (0, then "x") rather than a bare true.
(println (and (box 1) (box "") (box :kw)))
(println (and (box 1) (box false) (box "unreached")))
(println (or (box 0) (box false)))
(println (or (box nil) (box false)))
;; The case excluded before the fix: a non-bool value stopping or and
;; being handed back as-is -- the canonical (or x default) idiom, which
;; used to trap trying to unbox "x" as a strict bool.
(println (or (box nil) (box "x")))
(println (or (box 5) (box "unreached")))
;; not: truthiness, negated -- true only for nil and false.
(println (not (box nil)))

18
test/test_acceptance.ml
View File

@ -1252,16 +1252,20 @@ let () =
nil/false vs. everything else, Clojure's rule, on both backends; a
typed scrutinee stays strictly bool, which is a checker test
(test_flan.ml) and not a row here since a typed if given a non-bool
scrutinee would not compile. [when], [cond] and [and]'s and [or]'s
*tests* ride along for free because they desugar to [if] in
parse.ml; [not] and [while] get the same treatment by hand in
check.ml's check_truthy. [and]'s answer carries a non-bool dyn value
through, Clojure-style; [or]'s answer stays a plain bool, a
pre-existing asymmetry in its desugaring this item leaves alone. *)
scrutinee would not compile. [when], [cond], [and] and [or] ride
along for free because their tests desugar to [if] in parse.ml;
[not] and [while] get the same treatment by hand in check.ml's
check_truthy. [and] and [or] both hand back the actual operand that
decided them, Clojure-style -- [or] used to answer a bare bool
instead, its short-circuit sentinel sitting in the arm check_if
types first, and a non-bool dyn value reaching that position (the
canonical (or x default) idiom) used to trap rather than survive as
itself; [or] now binds its test to a temp and answers the temp,
fixing that. *)
let dyn_if_truthy_out =
"falsey\nfalsey\ntruthy\ntruthy\ntruthy\ntruthy\ntruthy\ntruthy\ntruthy\n\
truthy\ntruthy\ntruthy\nwhen 0 ran\nwhen empty-string ran\nb\nb\n:kw\n\
false\ntrue\nfalse\ntrue\ntrue\nfalse\nfalse\nfalse\n3\n2\n1\n"
false\n0\nfalse\nx\n5\ntrue\ntrue\nfalse\nfalse\nfalse\n3\n2\n1\n"
in
outputs "dyn if truthiness" "programs/dyn-if-truthy.flan" dyn_if_truthy_out;
outputs ~opt:"-O0" "dyn if truthiness, -O0" "programs/dyn-if-truthy.flan"

19
test/test_flan.ml
View File

@ -354,8 +354,13 @@ let () =
(match (parse1 "(and a b)").e with
| If (_, _, Some { e = Var "false"; _ }) -> ()
| _ -> check "and short-circuits" false);
(* or binds its test to a temp and answers the temp itself -- Clojure's
own expansion, and what lets or hand back the actual truthy operand
rather than a bare true (M2 queue item 7's review pass). *)
(match (parse1 "(or a b)").e with
| If (_, { e = Var "true"; _ }, Some _) -> ()
| Let ([ _ ],
[ { e = If ({ e = Var t1; _ }, { e = Var t2; _ }, Some _); _ } ])
when t1 = t2 -> ()
| _ -> check "or short-circuits" false);
(* ── Forms that bind or alter control are never calls ──────────── *)
@ -875,6 +880,18 @@ let () =
"(defn f [x] i32 (let [y x] (if (and y true) 0 1)))";
accepts "dyn or accepts a non-bool dyn operand"
"(defn f [x] i32 (let [y x] (if (or y true) 0 1)))";
(* A bare keyword condition used to be checked with want:Bool from the
start, landing on the keyword arm's enum-or-refuse case and refusing
by name -- ":kw is an enum member where an enum is expected ... but
bool is expected here", since there was no enum in play. Checked with
no expectation first, as every scrutinee now is, it resolves as the
dyn keyword instead, and a dyn keyword is unconditionally truthy: a
typed if with a bare keyword condition now compiles, and always takes
the then branch. The author's call, recorded at check_truthy: lispy
truthiness wins here, the lost diagnostic is not brought back, and
this pins the new answer down so it does not regress by accident. *)
accepts "a typed if with a bare keyword condition now compiles"
"(defn f [] i32 (if :kw 1 2))";
(* ── Unknown types ─────────────────────────────────────────────── *)
(* A lowercase name is a type variable (plan.org, Types), so a mistyped