Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
A first PR for a standard lib #30
base: master
Are you sure you want to change the base?
A first PR for a standard lib #30
Changes from 4 commits
919235d
6896de5
9ee3f53
1c90205
d778b11
3e82b23
bf7ee54
7aad918
08262cc
ffbfb3d
edd9b56
4fffea8
0db9e06
7e2d391
2ca2dc9
a9ad888
8b6daf0
bda6942
9241d45
a262a78
771cf88
926b318
1478aa2
59925b5
7e98dfd
3249f61
6da7793
122bbc1
1374831
c4b2312
cb58bef
e7ad840
e9ff2c3
e996b11
4895eb9
2fab48c
37936e3
208f72f
88b1653
abe766d
5aafb28
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LIST probably requires a macro or language support to be done right (variable number of arguments).
Explicit use of EVAL is probably not desirable, but if it were, you probably want the current env to be involved.
I'm chalking this up to probably not being a useful function as written.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I came up with that one, and i had a newer version that uses current-env, so that you can do nested lists.
I don't see any other way to produce a literal list (that's not quoted, but rather requires eval'ing args). I mean, aside from a bunch of consing. Obviously using
eval
is a terrible hack and yeslist
is supposed to be varargs. Edit: but i think it's useful until there's a nativelist
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a clue that the concept involved is off. If we really want a function that takes a list and outputs a list, we can just use the identity function.
Normal Lisp LIST, apart from being variadic, doesn't perform explicit evaluation. It just so happens that the args are evaluated before LIST ever sees them.
For example, here is how SBCL defines LIST:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I understand this.
What I really want is lisp's LIST, but I don't think that's possible to define in lurk itself right now. I'm sure you understand why I want it, it's for the same purpose everyone uses LIST in lisp - to make a literal list with expressions in it that I want evaluated first. Without that the only way I can think to make such a list is with repeated consing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, I understand. I think one urge is for Lisp's LIST, which can indeed be implemented with macros or when we have support for &REST parameters.
I think you also want something that is definitionally not LIST, but that would accomplish a practical purpose. That something is basically MAP-EVAL of some flavor — which is fine.
I just would not call it LIST, since that's confusing and also will clash with some future implementation of LIST which works as it should.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok yes I see your point, I started out trying to implement LIST but realized it's not possible and then kept the name on something that's not the same. What I ended up with is basically (map eval ...) like you said. I am fine with whatever it is not being in the stdlib but I will need something like this as a utility function at least until there's something better. I am ok with just copying it around. Putting it in the stdlib is probably bad because later it will have no purpose and should go away.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not even saying not to have such a function. I'm just saying don't call it LIST. You could call it MAP-EVAL or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nah, if we have
map
(which we will), writing(map eval lst)
is too trivial to be worth creating a new function.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it might be a little more complicated than you want to try to inline by hand. Remember:
As a test, you want to make sure something like this works:
I think for that to work, you probably need to also pass
(current-env)
todo-the-thing
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this whole file be one big
letrec
? As it stands, aren't we returning a bunch of different envs, each with one function in it? Maybe I'm misunderstanding what(current-env)
does.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also what is the difference between
list-eq
andeq
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
haha, that's funny, somehow I thought that eq didn't work on
bool
and on lists, but it works on both! These are removedThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re it being one big
letrec
or several, that's a good question. I don't know what convention is; as I understand it, as written eachlet
andletrec
updates the current environment to include that variable binding. I've only bundled related/helper functions into the samelet
orletrec
. @porcuquine thoughts on this stylistically?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe (current-env) doesn't do what I thought.
I guess it is not only returning the env at that point, it's also setting the top level's state to include all the variables from that point. In that case the name is a little misleading, the name sounds like it doesn't mutate any state. I'd call it something like
capture-env
, oruse-env
orset-top-level-env
etc.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(current-env)
returns the current lexical environment.Separately, when a
.lurk
file is processed by!(:load …)
, the toplevel lexical environment used by the REPL is replaced with the result of evaluating each form encountered.This has the net effect that sequential forms, each extending the environment, can be used to build up an environment containing all the definitions. This trick allows us to define libraries in different files than use them, and for libraries to depend on other libraries. It also allows more modular definitions within a single file.
This is mostly a hack that won't be so important once the RAM subset is available, but it certainly makes things easier in the current world, which we do want and need to support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So @porcuquine what's your thought on whether to define the lib in one big letrec or a bunch of different ones? I lean toward just using one (assuming that would work just as well).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably multiple, but logically grouped. I'm not 100% sure, but that's my instinct. That gives us an extra layer of structure and will probably help as intermediate stage toward breaking sections out into their own file/module/whatever. It will also facilitate re-ordering for performance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can just be called
length
. I think that matches common lisp. The same probably goes for most of these other functions - you usually don't need to havelist
in the name.Also I think we have automatic currying here, so you can try
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you say the same goes for
list-eq
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
list-eq
is justeq
. There's no need for such a function at all.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is treating NIL specially and will terminate the computation if a NIL is encountered in LST.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I think I mentioned earlier, NIL checks on the CAR of a list when recursing are usually a sign of confused logic (not an absolute rule, but it holds in this case).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, good point, I have to revisit a lot of the functions I wrote to correct this logic error 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here - suggest the
op
argument goes first so you can do this:(by the way I notice
+
isn't a first class function, so I had to introduce the lambda there, not sure if this is something I should open as a separate lurk issue or if it's intentional.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it's how it works. All the built-in operators are hardcoded in the circuit and are not actually 'functions'. We could define functions bound to the symbols in a library in the future, as a convenience — or not. Depending on how details of future versions evolve, along with the circuit, it's possible we revisit this, but for now it's how things are by design.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah good to know, I had the same question about
+