`nil` => `()`

[akkartik]

Since nobody objected at http://arclanguage.org/item?id=21047, I thought I'd give it a shot, based on @shawwn's https://github.com/arclanguage/anarki/pull/145. Unfortunately, I can't get a few tests to pass:

• @rocketnia's ns.arc.t -- almost everything outside the namespace suite.
• Making http requests using lib/client.arc -- the final test in lib/client.arc.t is failing.

Just thought I'd put my half-baked draft out there in case one of you sees the issue before I do.

[akkartik]

Never mind, I fixed most of the ns.arc issues.

Now there's one failing test in ns.arc.t and one in client.arc.t.

[rocketnia]

I haven't looked at client.arc.t, but here's the failing test in ns.arc.t:

(suite submodule
       (setup foo (simple-rmod t 1 nil 2)
              bar (make-sub-rmodule foo idfn))

       ; ... several other tests omitted ...

       ; We can use 'make-sub-rmodule.
       (test can-use-make-sub-rmodule
             (assert-same '(t) rmodule-keys.bar)))

Instead of returning '(t), rmodule-keys.bar is returning '(nil t). This test is designed to check that the nil variable can be filtered out by using a var-test condition of idfn. If 'nil should be a truthy value now, then the condition should be changed from idfn to [in _ 't] or something.

Some of the other tests in that suite make sure nil is a usable variable name when manipulating namespaces, and those are probably still useful tests to have, especially around the time of a change like this. :)

[rocketnia]

All right, I think I've figured out what's going on with client.arc.t too.

There's this code still in ac.rkt:

(define (ar-bflag key)
  (not (ar-false? (hash-ref (ar-declarations) key 'nil))))

This occurrence of 'nil should probably be replaced with ar-nil now.

Since this default value, 'nil, was truthy, it was causing the explicit-flush declaration to count as being set, which meant disp was not flushing its output port. This meant client.arc's request was never actually sent off to the server, which made it time out, yielding an empty response.

[rocketnia]

Please put an actual commit message on this if you don't mind. This change is pretty historic. :)

[rocketnia]

Oh, it might be good to delete ar-nill at this point. On the official branch it seems to serve only one purpose, namely converting table lookups that would result in #f or '() to 'nil, but it hasn't been used in Anarki for a while and probably should have been deleted a long time ago. It still converts things to 'nil, which now looks like it's not a really meaningful thing to do.

[akkartik]

Oh, definitely on the commit message. I'm fully expecting to edit and force push a few times before I merge.

But you're right that I've committed to this repo with the same message in the past :p Let me switch it to something else. Was intended to remind me to clean up before merge.

[akkartik]

One thing I noticed after creating this PR:

arc> (type '())
'sym

This remains compatible with before, and so may be ok. But I thought I'd call it out.

[akkartik]

@shawwn Why did you use ar-t and ar-nil in https://github.com/arclanguage/anarki/pull/145? I copied that blindly here, but I'm starting to reconsider.

-(define ar-nil '())
-(define ar-t 't)
-(xdef nil ar-nil)
-(xdef t   ar-t)
+(xdef nil '())
+(xdef t   't)

 (define (ar-nil? x)
-  (eqv? x ar-nil))
+  (eqv? x '()))

Seems cleaner?

[akkartik]

@rocketnia Thanks a lot for those tips.

I'm still having trouble with the failing ns.arc.t test. This works (inlining bar which is only used in this test case):

(test can-use-make-sub-rmodule
  (assert-same '(t) (rmodule-keys (make-sub-rmodule foo [is t _])))))

..and so does this:

(test can-use-make-sub-rmodule
  (assert-same '(t) (rmodule-keys (make-sub-rmodule foo testify.t)))))

But none of these work:

(test can-use-make-sub-rmodule
  (assert-same '(t) (rmodule-keys (make-sub-rmodule foo [~is nil _])))))

(test can-use-make-sub-rmodule
  (assert-same '(t) (rmodule-keys (make-sub-rmodule foo [~is '() _])))))

(test can-use-make-sub-rmodule
  (assert-same '(t) (rmodule-keys (make-sub-rmodule foo [no:is nil _])))))

Can you tell why this is? I want to make sure there isn't still some bug in this PR.

[shawwn]

My own opinion is that it would be a mistake to use '() rather than ar-nil throughout the codebase. The justification is that if you use ar-nil any time something should return nil, then you can swap the value of nil with #f or (void) or undefined (from racket/undefined).

This isn't merely hypothetical; (when (is 1 0) 42) should not necessarily return an empty list, whereas (is 1 0) should always return a boolean result.

I say "should" here as in "this is what users almost always want to happen, and results in shorter programs in practice."

It's doubly not-hypothetical because I've subbed out the value of ar-nil with (void) in laarc, which is distinct and separate from ar-false. Although this seems like a complication, the results are good: you can do things like (map [if (even _) _] '(0 1 2 3 4 5 6)) and get back (0 2 4 6) rather than (0 nil 2 nil 4 nil 6). Whereas (map even '(0 1 2 3 4 5 6)) gives '(#t #f #t #f #t #f #t)`.

void is also falsy in laarc, so it's not something you have to worry about most of the time.

That said, if anarki's decision is to stick with the conceptual purity of original lisps, then I agree that using '() directly would be cleaner, as long as you never want to revisit this decision later.

[akkartik]

Makes sense.

With regard to your final example, why not just (keep even _)?

Turning nil into void is incomprehensible to me :) But I see no reason to preclude such experiments. I'll leave these indirections in.

[shawwn]

One very-common pattern in my Arc code was (rem nil (map [if (foo _) (list 1 _ 2)] lst))

Without void you'd have to write (map [list 1 _ 2] (keep foo lst))

Which is fine, but "backwards" visually.

[shawwn]

Of course, I prefer

(accum a
  (each x lst
    (if (foo x) (a:list 1 x 2))))

In pg's original plan, you could write (each x lst (if (even x) (keep x))) and you'd get back a list of even numbers. Iterative programming is clearer and easier to reason about in some situations. I think each should just accum by default, but that leaves the question of what the variable should be named to actually accum a value.

(w/each a x lst (if (even x) (a x))) is just ugly, but (each x lst (if (even x) (a x))) would be too surprising.

[rocketnia]

@akkartik

Since you're making nil and '() distinct from 'nil, none of [~is nil _], [~is '() _], or [no:is nil _] will match the symbol 'nil. The name of the variable to be filtered out here is "nil", so it's a symbol in a meaningul way. If the symbol and the empty list are different values now, then this code is supposed to work with the symbol.

I wrote this suite of tests to make sure they worked even though one variable name, 'nil, was a falsy value. I didn't count on the idea that someday 'nil would be truthy, so this one test needs to be adjusted for that. And in this one, I was just exploiting the fact that 'nil was falsy to make my filtering easy to write, which is why changing the filter idfn to something like [is t _] or [isnt 'nil _] or [in _ 't] would preserve the spirit of the test.

---

@shawwn

Even if (if (even 1) 1) returned #<void>, it surprises me (map [if (even _) _] '(0 1 2 3 4 5 6)) wouldn't return '(0 #<void> 2 #<void> 4 #<void> 6). Surely you want to store void values in lists sometimes, right? Once you have a list like that, do you just never use map on it?

You can write (rem nil (map [if (foo _) (list 1 _ 2)] lst)) as (mappend [if (foo _) (list:list 1 _ 2)] lst). I agree this is a common pattern, and my approach to it has been to use mappend a lot, seemingly even more than I use keep or rem.

This isn't merely hypothetical; (when (is 1 0) 42) should not necessarily return an empty list, whereas (is 1 0) should always return a boolean result.

What is returns should be whatever works with Arc's if. I think there are advantages to this being an empty list: The only ADT Arc's if especially needs to pattern-match against is the list, and booleans and maybes are special cases of lists, so Arc has a few types covered all at once just by defining list operations.

That said, I think there are also advantages in distinguishing maybes and booleans from lists. For instance, that makes it easier to search my code for the places I use lists. So, I think of the type punning as part of Arc, but it's not my favorite part.

I think each should just accum by default

Whoa, I like that idea a lot. An each is only useful if its body performs some kind of effect, and since the each is processing a list, returning another list makes it compositional!

[akkartik]

God, there's another issue I'm not able to debug. When I do this:

 (define (ar-bflag key)
-  (not (ar-false? (hash-ref (ar-declarations) key 'nil))))
+  (not (ar-false? (hash-ref (ar-declarations) key ar-nil))))

I get a failure in test math.addition.cant-add-a-number-to-a-list in lib/tests/core-maths-tests.arc. Before:

arc> (+ '(1 2 3) 4)
car: contract violation
  expected: pair?
  given: 4
  context...:
   /Users/kagaram/play/anarki/ac.rkt:756:0: ar-denil-last
   /Applications/Racket v7.0/collects/racket/private/map.rkt:40:19: loop
   /Users/kagaram/play/anarki/ac.rkt:846:8: +
   /Users/kagaram/play/anarki/ac.rkt:1385:4

After:

arc> (+ '(1 2 3) 4)
(1 2 3 . 4)

So we're going into Racket's append for some reason even though the second argument is not a list. But execution doesn't go into ar-+2 anymore for some reason. And we're getting only one call to ac-call when we run the above, where before we were getting two.

Any guidance or help most appreciated.

[shawwn]

ar-denil-last is on the stack. I thought the point of the PR was no more denil/niltree.

ar-+2 only occurs when (declare 'direct-calls t) is set.

@rocketnia

Surely you want to store void values in lists sometimes, right?

Nope :) In Lua for example you can't store nil in lists. And in fact cons is updated so that if it comes across void it just skips it.

[rocketnia]

Took me a while to find it. There was a second bug which was making that test pass. :)

I was first testing on your branch, where I got a different error than you do in the before case:

arc> (+ '(1 2 3) 4)
+: contract violation
  expected: number?
  given: '(1 2 3)
  argument position: 1st
  other arguments...:
   4
  context...:
   /home/nia/mine/drive/repo/mine/prog/repo/not-mine/anarki/ac.rkt:805:0: ar-+2
   /home/nia/mine/drive/repo/mine/prog/repo/not-mine/anarki/ac.rkt:1326:4

This definitely has something to do with the direct-calls declaration. All declarations are treated as true in the before case, and direct-calls is one of those. When I toggle it using (= declarations!direct-calls t) and (= declarations!direct-calls nil), I toggle between getting this error and getting a result of '(1 2 3 . 4) in the same REPL session.

This only affects binary calls. The call (+ '(1 2 3) '(4 5) 6) works either way, always producing '(1 2 3 4 5 . 6).

Turns out that error happens because ar-+2 only enters the list case if both arguments are lists, so it causes an error in this case, but + enters the list case whenever the first argument is a list, so it gets the Racket append behavior for improper lists. Since ar-+2 is only called if direct-calls is in use, its inconsistent behavior is only visible in that case. Compared to the simple behavior of +, it seems like ar-+2 and the test are in the wrong here.

But on the master branch, what's going on is different: In +, it's not a simple call to append because first the arguments are all converted from Arc lists to Racket lists using ar-denil-last before they're passed in, even the last argument which Racket would happily accept either way. The behavior of ar-denil-last on the master branch raises an error (implicitly) unless its argument is a list that ends with '() or 'nil, so that's where the complaint about an improper list comes from.

To fix the failing test, you could modify the definitions of + and ar-+2 so they explicitly check that the last argument is a proper Racket list? and raise an exception if not. Alternatively, they could add one more empty list to the end so append checks all the arguments. A bit ugly, but it makes the test pass.

Well, except both approaches are only correct if we're sure the Arc empty list will always be '() from here on out. If it's not, then we really need to bring back a function like ar-denil-last to convert whatever ar-nil is to '(), right?

I suppose I'm for this change either way. While I don't understand the full extent of @shawwn's idea for customizing nil, I think the occasional appearance of Racket '() in Arc has always been due to implementation flaws and optimizations rather than some decisive design plan. Making Anarki's empty lists be represented by the same value all the time is a good step toward having more reliable support for other things, such as using Anarki values as Racket hash keys.

[akkartik]

Thanks so much for the detailed explanation! All tests passing now.

[akkartik]

CI is failing with this error after passing all the unit tests, while trying to check the examples:

checking examples interspersed in the codebase
_gacc87268: undefined;
 cannot reference an identifier before its definition
  in module: "/home/travis/build/arclanguage/anarki/ac.rkt"

Works for me fine locally. I'm using Racket 7.0. CI is on Racket 6.11.

[rocketnia]

CI is failing on all four versions it's testing: 6.11, 6.12, 7.0, and (the latest version, obtained automatically) 7.2.

It works locally for me on 7.0 and 6.11. I wonder if it's an environment variable or something.

[rocketnia]

I'm going to make a new branch where I can spam a bunch of commits to try to get Travis working. :D

[rocketnia]

Whoops, finger slipped onto the "close and comment" button.

[rocketnia]

It looks like commenting out the lines that load lib/collect.arc cause the tests to pass: https://travis-ci.org/arclanguage/anarki/builds/487644932

Now I'm trying to debug what's going on in collect.arc.

[rocketnia]

Very bizarre. Two of the examples in collect.arc work, and then a very similar one fails:

checking examples interspersed in the codebase
blah collect expansion result:
(accum gacc-from-collect86303 (up i 1 3 (gacc-from-collect86303 i)))
blah beginning collect example 1
blah collect expansion result:
(accum gacc-from-collect86350 (up i 1 3 (gacc-from-collect86350 (* i 2))))
blah beginning collect example 2
blah collect expansion result:
(accum gacc-from-collect86397 (up i 1 3 (up j 1 3 (if (< i j) (gacc-from-collect86397 (list i j))))))
blah beginning collect example 3
_gacc-from-collect86397: undefined;
 cannot reference an identifier before its definition
  in module: "/home/travis/build/arclanguage/anarki/ac.rkt"
The command "./arc.sh -n build-web-help.arc" exited with 1.

I suppose it might have something to do with compiling if. But why would that be different on Travis CI than on our local copies?

I notice this gacc-from-collect86397 is being compiled as a top-level variable (getting that underscore added to it), so the lexical environment passed around at expansion time might be getting messed up somewhere.

[rocketnia]

Huh. Adding the -l errortrace option to the command line may make Anarki terribly slow, but it gives us a surprisingly informative stack trace. I'm currently testing with a few more collect examples added in and with a bunch of debug printing, and you can see the stack trace I get:

checking examples interspersed in the codebase
blah collect expansion result:
(accum gacc-from-collect18834 (up i 1 3 (gacc-from-collect18834 i)))
blah compiled collect example 1, now executing it
declarations* = (tagged table ((atstrings t)))
blah collect expansion result:
(accum gacc-from-collect18846 (up i 1 3 (gacc-from-collect18846 (* i 2))))
blah compiled collect example 2, now executing it
declarations* = (tagged table ((atstrings t)))
blah collect expansion result:
(accum gacc-from-collect18858 (up i 1 3 (if (< i 3) (gacc-from-collect18858 (* i 2)))))
blah compiled collect example 3 (created for debugging), now executing it
declarations* = (tagged table ((atstrings t)))
blah collect expansion result:
(accum gacc-from-collect18870 (up i 1 3 (up j 1 3 (gacc-from-collect18870 (* i 2)))))
blah compiled collect example 4 (created for debugging), now executing it
declarations* = (tagged table ((atstrings t)))
blah collect expansion result:
(accum gacc-from-collect18889 (up i 1 3 (up j 1 3 (gacc-from-collect18889 (list i j)))))
blah compiled collect example 5 (created for debugging), now executing it
declarations* = (tagged table ((atstrings t)))
blah collect expansion result:
(accum gacc-from-collect18908 (up i 1 3 (up j 1 3 (if (< i j) (gacc-from-collect18908 (list i j))))))
blah compiled collect example 6 (duplicate of what was originally 3), now executing it
declarations* = (tagged table ((atstrings t)))
blah collect expansion result:
(accum gacc-from-collect18927 (up i 1 3 (up j 1 3 (if (< i j) (gacc-from-collect18927 (list i j))))))
blah compiled collect example 7 (originally 3), now executing it
declarations* = (tagged table ((atstrings t)))
_gacc-from-collect18858: undefined;
 cannot reference an identifier before its definition
  in module: "/home/travis/build/arclanguage/anarki/ac.rkt"
  errortrace...:
   /home/travis/racket/collects/racket/private/kw.rkt:1133:25: (ar-coerce _gacc-from-collect18858 (quote fn))
   /home/travis/racket/collects/racket/private/kw.rkt:1133:25: ((ar-coerce _gacc-from-collect18858 (quote fn)) ((ar-coerce _* (quote fn)) i (quote 2)))
   /home/travis/racket/collects/racket/private/kw.rkt:1133:25: ((ar-coerce _call/ec (quote fn)) (let-values (((| recur|) (lambda (g19062) (....)))) | recur|))
   /home/travis/racket/collects/racket/private/kw.rkt:1133:25: ((let-values (((| recur|) (lambda (g19061) ((....) ....) (....) ....))) | recur|) (quote ()))
   /home/travis/racket/collects/racket/private/kw.rkt:1133:25: ((ar-coerce _call/ec (quote fn)) (lambda (g19059) ((lambda (break) ((....) ....)) (lambda g19065 (....)))))
   /home/travis/racket/collects/racket/private/kw.rkt:1133:25: ((lambda () ((ar-coerce _call/ec (quote fn)) (lambda (g19059) ((....) ....))) ((ar-coerce _rev (quote ....)) gacc-from-accum19058)))
   /home/travis/racket/collects/racket/private/kw.rkt:1133:25: ((ar-coerce _eval (quote fn)) expr)
   /home/travis/racket/collects/racket/private/kw.rkt:1133:25: ((ar-coerce _iso (quote fn)) expected ((ar-coerce _eval (quote fn)) expr))
   /home/travis/racket/collects/racket/private/kw.rkt:1133:25: ((ar-coerce _no (quote fn)) ((ar-coerce _iso (quote fn)) expected ((ar-coerce _eval (quote ....)) expr)))
   /home/travis/racket/collects/racket/private/kw.rkt:1133:25: (ar-false? ((ar-coerce _no (quote fn)) ((ar-coerce _iso (quote fn)) expected ((ar-coerce (....) ....) expr))))
   /home/travis/racket/collects/racket/private/kw.rkt:1133:25: (not (ar-false? ((ar-coerce _no (quote fn)) ((ar-coerce _iso (....)) expected ((....) ....)))))
   /home/travis/racket/collects/racket/private/kw.rkt:1133:25: ((ar-coerce _print-example-session (quote fn)) expr expected)
   /home/travis/racket/collects/racket/private/kw.rkt:1133:25: ((let-values (((| recur|) (lambda () ((....) ....) (....)))) | recur|))
   /home/travis/racket/collects/racket/private/kw.rkt:1133:25: ((ar-coerce _protect (quote fn)) (let-values (((| helpstr|) (lambda () (....)))) | helpstr|) (let-values (((| helpstr|) (lambda () ....))) | helpstr|))
   /home/travis/racket/collects/racket/private/kw.rkt:1133:25: ((let-values (((| helpstr|) (lambda (gv1950) ((....) ....) (....)))) | helpstr|) ((ar-coerce _outstring (quote fn))))
   /home/travis/racket/collects/racket/private/kw.rkt:1133:25: ((let-values (((| display-web-help-section|) (lambda (doc) ((....) ....) (....) ....))) | display-web-help-section|) ((ar-coerce _helpstr (quote fn)) name))
   /home/travis/racket/collects/racket/private/kw.rkt:1133:25: ((let-values (((| display-web-help-section|) (lambda () ((....) ....) (....) ....))) | display-web-help-section|))
   /home/travis/racket/collects/racket/private/kw.rkt:1133:25: ((let-values (((| recur|) (lambda () ((....) ....) (....)))) | recur|))
   /home/travis/racket/collects/racket/private/kw.rkt:1133:25: ((let-values (((| display-web-help-section|) (lambda () ((....) ....) (....) ....))) | display-web-help-section|))
   /home/travis/racket/collects/racket/private/kw.rkt:1133:25: ((lambda () ((ar-coerce _pr (quote fn)) (string-copy (quote "<body>"))) ((lambda () ((....) ....) (....) ....)) ((lambda () (....) ....)) ((lambda (....) ....) ((....)) (....) ....) ((ar-coerce ....) (....))))
   /home/travis/racket/collects/racket/private/kw.rkt:1133:25: ((lambda () ((ar-coerce _pr (quote fn)) (string-copy (quote "<head>"))) ((ar-coerce _pr (quote ....)) (string-copy (quote ....))) ((lambda () (....) ....)) ((lambda () ....)) ((lambda ....)) ((....) ....)))
   /home/travis/racket/collects/racket/private/kw.rkt:1133:25: ((lambda () ((ar-coerce _pr (quote fn)) (string-copy (quote "<html lang=\"en\">"))) ((lambda () ((....) ....) (....) ....)) ((ar-coerce _pr (....)) (string-copy (....)))))
   /home/travis/racket/collects/racket/private/kw.rkt:1133:25: ((ar-coerce _protect (quote fn)) (lambda () ((lambda () ((....) ....)))) (lambda () ((ar-coerce _close (....)) out)))
   string::41: (aload-with-main-settings (vector-ref (current-command-line-arguments) 0))
The command "./arc.sh -n build-web-help.arc" exited with 1.

Lol, no wonder we're not experiencing it locally! Look at that | display-web-help-section|. It's not happening during tests.arc; somehow it's happening during build-web-help.arc.

[rocketnia]

I figured out the source of the problem here. Since you don't need to traverse some lists to convert 'nil to '() or back, you're not copying them as often anymore. The collect macro in collect.arc performs mutation on its argument list, and now that means it modifies the s-expression stored in the examples* collection. When help.arc tries to render help text, it evaluates the example code to see which examples might be out of date. This means it evaluates an s-expression that's been slightly modified from the original.

Here's the before and after for one of the extra examples I put in for debugging, which can be seen in the Travis CI log:

(collect (* i 2) (for i from 1 to 3) (if (< i 3)))
(collect (* i 2) (for i from 1 to 3) (if (< i 3) (gacc-from-collect18565 (* i 2))))

The (for ...) list isn't modified because collect treats it as a sugar for (up ...). It replaces it with an (up ...) list and then mutates that one. Since (if ...) isn't one of these special cases, collect mutates it directly.

I'm not sure what part we should think of as broken here. I suppose I don't think the collect macro should mutate its arguments, but Arc provides mutable cons cells, so I don't think it was going against the grain of the language to do so.

I suppose the question is:

• should macros be able to rely on their arguments being defensively copied so their mutations will be discarded when they're done, or

• should macros be able to rely on their arguments not being defensively copied so they can use mutations of syntax to cause an impact in future evaluations of an expression?

[akkartik]

Whoa. I'm glad I resisted the temptation to merge this change. Thanks a lot for this tour de force of debugging, @rocketnia.

[rocketnia]

One thing I learned from this stretch of debugging is that I would have saved us both a lot of confusion if only I had ever put a single print line in build-web-help.arc that said, "Now generating the HTML help documentation..." 😅

[shawwn]

I think each should just accum by default

Whoa, I like that idea a lot. An each is only useful if its body performs some kind of effect, and since the each is processing a list, returning another list makes it compositional!

@rocketnia @akkartik See PR #152 for an attempt at this.

[akkartik]

@rocketnia I was somehow entirely oblivious to build-web-help.arc. Maybe make the page you're generating more discoverable? Not obvious to me where the docstrings go, since they're not at http://arclanguage.github.io/anarki. I also tried http://arclanguage.github.io/anarki/site/help/index.html but that isn't it either.

[rocketnia]

@akkartik I'm not saying discoverability couldn't be way better, but it's at https://arclanguage.github.io/anarki/help/, as linked to by the "reference documentation" link at https://arclanguage.github.io/. You can read more about it here: http://arclanguage.org/item?id=20595

[akkartik]

Ah, that makes sense now. Thank you!

[akkartik]

@rocketnia I'm trying to make detailed sense of your debugging comment above, and not quite there. If I understand correctly, build-web-help.arc is running tests.arc and then inspecting the code that ran in the examples phase somehow. But I don't see examples* being used anywhere? I also thought for a while that maybe we were running tests and build-web-help.arc in the same Arc session. But that's not true in .travis.yml, and I can also confirm that I can run the tests multiple times without issue.

Were the examples for collect rendering incorrectly at http://arclanguage.github.io/anarki/help, so that the code in the examples wasn't quite what was in the sources? I can't see that anymore since your latest commit :) However, when I run arc.sh -n build-web-help.arc locally without this commit I see no errors. Could you point out how I can reproduce the issue locally? Thanks a lot.

[rocketnia]

@akkartik

If I understand correctly, build-web-help.arc is running tests.arc and then inspecting the code that ran in the examples phase somehow. But I don't see examples* being used anywhere?

The build-web-help.arc file uses the function helpstr, which is defined in lib/help.arc. That function uses examples*.

Before I introduced build-web-help.arc, the result of helpstr was only used to generate documentation when calling help at the REPL. So I think this would have still been broken, but nobody would have noticed until someone was at the REPL and decided to write:

arc> help.collect

Were the examples for collect rendering incorrectly at http://arclanguage.github.io/anarki/help, so that the code in the examples wasn't quite what was in the sources?

No, for a couple of reasons:

• The configuration in .travis.yml is set up to only run deploy-to-gh-pages.sh if the build was successful (which these builds weren't).

• The code in deploy-to-gh-pages.sh explicitly checks that it was not called by a pull request build before it deploys to the gh-pages branch. I think the GitHub pages site should reflect what's actually on the Anarki master branch, not just whatever pull request to master has been built most recently.

I can't see that anymore since your latest commit :) However, when I run arc.sh -n build-web-help.arc locally without this commit I see no errors.

Hmm, you're on the drop-nil branch when you run arc.sh -n build-web-help.arc and it's working?

[rocketnia]

If you had looked at the built build-gh-pages/site/help/index.html file after an unsuccessful build, I'm not sure what it would look like. I suspect it would contain all the help entries up to (but not including) collect, plus maybe some gobbeldygook that was printed when the error occurred.

[akkartik]

You're right, I think I wasn't on the drop-nil branch before :/ The following both cause errors now:

$ ./arc.sh -n build-web-help.arc

and

$ ./arc.sh
arc> (load "tests.arc")  ; error
arc> (help collect)  ; error

Thanks!

I think the cleanest way to fix the issue is in tests.arc:

 (prn "checking examples interspersed in the codebase")
 (each (name examples-and-expected) examples*
-  (each (example expected) pair.examples-and-expected
+  (each (example expected) (pair copy.examples-and-expected)
     (when (and (~is expected '_)
                (~iso eval.example expected)
                (~and (caris expected 'valueof)

We should now also be able to support mutation in macros. #154 is no longer necessary after this change. (Though it's a more elegant implementation by far and worth keeping.)

The more general lesson for me is that our framework for managing examples has some rough edges :) Where we run into more of those we should mould our examples* harness rather than impose constraints on macros people can write.

[akkartik]

Ok, rerunning CI after fixing in master and rebasing from it. Let's see how this goes.

[akkartik]

Woohoo, all checks passed!

[akkartik]

I just found one issue with this change that was anticipated by @rocketnia at https://github.com/arclanguage/anarki/pull/148#issuecomment-459378648:

arc> (case nil nil 34 35)
35  # should be 34 since nil == nil

This is because:

arc> (macex '(case nil nil 34 35))
'((fn (g2521) (if (is g2521 'nil) 34 35)) nil)

In other words, case is performing (is nil 'nil) here.

One way out would be to stop quoting cases in case, something I've always found hacky. But I'm probably missing other implications.

(Thanks @Kinnardian for catalyzing this realization.)

[rocketnia]

@akkartik I would've expected expressions like (case ... nil ...) to be refactored to various things like (case ... () ...), (if (no ...) ...), or (if (in ... 'nil '()) ...) depending on their intent. Some may even be kept as-is if their intent was to match the symbol named "nil".

If you want a case without quoting, doesn't Anarki have that as switch? That makes (switch ... nil ...) an alternative to (case ... () ...).