Skip to content

Fn arg check#212

Merged
halgari merged 5 commits into
pixie-lang:masterfrom
thomasmulvaney:fn-arg-check
Mar 9, 2015
Merged

Fn arg check#212
halgari merged 5 commits into
pixie-lang:masterfrom
thomasmulvaney:fn-arg-check

Conversation

@thomasmulvaney
Copy link
Copy Markdown
Member

Fixes #211

I wrote tests and some better checks a while ago, but it seems those tests were rather broken. See tests-fns.pxi. Something like (is (thrown? ...)) from clojure.test would be nice.

This also ended up catching some things with extraneous args.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, a missing )!

@pmonks
Copy link
Copy Markdown
Contributor

pmonks commented Mar 5, 2015

Something like (is (thrown? ...)) from clojure.test would be nice.

+1

Also, I was thinking test-fns.pxi should probably have some tests around multi-arity and variable-arity fns i.e. that these argument count validations don't get triggered. wdyt?

Passing an invalid number of args to a MultiArityFn
should throw and error now which includes the name of the
function.
@thomasmulvaney
Copy link
Copy Markdown
Member Author

I've added in some MultiArityFn and VariadicFn runtime exceptions. The messages are probably not the best atm and I had to touch a fair few files. I'm still not that happy with the tests either, but its something for now.

@thomasmulvaney
Copy link
Copy Markdown
Member Author

@pmonks the false positives you mention are exactly what was wrong with my original tests. I've added an assert-throws? macro to pixie.tests to make this stuff work properly.

halgari added a commit that referenced this pull request Mar 9, 2015
@halgari halgari merged commit dea06b1 into pixie-lang:master Mar 9, 2015
@thomasmulvaney thomasmulvaney deleted the fn-arg-check branch March 9, 2015 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add arg count checking for fn calls

3 participants