Fix cond ambiguity: check ALL clauses with cond-scheme?, not just first
The cond special form misclassified Clojure-style as scheme-style when the first test was a 2-element list like (nil? x) — treating it as a scheme clause ((test body)) instead of a function call. Define cond-scheme? using every? to check ALL clauses, fix eval.sx sf-cond and render.sx eval-cond, rewrite engine.sx parse-time/filter-params as nested if to avoid the ambiguity, add regression tests across eval/ render/aser specs. 378/378 tests pass. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
File diff suppressed because it is too large
Load Diff
@@ -539,6 +539,9 @@ class PyEmitter:
|
|||||||
clauses = expr[1:]
|
clauses = expr[1:]
|
||||||
if not clauses:
|
if not clauses:
|
||||||
return "NIL"
|
return "NIL"
|
||||||
|
# Check ALL clauses are 2-element lists (scheme-style).
|
||||||
|
# Checking only the first is ambiguous — (nil? x) is a 2-element
|
||||||
|
# function call, not a scheme clause ((test body)).
|
||||||
is_scheme = (
|
is_scheme = (
|
||||||
all(isinstance(c, list) and len(c) == 2 for c in clauses)
|
all(isinstance(c, list) and len(c) == 2 for c in clauses)
|
||||||
and not any(isinstance(c, Keyword) for c in clauses)
|
and not any(isinstance(c, Keyword) for c in clauses)
|
||||||
|
|||||||
@@ -34,11 +34,12 @@
|
|||||||
(define parse-time
|
(define parse-time
|
||||||
(fn (s)
|
(fn (s)
|
||||||
;; Parse time string: "2s" → 2000, "500ms" → 500
|
;; Parse time string: "2s" → 2000, "500ms" → 500
|
||||||
(cond
|
;; Uses nested if (not cond) because cond misclassifies 2-element
|
||||||
(nil? s) 0
|
;; function calls like (nil? s) as scheme-style ((test body)) clauses.
|
||||||
(ends-with? s "ms") (parse-int s 0)
|
(if (nil? s) 0
|
||||||
(ends-with? s "s") (* (parse-int (replace s "s" "") 0) 1000)
|
(if (ends-with? s "ms") (parse-int s 0)
|
||||||
:else (parse-int s 0))))
|
(if (ends-with? s "s") (* (parse-int (replace s "s" "") 0) 1000)
|
||||||
|
(parse-int s 0))))))
|
||||||
|
|
||||||
|
|
||||||
(define parse-trigger-spec
|
(define parse-trigger-spec
|
||||||
@@ -219,20 +220,19 @@
|
|||||||
;; Filter form parameters by sx-params spec.
|
;; Filter form parameters by sx-params spec.
|
||||||
;; all-params is a list of (key value) pairs.
|
;; all-params is a list of (key value) pairs.
|
||||||
;; Returns filtered list of (key value) pairs.
|
;; Returns filtered list of (key value) pairs.
|
||||||
(cond
|
;; Uses nested if (not cond) — see parse-time comment.
|
||||||
(nil? params-spec) all-params
|
(if (nil? params-spec) all-params
|
||||||
(= params-spec "none") (list)
|
(if (= params-spec "none") (list)
|
||||||
(= params-spec "*") all-params
|
(if (= params-spec "*") all-params
|
||||||
(starts-with? params-spec "not ")
|
(if (starts-with? params-spec "not ")
|
||||||
(let ((excluded (map trim (split (slice params-spec 4) ","))))
|
(let ((excluded (map trim (split (slice params-spec 4) ","))))
|
||||||
(filter
|
(filter
|
||||||
(fn (p) (not (contains? excluded (first p))))
|
(fn (p) (not (contains? excluded (first p))))
|
||||||
all-params))
|
all-params))
|
||||||
:else
|
(let ((allowed (map trim (split params-spec ","))))
|
||||||
(let ((allowed (map trim (split params-spec ","))))
|
(filter
|
||||||
(filter
|
(fn (p) (contains? allowed (first p)))
|
||||||
(fn (p) (contains? allowed (first p)))
|
all-params))))))))
|
||||||
all-params)))))
|
|
||||||
|
|
||||||
|
|
||||||
;; --------------------------------------------------------------------------
|
;; --------------------------------------------------------------------------
|
||||||
|
|||||||
@@ -309,14 +309,18 @@
|
|||||||
nil))))
|
nil))))
|
||||||
|
|
||||||
|
|
||||||
|
;; cond-scheme? — check if ALL clauses are 2-element lists (scheme-style).
|
||||||
|
;; Checking only the first arg is ambiguous — (nil? x) is a 2-element
|
||||||
|
;; function call, not a scheme clause ((test body)).
|
||||||
|
(define cond-scheme?
|
||||||
|
(fn (clauses)
|
||||||
|
(every? (fn (c) (and (= (type-of c) "list") (= (len c) 2)))
|
||||||
|
clauses)))
|
||||||
|
|
||||||
(define sf-cond
|
(define sf-cond
|
||||||
(fn (args env)
|
(fn (args env)
|
||||||
;; Detect scheme-style: first arg is a 2-element list
|
(if (cond-scheme? args)
|
||||||
(if (and (= (type-of (first args)) "list")
|
|
||||||
(= (len (first args)) 2))
|
|
||||||
;; Scheme-style: ((test body) ...)
|
|
||||||
(sf-cond-scheme args env)
|
(sf-cond-scheme args env)
|
||||||
;; Clojure-style: test body test body ...
|
|
||||||
(sf-cond-clojure args env))))
|
(sf-cond-clojure args env))))
|
||||||
|
|
||||||
(define sf-cond-scheme
|
(define sf-cond-scheme
|
||||||
|
|||||||
@@ -134,12 +134,8 @@
|
|||||||
;; (test body test body ...).
|
;; (test body test body ...).
|
||||||
(define eval-cond
|
(define eval-cond
|
||||||
(fn (clauses env)
|
(fn (clauses env)
|
||||||
(if (and (not (empty? clauses))
|
(if (cond-scheme? clauses)
|
||||||
(= (type-of (first clauses)) "list")
|
|
||||||
(= (len (first clauses)) 2))
|
|
||||||
;; Scheme-style
|
|
||||||
(eval-cond-scheme clauses env)
|
(eval-cond-scheme clauses env)
|
||||||
;; Clojure-style
|
|
||||||
(eval-cond-clojure clauses env))))
|
(eval-cond-clojure clauses env))))
|
||||||
|
|
||||||
(define eval-cond-scheme
|
(define eval-cond-scheme
|
||||||
|
|||||||
@@ -1357,9 +1357,13 @@ def sf_when(args, env):
|
|||||||
else:
|
else:
|
||||||
return NIL
|
return NIL
|
||||||
|
|
||||||
|
# cond-scheme?
|
||||||
|
def cond_scheme_p(clauses):
|
||||||
|
return every_p(lambda c: ((type_of(c) == 'list') if not sx_truthy((type_of(c) == 'list')) else (len(c) == 2)), clauses)
|
||||||
|
|
||||||
# sf-cond
|
# sf-cond
|
||||||
def sf_cond(args, env):
|
def sf_cond(args, env):
|
||||||
if sx_truthy(((type_of(first(args)) == 'list') if not sx_truthy((type_of(first(args)) == 'list')) else (len(first(args)) == 2))):
|
if sx_truthy(cond_scheme_p(args)):
|
||||||
return sf_cond_scheme(args, env)
|
return sf_cond_scheme(args, env)
|
||||||
else:
|
else:
|
||||||
return sf_cond_clojure(args, env)
|
return sf_cond_clojure(args, env)
|
||||||
@@ -1859,7 +1863,7 @@ def render_attrs(attrs):
|
|||||||
|
|
||||||
# eval-cond
|
# eval-cond
|
||||||
def eval_cond(clauses, env):
|
def eval_cond(clauses, env):
|
||||||
if sx_truthy(((not sx_truthy(empty_p(clauses))) if not sx_truthy((not sx_truthy(empty_p(clauses)))) else ((type_of(first(clauses)) == 'list') if not sx_truthy((type_of(first(clauses)) == 'list')) else (len(first(clauses)) == 2)))):
|
if sx_truthy(cond_scheme_p(clauses)):
|
||||||
return eval_cond_scheme(clauses, env)
|
return eval_cond_scheme(clauses, env)
|
||||||
else:
|
else:
|
||||||
return eval_cond_clojure(clauses, env)
|
return eval_cond_clojure(clauses, env)
|
||||||
@@ -3833,4 +3837,4 @@ def render(expr, env=None):
|
|||||||
|
|
||||||
def make_env(**kwargs):
|
def make_env(**kwargs):
|
||||||
"""Create an environment with initial bindings."""
|
"""Create an environment with initial bindings."""
|
||||||
return _Env(dict(kwargs))
|
return _Env(dict(kwargs))
|
||||||
|
|||||||
@@ -115,6 +115,13 @@
|
|||||||
(assert-equal "(p \"two\")"
|
(assert-equal "(p \"two\")"
|
||||||
(render-sx "(cond false (p \"one\") true (p \"two\") :else (p \"three\"))")))
|
(render-sx "(cond false (p \"one\") true (p \"two\") :else (p \"three\"))")))
|
||||||
|
|
||||||
|
(deftest "cond with 2-element predicate test"
|
||||||
|
;; Regression: cond misclassifies (nil? x) as scheme-style clause.
|
||||||
|
(assert-equal "(p \"yes\")"
|
||||||
|
(render-sx "(cond (nil? nil) (p \"yes\") :else (p \"no\"))"))
|
||||||
|
(assert-equal "(p \"no\")"
|
||||||
|
(render-sx "(cond (nil? \"x\") (p \"yes\") :else (p \"no\"))")))
|
||||||
|
|
||||||
(deftest "let binds then serializes"
|
(deftest "let binds then serializes"
|
||||||
(assert-equal "(p \"hello\")"
|
(assert-equal "(p \"hello\")"
|
||||||
(render-sx "(let ((x \"hello\")) (p x))")))
|
(render-sx "(let ((x \"hello\")) (p x))")))
|
||||||
|
|||||||
@@ -277,6 +277,29 @@
|
|||||||
false "b"
|
false "b"
|
||||||
:else "c")))
|
:else "c")))
|
||||||
|
|
||||||
|
(deftest "cond with 2-element predicate as first test"
|
||||||
|
;; Regression: cond misclassifies Clojure-style as scheme-style when
|
||||||
|
;; the first test is a 2-element list like (nil? x) or (empty? x).
|
||||||
|
;; The evaluator checks: is first arg a 2-element list? If yes, treats
|
||||||
|
;; as scheme-style ((test body) ...) — returning the arg instead of
|
||||||
|
;; evaluating the predicate call.
|
||||||
|
(assert-equal 0 (cond (nil? nil) 0 :else 1))
|
||||||
|
(assert-equal 1 (cond (nil? "x") 0 :else 1))
|
||||||
|
(assert-equal "empty" (cond (empty? (list)) "empty" :else "not-empty"))
|
||||||
|
(assert-equal "not-empty" (cond (empty? (list 1)) "empty" :else "not-empty"))
|
||||||
|
(assert-equal "yes" (cond (not false) "yes" :else "no"))
|
||||||
|
(assert-equal "no" (cond (not true) "yes" :else "no")))
|
||||||
|
|
||||||
|
(deftest "cond with 2-element predicate and no :else"
|
||||||
|
;; Same bug, but without :else — this is the worst case because the
|
||||||
|
;; bootstrapper heuristic also breaks (all clauses are 2-element lists).
|
||||||
|
(assert-equal "found"
|
||||||
|
(cond (nil? nil) "found"
|
||||||
|
(nil? "x") "other"))
|
||||||
|
(assert-equal "b"
|
||||||
|
(cond (nil? "x") "a"
|
||||||
|
(not false) "b")))
|
||||||
|
|
||||||
(deftest "and"
|
(deftest "and"
|
||||||
(assert-true (and true true))
|
(assert-true (and true true))
|
||||||
(assert-false (and true false))
|
(assert-false (and true false))
|
||||||
|
|||||||
@@ -151,6 +151,13 @@
|
|||||||
(assert-equal "<p>hello</p>"
|
(assert-equal "<p>hello</p>"
|
||||||
(render-html "(let ((x \"hello\")) (p x))")))
|
(render-html "(let ((x \"hello\")) (p x))")))
|
||||||
|
|
||||||
|
(deftest "cond with 2-element predicate test"
|
||||||
|
;; Regression: cond misclassifies (nil? x) as scheme-style clause.
|
||||||
|
(assert-equal "<p>yes</p>"
|
||||||
|
(render-html "(cond (nil? nil) (p \"yes\") :else (p \"no\"))"))
|
||||||
|
(assert-equal "<p>no</p>"
|
||||||
|
(render-html "(cond (nil? \"x\") (p \"yes\") :else (p \"no\"))")))
|
||||||
|
|
||||||
(deftest "let preserves outer scope bindings"
|
(deftest "let preserves outer scope bindings"
|
||||||
;; Regression: process-bindings must preserve parent env scope chain.
|
;; Regression: process-bindings must preserve parent env scope chain.
|
||||||
;; Using merge() on Env objects returns empty dict (Env is not dict subclass).
|
;; Using merge() on Env objects returns empty dict (Env is not dict subclass).
|
||||||
|
|||||||
Reference in New Issue
Block a user