search

minoki / LunarML

The Standard ML compiler that produces Lua/JavaScript
MIT License
356 stars 10 forks source link

Nixpkgs bundle feedback #4

Closed toastal closed 1 year ago

toastal commented 2 years ago

Hi. I was looking to test out this project for doing Neovim configuration because I like the ML family and this is one of the only active ML projects compiling to Lua/LuaJIT (though never touched SML). The first step was getting it to compile on NixOS and with Nixpkgs for myself. However, I ran into a couple of snags that I think make help make the project a bit more robust. I did get the project to build and was thinking of submitting it, but I feel y'all may have some ideas on how to round off the rough edges.

To start, here is my branch and the main file in question to make the derivation: https://gitlab.com/toastal/nixpkgs/-/blob/lunarml/pkgs/development/compilers/lunarml/default.nix

As for the feedback:

  1. I would like to see make install from the root as the installPhase block of mine is a bit of mess as I can't seem to figure out the install ... flags so I went with cp. A Nix derivation prefers the Unix-style file structure with binaries in bin, libraries in lib/$PROJECT_NAME, docs in share/doc/$PROJECT_NAME, etc. This leads to more follow ups...
  2. Can the sml-lib not be hardcoded from the binary? This prevents me from running make test in the checkPhase because I used substituteInPlace to rewrite that directory. Because I put lunarml in bin and sml-lib in lib/$PROJECT_NAME for the final build, compiling this hardcoded path is a problem as make test is ran in the current directory that doesn't have a lib/$PROJECT_NAME/sml-lib to work with.
  3. Is it ideal to fetch the libraries with curl in a Makefile? This is subjective, but the Nix sandbox doesn't allow a lot of IO, like connecting to the network, so I couldn't just cd thirdparty && make install. I'm fetching the files in the derivation and it works, so this is more of an acknowledgement.

Tangential feature requests:

  1. Can the compiler do stdin and stdout compilation (no writing/reading files)? Ultimately I would like to hook this into home-manager's Neovim configuration machinery where they allow a specified Lua compiler for configuration. Currently it supports VimL, Lua... but also Fennel and Teal. Skipping the disk makes this a lot easier in many use cases like this.
  2. Tree-shaking unused functions. Compiling hello.sml was fast, but it's over 7000 lines 😅
  3. I'd love to see documentation on how to use your own or other's libraries. I don't quite understand how $SML_LIB is supposed to work.
  4. Some sort of nested set() function (didn't see). Like set' [ "vim"; "g"; "scrolloff" ] 4 for vim.g.scrolloff = 4.
minoki commented 2 years ago

Thanks for the feedback!

As for the feedback:

  1. OK.
  2. OK, I'll make the path customizable via an command-line option or environment variable. Is that sufficient?
  3. The repository contains thirdparty/ml-yacc.tgz and thirdparty/smlnj-lib.tgz. Can't they be used?

As for the feature requests:

  1. OK. I'll add options like --input-stdin / --output-to-stdout.
  2. In fact, LunarML already does dead code elimination. The problem is, DCE and other optimizations like inlining are not very powerful yet.
  3. There are package managers for SML like smackage or smlpkg. We'll need to find out if they are usable with LunarML.
  4. Writing your own set function shouldn't be too hard. Here's one (not tested):
local
fun set' (table, fieldName, [], newValue) = Lua.set (table, Lua.fromString fieldName, newValue)
  | set' (table, fieldName, x :: xs, newValue) = set' (Lua.field (table, fieldName), x, xs, newValue)
in
(* set : string list -> Lua.value -> unit *)
fun set [] newValue = raise Fail "set: empty"
  | set [name] newValue = raise Fail "We don't have Lua.setGlobal yet"
  | set (name :: x :: xs) newValue = set' (Lua.global name, x, xs, newValue)
end
val () = set ["vim", "g", "scrolloff"] (Lua.fromInt 4)

Well, I'll consider adding Lua.setGlobal and Lua.setField...

toastal commented 2 years ago

Thanks for the quick response.

OK, I'll make the path customizable via an command-line option or environment variable. Is that sufficient?

Either of these can be used from the sandbox just fine. It could help users that want to develop on alternative stdlibs?? Unsure.

The repository contains thirdparty/ml-yacc.tgz and thirdparty/smlnj-lib.tgz. Can't they be used?

Ah, I didn't check until after I ran make install so I didn't see it. I could, but usually I'm not a fan of checking binary files into my repo for the long-term as the git pulls can get unreasonable. But I probably should use the tarballs while they are there.

OK. I'll add options like --input-stdin / --output-to-stdout

I'd go for just plain --stdin and --stdout or have these be the defaults and and files need -o and -f or -i. I've seen these patterns most commonly with tools--plus it's a bit redundant to "input in" and "output out".

In fact, LunarML already does dead code elimination.

Ah. The -O flag didn't seem to do much on the hello.sml so I wasn't sure.

Writing your own set function shouldn't be too hard. Here's one (not tested):

Sure, but I think this may come up a lot in folks' code when interacting with Lua code bases like the ones in Neovim I mentioned. I see love.graphics.arc has a lot of nested calls. Awesome (window manager) can too. I have a hunch that it would be common enough task with FFI that it could be stdlib-worthy.

minoki commented 1 year ago

I've changed the directory layout and added -B option to specify the library directory (default: ../lib/lunarml).

toastal commented 1 year ago

https://gitlab.com/toastal/nixpkgs/-/blob/lunarml/pkgs/development/compilers/lunarml/default.nix

Much cleaner. I'll close this for now because the Nix bundling is now very concise with little issue. Anything other feedback/suggestions would be better raised as separate issues if any pique the project's interest.