From 71174a5c248b91f1dbada1f028adfb1332b3561c Mon Sep 17 00:00:00 2001 From: Thomas Mulvaney Date: Wed, 4 Mar 2015 23:53:23 +0000 Subject: [PATCH 1/5] Fixes Issue #211 Turns out my tests were rather poor... --- pixie/vm/code.py | 9 +++++++-- pixie/vm/interpreter.py | 8 ++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/pixie/vm/code.py b/pixie/vm/code.py index 0ec2344b..729ec4ea 100644 --- a/pixie/vm/code.py +++ b/pixie/vm/code.py @@ -1,6 +1,6 @@ py_object = object import pixie.vm.object as object -from pixie.vm.object import affirm +from pixie.vm.object import affirm, runtime_error from pixie.vm.primitives import nil, false from rpython.rlib.rarithmetic import r_uint from rpython.rlib.jit import elidable_promote, promote @@ -209,7 +209,12 @@ def get_debug_points(self): return self._debug_points def invoke(self, args): - return self.invoke_with(args, self) + if len(args) == self.get_arity(): + return self.invoke_with(args, self) + else: + runtime_error(u"Invalid number of arguments " + unicode(len(args)) + + u" for function '" + unicode(str(self._name)) + u"'. Expected " + + unicode(str(self.get_arity()))) def invoke_with(self, args, this_fn): try: diff --git a/pixie/vm/interpreter.py b/pixie/vm/interpreter.py index 7cc5da9e..9e80107a 100644 --- a/pixie/vm/interpreter.py +++ b/pixie/vm/interpreter.py @@ -98,12 +98,8 @@ def push_nth(self, delta): self.push(self.nth(delta)) def push_arg(self, idx): - if not 0 <= idx < len(self.args): - runtime_error(u"Invalid number of arguments " + unicode(str(idx)) - + u" for function '" + unicode(str(self.code_obj._name)) + u"'. Expected " - + unicode(str(self.code_obj.get_arity()))) - - self.push(self.args[r_uint(idx)]) + if 0 <= idx < len(self.args): + self.push(self.args[r_uint(idx)]) @unroll_safe def push_n(self, args, argc): From 2fcf979c380b07192c6f0d0c8192fbc442d2eb8c Mon Sep 17 00:00:00 2001 From: Thomas Mulvaney Date: Wed, 4 Mar 2015 23:55:15 +0000 Subject: [PATCH 2/5] Runtime arg error caught some stuff... more --- tests/pixie/tests/test-stdlib.pxi | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/pixie/tests/test-stdlib.pxi b/tests/pixie/tests/test-stdlib.pxi index 2439d837..c0c0b711 100644 --- a/tests/pixie/tests/test-stdlib.pxi +++ b/tests/pixie/tests/test-stdlib.pxi @@ -311,9 +311,9 @@ (t/assert= (merge-with identity {} {:a 1, :b 2}) {:a 1, :b 2}) (t/assert= (merge-with identity {:a 1} {:b 2}) {:a 1, :b 2}) - (t/assert= (merge-with #(identity %1) {:a 1} {:a 2}) {:a 1}) - (t/assert= (merge-with #(identity %1) {:a 1} {:a 2} {:a 3}) {:a 1}) - (t/assert= (merge-with #(identity %2) {:a 1} {:a 2}) {:a 2}) + (t/assert= (merge-with (fn [a b] a) {:a 1} {:a 2}) {:a 1}) + (t/assert= (merge-with (fn [a b] a) {:a 1} {:a 2} {:a 3}) {:a 1}) + (t/assert= (merge-with (fn [a b] b) {:a 1} {:a 2}) {:a 2}) (t/assert= (merge-with + {:a 21} {:a 21}) {:a 42}) (t/assert= (merge-with + {:a 21} {:a 21, :b 1}) {:a 42, :b 1})) @@ -333,9 +333,8 @@ (t/deftest test-range (t/assert= (= (-seq (range 10)) - (-seq (-iterator (range 10)) - (reduce conj nil (range 10)) - '(0 1 2 3 4 5 6 7 8 9))) + (-seq (-iterator (range 10))) + '(0 1 2 3 4 5 6 7 8 9)) true)) (t/deftest test-ns From 4d0d836f620898c5eb387d553f72938adba147dc Mon Sep 17 00:00:00 2001 From: Thomas Mulvaney Date: Thu, 5 Mar 2015 09:23:43 +0000 Subject: [PATCH 3/5] Sets the _name in BaseCode for MultiArityFns Passing an invalid number of args to a MultiArityFn should throw and error now which includes the name of the function. --- pixie/vm/code.py | 9 +++++---- pixie/vm/compiler.py | 2 +- pixie/vm/interpreter.py | 3 +-- pixie/vm/libs/string.py | 4 ++-- tests/pixie/tests/test-fns.pxi | 3 ++- 5 files changed, 11 insertions(+), 10 deletions(-) diff --git a/pixie/vm/code.py b/pixie/vm/code.py index 729ec4ea..7b87df5c 100644 --- a/pixie/vm/code.py +++ b/pixie/vm/code.py @@ -130,15 +130,16 @@ class MultiArityFn(BaseCode): def type(self): return MultiArityFn._type - def __init__(self, arities, required_arity=0, rest_fn=None, meta=nil): + def __init__(self, name, arities, required_arity=0, rest_fn=None, meta=nil): BaseCode.__init__(self) + self._name = name self._arities = arities self._required_arity = required_arity self._rest_fn = rest_fn self._meta = meta def with_meta(self, meta): - return MultiArityFn(self._arities, self._required_arity, self._rest_fn, meta) + return MultiArityFn(self._name, self._arities, self._required_arity, self._rest_fn, meta) @elidable_promote() def get_fn(self, arity): @@ -155,7 +156,7 @@ def get_fn(self, arity): if self._rest_fn: acc.append(u" or more") - affirm(False, u"Wrong number of args to fn: got " + unicode(str(arity)) + u" expected " + u",".join(acc)) + runtime_error(u"Wrong number of args to fn " + unicode(self._name) + " got " + unicode(str(arity)) + u" expected " + u",".join(acc)) def invoke(self, args): return self.invoke_with(args, self) @@ -212,7 +213,7 @@ def invoke(self, args): if len(args) == self.get_arity(): return self.invoke_with(args, self) else: - runtime_error(u"Invalid number of arguments " + unicode(len(args)) + runtime_error(u"Invalid number of arguments " + unicode(str(len(args))) + u" for function '" + unicode(str(self._name)) + u"'. Expected " + unicode(str(self.get_arity()))) diff --git a/pixie/vm/compiler.py b/pixie/vm/compiler.py index dc36e169..afd06a1a 100644 --- a/pixie/vm/compiler.py +++ b/pixie/vm/compiler.py @@ -33,7 +33,7 @@ def gensym2(prefix): return rt.symbol(rt.str(prefix, i)) gensym = code.intern_var(u"pixie.stdlib", u"gensym") -gensym.set_root(code.MultiArityFn({0: code.wrap_fn(gensym1), 1: code.wrap_fn(gensym2)})) +gensym.set_root(code.MultiArityFn(u"gensym", {0: code.wrap_fn(gensym1), 1: code.wrap_fn(gensym2)})) class with_ns(object): def __init__(self, nm, include_stdlib=False): diff --git a/pixie/vm/interpreter.py b/pixie/vm/interpreter.py index 9e80107a..a424e785 100644 --- a/pixie/vm/interpreter.py +++ b/pixie/vm/interpreter.py @@ -145,8 +145,7 @@ def make_multi_arity(frame, argc): else: fn = frame.pop() d[a] = fn - - return code.MultiArityFn(d, required_arity, rest_fn) + return code.MultiArityFn(fn._name, d, required_arity, rest_fn) class ShallowContinuation(Object): _type = Type(u"pixie.stdlib.ShallowContinuation") diff --git a/pixie/vm/libs/string.py b/pixie/vm/libs/string.py index 13977765..967046c8 100644 --- a/pixie/vm/libs/string.py +++ b/pixie/vm/libs/string.py @@ -47,7 +47,7 @@ def index_of4(a, sep, start, end): runtime_error(u"Third and fourth argument must be non-negative integers") index_of = intern_var(u"pixie.string.internal", u"index-of") -index_of.set_root(MultiArityFn({2: wrap_fn(index_of2), 3: wrap_fn(index_of3), 4: wrap_fn(index_of4)}, +index_of.set_root(MultiArityFn(u"index-of", {2: wrap_fn(index_of2), 3: wrap_fn(index_of3), 4: wrap_fn(index_of4)}, required_arity = 2)) def substring2(a, start): @@ -64,7 +64,7 @@ def substring3(a, start, end): runtime_error(u"Second and third argument must be non-negative integers") substring = intern_var(u"pixie.string.internal", u"substring") -substring.set_root(MultiArityFn({2: wrap_fn(substring2), 3: wrap_fn(substring3)}, +substring.set_root(MultiArityFn(u"substring", {2: wrap_fn(substring2), 3: wrap_fn(substring3)}, required_arity = 2)) @as_var("pixie.string.internal", "upper-case") diff --git a/tests/pixie/tests/test-fns.pxi b/tests/pixie/tests/test-fns.pxi index 3f736858..a42bf20c 100644 --- a/tests/pixie/tests/test-fns.pxi +++ b/tests/pixie/tests/test-fns.pxi @@ -18,7 +18,8 @@ (t/deftest test-code-arity-errors (let [arity-0 (fn arity-0 []) arity-1 (fn arity-1 [a]) - arity-2 (fn arity-2 [a b])] + arity-2 (fn arity-2 [a b]) + multi-arity (fn arity-0-or-1)] (try (arity-0 :foo) (catch e From 757c5a246702042a81d1141ff4baaff278ea983e Mon Sep 17 00:00:00 2001 From: Thomas Mulvaney Date: Thu, 5 Mar 2015 09:46:10 +0000 Subject: [PATCH 4/5] Proper errors for MultiArityFns with VariadicFns --- pixie/vm/code.py | 17 +++++++++++-- pixie/vm/interpreter.py | 4 +++- tests/pixie/tests/test-fns.pxi | 44 ++++++++++++++++++++++++++++++++-- 3 files changed, 60 insertions(+), 5 deletions(-) diff --git a/pixie/vm/code.py b/pixie/vm/code.py index 7b87df5c..ed17035f 100644 --- a/pixie/vm/code.py +++ b/pixie/vm/code.py @@ -101,6 +101,9 @@ def meta(self): def with_meta(self, meta): assert false, "not implemented" + def name(self): + return self._name + def set_macro(self): self._is_macro = True @@ -154,9 +157,9 @@ def get_fn(self, arity): acc.append(unicode(str(x))) if self._rest_fn: - acc.append(u" or more") + acc.append(unicode(str(self._rest_fn.required_arity())) + u" or more") - runtime_error(u"Wrong number of args to fn " + unicode(self._name) + " got " + unicode(str(arity)) + u" expected " + u",".join(acc)) + runtime_error(u"Wrong number of arguments " + unicode(str(arity)) + u" for function '" + unicode(self._name) + u"'. Expected " + u",".join(acc)) def invoke(self, args): return self.invoke_with(args, self) @@ -260,6 +263,12 @@ def __init__(self, code, required_arity, meta=nil): def with_meta(self, meta): return VariadicCode(self._code, self._required_arity, meta) + + def name(self): + return None + + def required_arity(self): + return self._required_arity def invoke(self, args): return self.invoke_with(args, self) @@ -298,6 +307,10 @@ def __init__(self, code, closed_overs, meta=nil): def with_meta(self, meta): return Closure(self._code, self._closed_overs, meta) + + def name(self): + return None + def invoke(self, args): return self.invoke_with(args, self) diff --git a/pixie/vm/interpreter.py b/pixie/vm/interpreter.py index a424e785..bbc96324 100644 --- a/pixie/vm/interpreter.py +++ b/pixie/vm/interpreter.py @@ -136,6 +136,7 @@ def make_multi_arity(frame, argc): d = {} required_arity = 0 rest_fn = None + fn_name = None for i in range(argc): a = frame.get_inst() if a & 256: @@ -144,8 +145,9 @@ def make_multi_arity(frame, argc): rest_fn = frame.pop() else: fn = frame.pop() + fn_name = fn.name() d[a] = fn - return code.MultiArityFn(fn._name, d, required_arity, rest_fn) + return code.MultiArityFn(fn_name, d, required_arity, rest_fn) class ShallowContinuation(Object): _type = Type(u"pixie.stdlib.ShallowContinuation") diff --git a/tests/pixie/tests/test-fns.pxi b/tests/pixie/tests/test-fns.pxi index a42bf20c..b5f2e8e2 100644 --- a/tests/pixie/tests/test-fns.pxi +++ b/tests/pixie/tests/test-fns.pxi @@ -19,7 +19,10 @@ (let [arity-0 (fn arity-0 []) arity-1 (fn arity-1 [a]) arity-2 (fn arity-2 [a b]) - multi-arity (fn arity-0-or-1)] + arity-0-or-1 (fn arity-0-or-1 ([]) ([a])) + arity-1-or-3 (fn arity-1-or-3 ([a]) ([a b c])) + arity-0-or-1-or-3-or-more + (fn arity-0-or-1-or-3-or-more ([]) ([a]) ([a b c & more]))] (try (arity-0 :foo) (catch e @@ -55,4 +58,41 @@ (catch e (t/assert= (ex-msg e) - "Invalid number of arguments 1 for function 'arity-2'. Expected 2"))))) + "Invalid number of arguments 1 for function 'arity-2'. Expected 2"))) + + (try + (arity-0-or-1 :foo :bar) + (catch e + (t/assert= + (ex-msg e) + "Wrong number of arguments 2 for function 'arity-0-or-1'. Expected 1,0"))) + + (try + (arity-0-or-1 :foo :bar :baz) + (catch e + (t/assert= + (ex-msg e) + "Wrong number of arguments 3 for function 'arity-0-or-1'. Expected 1,0"))) + + (try + (arity-1-or-3 :foo :bar) + (catch e + (t/assert= + (ex-msg e) + "Wrong number of arguments 2 for function 'arity-1-or-3'. Expected 3,1"))) + + (try + (arity-1-or-3) + (catch e + (t/assert= + (ex-msg e) + "Wrong number of arguments 0 for function 'arity-1-or-3'. Expected 3,1"))) + + (try + (arity-0-or-1-or-3-or-more) + (catch e + (t/assert= + (ex-msg e) + "Wrong number of arguments 0 for function 'arity-0-or-1-or-3-or-more'. Expected 0, 1, 3 or more"))) + + )) From 7191e20433cbb8ab5a234c1999af03723976c806 Mon Sep 17 00:00:00 2001 From: Thomas Mulvaney Date: Thu, 5 Mar 2015 21:37:02 +0000 Subject: [PATCH 5/5] add a assert-throws? function --- pixie/test.pxi | 12 ++++ tests/pixie/tests/test-fns.pxi | 106 ++++++++++----------------------- 2 files changed, 45 insertions(+), 73 deletions(-) diff --git a/pixie/test.pxi b/pixie/test.pxi index 41c14914..cf77ccf2 100644 --- a/pixie/test.pxi +++ b/pixie/test.pxi @@ -61,10 +61,22 @@ yr# ~y] (assert (= xr# yr#) (str (show '~x xr#) " != " (show '~y yr#))))) +(defmacro assert-throws? [klass msg body] + `(try + ~body + (assert false (str "Expected a " ~klass " exception: " ~msg)) + (catch e# + (assert (= (type e#) ~klass) + (str "Expected exception of class " ~klass " but got " (type e#))) + (assert (= (ex-msg e#) ~msg) + (str "Expected message: " ~msg " but got " (ex-msg e#)))))) + (defmacro assert [x] `(let [x# ~x] (assert x# (str '~x " is " x#)))) + + (defn show ([orig res] (if (= orig res) diff --git a/tests/pixie/tests/test-fns.pxi b/tests/pixie/tests/test-fns.pxi index b5f2e8e2..12f74bc9 100644 --- a/tests/pixie/tests/test-fns.pxi +++ b/tests/pixie/tests/test-fns.pxi @@ -23,76 +23,36 @@ arity-1-or-3 (fn arity-1-or-3 ([a]) ([a b c])) arity-0-or-1-or-3-or-more (fn arity-0-or-1-or-3-or-more ([]) ([a]) ([a b c & more]))] - (try - (arity-0 :foo) - (catch e - (t/assert= - (ex-msg e) - "Invalid number of arguments 1 for function 'arity-0'. Expected 0"))) - (try - (arity-0 :foo :bar) - (catch e - (t/assert= - (ex-msg e) - "Invalid number of arguments 2 for function 'arity-0'. Expected 0"))) - (try - (arity-1) - (catch e - (t/assert= - (ex-msg e) - "Invalid number of arguments 0 for function 'arity-1'. Expected 1"))) - (try - (arity-1 :foo :bar) - (catch e - (t/assert= - (ex-msg e) - "Invalid number of arguments 2 for function 'arity-1'. Expected 1"))) - (try - (arity-2) - (catch e - (t/assert= - (ex-msg e) - "Invalid number of arguments 0 for function 'arity-2'. Expected 2"))) - (try - (arity-2 :foo) - (catch e - (t/assert= - (ex-msg e) - "Invalid number of arguments 1 for function 'arity-2'. Expected 2"))) - - (try - (arity-0-or-1 :foo :bar) - (catch e - (t/assert= - (ex-msg e) - "Wrong number of arguments 2 for function 'arity-0-or-1'. Expected 1,0"))) - - (try - (arity-0-or-1 :foo :bar :baz) - (catch e - (t/assert= - (ex-msg e) - "Wrong number of arguments 3 for function 'arity-0-or-1'. Expected 1,0"))) - - (try - (arity-1-or-3 :foo :bar) - (catch e - (t/assert= - (ex-msg e) - "Wrong number of arguments 2 for function 'arity-1-or-3'. Expected 3,1"))) - - (try - (arity-1-or-3) - (catch e - (t/assert= - (ex-msg e) - "Wrong number of arguments 0 for function 'arity-1-or-3'. Expected 3,1"))) - - (try - (arity-0-or-1-or-3-or-more) - (catch e - (t/assert= - (ex-msg e) - "Wrong number of arguments 0 for function 'arity-0-or-1-or-3-or-more'. Expected 0, 1, 3 or more"))) - - )) + (t/assert-throws? RuntimeException + "Invalid number of arguments 1 for function 'arity-0'. Expected 0" + (arity-0 :foo)) + (t/assert-throws? RuntimeException + "Invalid number of arguments 2 for function 'arity-0'. Expected 0" + (arity-0 :foo :bar)) + (t/assert-throws? RuntimeException + "Invalid number of arguments 0 for function 'arity-1'. Expected 1" + (arity-1)) + (t/assert-throws? RuntimeException + "Invalid number of arguments 2 for function 'arity-1'. Expected 1" + (arity-1 :foo :bar)) + (t/assert-throws? RuntimeException + "Invalid number of arguments 0 for function 'arity-2'. Expected 2" + (arity-2)) + (t/assert-throws? RuntimeException + "Invalid number of arguments 1 for function 'arity-2'. Expected 2" + (arity-2 :foo)) + (t/assert-throws? RuntimeException + "Wrong number of arguments 2 for function 'arity-0-or-1'. Expected 1,0" + (arity-0-or-1 :foo :bar)) + (t/assert-throws? RuntimeException + "Wrong number of arguments 3 for function 'arity-0-or-1'. Expected 1,0" + (arity-0-or-1 :foo :bar :baz)) + (t/assert-throws? RuntimeException + "Wrong number of arguments 2 for function 'arity-1-or-3'. Expected 3,1" + (arity-1-or-3 :foo :bar)) + (t/assert-throws? RuntimeException + "Wrong number of arguments 0 for function 'arity-1-or-3'. Expected 3,1" + (arity-1-or-3)) + (t/assert-throws? RuntimeException + "Wrong number of arguments 2 for function 'arity-0-or-1-or-3-or-more'. Expected 1,0,3 or more" + (arity-0-or-1-or-3-or-more :foo :bar))))