NixOS / nix

Nix, the purely functional package manager
https://nixos.org/
GNU Lesser General Public License v2.1
12.69k stars 1.51k forks source link

Separate the parser from libexpr #8164

Open theoparis opened 1 year ago

theoparis commented 1 year ago

Is your feature request related to a problem? Please describe.

I have been wanting to write a experimental LLVM JIT Compiler for the nix expression language (it is not meant to replace nix anytime soon and is just for fun). However, I have found that the only C/C++ library for parsing nix is this repository here, and that I can't simply use the parser by itself without using a ton of other dependencies along with the nix evaluator.

Describe the solution you'd like

I'm not sure what the best way of doing this is.... However, since the parser is stuck with including eval.hh at the moment I think it would be worth considering #1102 again. I'm not sure why it has been closed since 2018 when there hasn't been any progress... ๐Ÿ˜•

Describe alternatives you've considered

I could have used rnix-parser, however that requires me to write C++ bindings for rnix which would take a lot of work.

There also seems to be nixexpr written with PegTL however the license is too restrictive.

Additional context

See https://github.com/NixOS/nix/issues/1102 and https://github.com/NixOS/nix/blob/master/src/libexpr/parser.y#L24

roberth commented 1 year ago

The libnixexpr architecture isn't ideally suited for extracting just the parser, as we parse immediately into a data structure with evaluation methods. We'll probably want to keep it that way for simplicity and efficiency ingesting a lot of mostly single use files.

So instead of extracting a parser library, perhaps we could extract an evaluator library? We'd have something like

I like the idea of such a division of labor. Having a proper interface between these parts should also help the integration of a JIT.

cc @Ericson2314

thufschmitt commented 1 year ago

That would be nice, but also a huge amount of work and dangerous refactorings, so I wouldn't expect this to happen any time soon, unfortunately. Even if someone were to do that refactoring, getting it to a level where we can confidently merge it would take a lot of time.

Another alternative might be https://github.com/nix-community/tree-sitter-nix, although I think remembering that tree-sitter isn't a great fit for an evaluator for some reasons

roberth commented 1 year ago

dangerous refactorings

If anything about moving code around is dangerous, we have a serious underlying problem. Discovering what that problem is will make Nix more robust, which is all the more reason to do it.

getting it to a level where we can confidently merge it would take a lot of time.

Indeed a big PR that does everything in one go is impossible to review, so let's not do that. We'll want to do something like this in reviewable steps. If we want to do it. I wouldn't do it in the short term.

thufschmitt commented 1 year ago

@roberth I definitely didn't say we shouldn't do it eventually. Just that it will be a significant change that will take a lot of effort and time.

Ericson2314 commented 1 year ago

Yeah definitely agree with the "refactoring to discover issues" is good framing. @theoparis If you would like to take a stab at this, and can break it into multiple PRs so no single one is too big (I imagine there would be some preparatory work and then pulling the trigger), that would be a very nice contribution and we would greatly appreciate it.

I'm not sure why it has been closed since 2018 when there hasn't been any progress... confused

Indeed. I gave it a different title to emphasize the specification aspects and reopened it.

RossComputerGuy commented 1 year ago

With how much of a change this would introduce, I kinda feel like this should've begun as an RFC. It won't exactly be that simple to just tear out and split the code.

Ericson2314 commented 1 year ago

@RossComputerGuy I am not sure this requires an RFC because no behavior change is being proposed. Big refactors that don't change behavior are something we must be careful to review, but ultimately are not something non-developer end users need not worry about in and of themselves.

roberth commented 1 year ago

Yeah, no RFC needed, just a plan. I'd do something like:

  1. create the libnixlang library
  2. on a temporary branch, create a proof of concept by moving things into that library, and see if it makes sense to commit an intermediate stage of this process of moving things over
  3. check with the Nix team what'd be good timing for the refactor
  4. create the definitive branch that moves things over in reviewable commits/PRs
roberth commented 1 year ago

Alternatively, you could make LLVM an optional dependency of libexpr and develop the JIT right here in your fork of the nix project, so that you can reuse all the existing setup we have for those dependencies. That's what nix develop is for.

inclyc commented 1 year ago

Separate the parser from libexpr

Actually it is very easy to do such thing. The parser currently couples with nix::EvalState but it actually needs only nix::SymbolTable and nix::PosTable, since the grammars are declarative, it is easy to extract the parser by just copying parser.y and lexer.l in your repository.

Another alternative might be https://github.com/nix-community/tree-sitter-nix, although I think remembering that tree-sitter isn't a great fit for an evaluator for some reasons

This parser might be not suitable for JIT compilers because it just parses the file, not perform any desugaring & semantic checking. We actually do this kind of things in parser.y such as checking if there is a duplicated attribute. Using tree-sitter-nix requires to rewrite such logic again, which is definitely not good for software engineering.

๐Ÿ‘ to @roberth's solution. And I would suggest decoupling "eval" stuff from "parsing" stuff also.