-
Notifications
You must be signed in to change notification settings - Fork 52
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
Change run
to import
and typecheck it properly
#495
Comments
OK, in that case I've marked it |
Perhaps
Then running We might also want to consider "dhall style" import. If I understand correctly how this would work, then any Term could be an import statement (e.g. a file or URL), and the import resolution system would substitute such terms with the content of their import locations. So for example, given this file:
Then the following expressions are valid:
And we could use the dot syntax for import that contains a list of definitions. For example, instead of using
Well if we can make this work, then it would be a nice solution for code packaging and distribution. Though I'm not really sure it's worth the effort, but I figured it may be useful to mention it here. |
@TristanCacqueray thanks for the idea! I think Swarm is actually well suited for this style since everything in the language is an expression. Even a file full of In fact, now that I think about it, this is pretty much already how I was suggesting The issue of namespacing is a separate issue, I think. Having names from imports automatically qualified seems too heavyweight --- I definitely don't want to have to type |
An important caveat / thing that has to be properly addressed and designed for is that there needs to be an explicit difference between "re-fetch the most current version of this thing from the filesystem or network since it's been updated and I want the latest version" and "refer to a cached version of this thing which is named by an external URI but shouldn't be re-fetched every time it is used". This is more than just a difference in semantics; the first must have some sort of |
Is For example, imagine running I think what we need is to:
Also EDIT: Actually we need to have the source loaded into memory to construct a well-typed |
What you said about having
This is actually much more theoretically complex and interesting than I thought. Basically because Swarm programs are going to be long-running, we need to support some kind of "hot reloading". Dhall does not have to deal with this because Dhall programs do not run for a long time; as I understand it has an "import resolution" phase at the beginning where it essentially replaces all import expressions with the current contents of the referenced file, and only then proceeds to evaluate everything using that snapshot of all the imported file contents. If you edit the files while Dhall is evaluating things it will have no effect; you would have to re-run the Dhall program from the beginning to see the changes. |
I am very confused. Your first branch of bullet points leads to caching files, but that is exactly what I was pondering previously. But in the second branch you argue for keeping run as it is now in some form. I do not understand why it would be a good idea to hot reload instead of doing the loading once at the beginning like Dhall. Its approach sounds very reasonable to me and exactly like what this Issue originally proposed. In particular I do not see what prevents us from resolving the source at the time it is type-checked — even if it is number 42 or “Hello world!” If I want to import over network I can probably live with a less then a second delay of downloading a file in the background. The technical difficulty would be to put type checking in another thread and make sure it returns before we let the command be executed. But that to me sounds much preferable to:
|
Sorry for the confusion --- I was trying to argue why we need both.
Hmm... maybe you are right. 🤔 The scenario I had in mind is when you have a file of definitions,
(or whatever the syntax is), this causes the definitions to be loaded from the file, typechecked etc. But later you edit the file and then want to reload the new, changed definitions. So you type In summary, I was confused about the difference between robot programs running for a long time, and a game running for a long time during which you might enter many REPL inputs. As you say, I think the key idea is to resolve imports at typechecking time, just like Dhall does. That naturally implies that a long-running robot program will never see updates to remote files (because it is typechecked once and the runs for a long time), but entering stuff at the REPL is a natural opportunity to get the most recent version of something every time you reference it (because every time you enter something at the REPL, it is a new Swarm expression which has to be type-checked etc.). So in the end, I agree we are back to what was originally proposed in this issue. But now I understand it much better. 😄 |
@byorgey OK, I think we understood each other, hopefully I did not come across as too dismissive. 🙂 The thing leads me to argue for long running robots to not get extra power from PS: Also immediately type checked import does not give extra power, so we do not need to come up with a capability and device for it! 😄 |
No, not at all! I'm really grateful to have someone who won't let me get away with nonsense. 😄
Yes, I completely, 100% agree. Like I said, when I was talking about "hot swapping" what I actually had in mind was the ability to reload a previously loaded module by typing it at the REPL again (which is not really hot swapping at all), not hot swapping new code into a running robot.
Agreed, yay! |
@byorgey can we improve this in a minimal way and get 90% of the benefits? I am thinking about initially restricting With that restriction, we could add a stage before type-checking where in
This would involve adding What do you think @byorgey, is this a reasonable proposal or a dead end? 🙂 |
I think it is a reasonable proposal but I do not think it is actually much easier than the full thing. It is like getting 90% of the benefits with 90% of the work. 😉 I am actually thinking of working on this next, and I don't think it will be hard. |
@byorgey great, I trust your judgement and I am happy to leave it to you. 👍 In my defence you tagged this as C-Project so I thought there would be a lot more work involved with caching the files and resolving collisions and whatnot. 😅 |
I mean, there are probably lots of details to get right so it will be a decent amount of work, hence deserving 🤔 maybe we need different sets of tags to distinguish between how difficult something will be and how much work it will be... |
OK, I take it back, this is super annoying. 😠 Basically the problem is that It would be easy enough to just put UPDATE: It's even worse than I thought, the tendrils reach all the way into reading the palette in a world description, because it has a |
Loading and resolving imports at parse time is simply not going to work. I think the next thing I'm going to try is to parameterize |
@byorgey robots also need this change, because they are expected to have So I think they need an extra |
@xsebek agreed! I think it is going to be a far-reaching change but I'm hoping parameterizing everything by phase will make it all work. |
…actoring (#2074) A bit of initial refactoring towards #495 . - Replace `TSyntax` with `Syntax` in a couple places where the types are not needed. - Once we implement #495 producing `TSyntax` is going to get a lot harder, so the more places we can eliminate it the better. - Also some related minor refactoring, and a new function `readNonemptyTerm`. We're not using it yet, but (1) I thought I would need it in this PR, and (2) when I realized I didn't, I decided to leave it there anyway since it will likely come in handy later.
Now that `Failure` is moved to `swarm-util` (#2155), that was the last remaining dependency of `Swarm.Game.ResourceLoading` which prevented it from moving as well. The `ResourceLoading` code is not specific to Swarm scenarios; this PR moves it to `swarm-util` to make it more generally applicable. In fact, the `ResourceLoading` code is already being used to load `.sw` files when executing the `Run` command; in order to implement #495 we will need to do that loading in order to resolve `import` expressions, but that code will live in `swarm-lang`, which cannot import `swarm-scenario`.
Elsewhere (e.g. #7, #208, #328) I have talked about how I don't like the
run
command and want to get rid of it. However, I'd like to explore an alternative: maybe I don't like it because it's not typechecked properly!In particular, right now
run : string -> cmd ()
, and typechecking it is utterly trivial: just make sure it is given a string as an argument. However, this leads to a few problems:x.sw
definesgo : cmd ()
, thenrun("x.sw"); go
would work perfectly fine at runtime, but currently does not typecheck, with an error aboutgo
being unbound.run
prints value with bogus type #328 , the type of the value in the file is not taken into account, giving us bizarre things being printed out liketrue : ()
.run
works now can break type safety entirely:run
breaks type safety #2068.So here's an alternative we might contemplate:
run
toimport
and make it special built-in syntax (requiring an argument which is a string constant) rather than a command constant.import "foo"
, readfoo.sw
from disk and typecheck it (unless it has already been imported before, in which can we can just look it up in a map). The type ofimport "foo"
is simply the type of the Swarm expression infoo.sw
, and also binds the names of any definitions it contains. In other words it should behave exactly as if the contents offoo.sw
had been typed in place of theimport
(modulo the fact that files can be imported more than once but only need to be checked once).IO
to the type inference monad. Or actually, maybe not --- perhaps we could add a phase in between parsing and type checking where we gather up all theimport
statements and load and parse their referents into aMap
, recursively processing any imports encountered; then we pass that map to the typechecker too. In any case it seems like it requires us to addIO
to theprocessTerm
function in one way or another.This would still not be a true module system (in particular there's no way to do qualified imports, or hide names, etc.) but it would be way better than what we have, and it would at least solve #208 and #328 .
The text was updated successfully, but these errors were encountered: