Skip to content
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

New language frontend merging & architectural change #283

Closed
38 of 41 tasks
inclyc opened this issue Dec 24, 2023 · 1 comment · Fixed by #327
Closed
38 of 41 tasks

New language frontend merging & architectural change #283

inclyc opened this issue Dec 24, 2023 · 1 comment · Fixed by #327
Labels
help wanted Extra attention is needed tracking Tracking issue

Comments

@inclyc
Copy link
Member

inclyc commented Dec 24, 2023

Motivation

For now controllers just send full text file to workers, and periodically killing workers, if they are outdated.
This is very error-prone because the controller parses the file for location information, or attrpath construction, and workers parses these files again.
Previously we don't have a memory-safe nix parser and it is coupled with evaluators in upstream C++ nix. Thus the only solution is sending raw text contents to the worker, and let them evaluate the file, the controller just roughly redirect lsp requests to workers. And since the controller does not have a good parser, we have experienced very high overhead.

Related issues:

Previous Work

There are a parser generated by bison and in-complete semantic analysis module in these two PRs. For now they are outdated because we have a new handwritten parser.

The iteration plan

Let the controller do all parsing stuff, and serialize ASTs to workers.

As commented above, currently both controller process & worker process do parsing stuff, and I think a better architecture is just offload all parsing job to the controller. Serialize ASTs in shared memory and let workers fetch the AST and perform evaluation.

The controller will ask worker process values & envs for specific nodes from worker, not sending full text contents. The serialization format & implementation:

Uses a new language frontend just for parsing, but uses official nix implementation for evaluation

This is a technical decision. Because evaluator is not very easy to maintain and current implementation "just works" for language servers. Here are PRs related to the new frontend:

Misc Refactor

Parser

string_part : interpolation
            | STRING_PART
            | STRING_ESCAPE
interpolation : "${" expr "}"
string : " string_part* "
       | '' string_part* ''
 path : path_fragment (path_fragment)*  path_end
Context     PS_Expr       PS_Path        PS_Path
'(' expr ')'
attrset_expr : REC? '{' binds '}'

binds : ( binding | inherit )*

inherit :  'inherit' '(' expr ')' inherited_attrs ';'
        |  'inherit' inherited_attrs ';'

inherited_attrs: attrname*

binding : attrpath '=' expr ';'

attrpath : attrname ('.' attrname)*
expr_simple :  INT
            | FLOAT
            | string
            | path
            | hpath
            | URI
            | expr_paren
            | legacy_let
            | attrset_expr
            | list_expr

hpath : "~" prefixed path, $HOME hack

expr_select : expr_simple '.' attrpath
            | expr_simple '.' attrpath 'or' expr_select
            | expr_simple 'or' <-- special "apply", 'or' is argument
            | expr_simple
expr_app : expr_app expr_select
         | expr_select
list_expr : '[' expr_select* ']'
if_expr : 'if' expr 'then' expr 'else' expr
assert_expr : 'assert' expr ';' expr
let_in_expr :  'let' binds 'in' expr
legacy_let: `let` `{` binds `}`
  • Parse Legacy Let
with_expr :  'with' expr ';' expr
expr_op : '!' expr_op
        | '-' expr_op
        | expr_op BINARY_OP expr_op
        | expr_app
lambda_expr : lambda_arg ':' expr

lambda_arg : ID
           | ID @ {' formals '}'
           | '{' formals '}'
           | '{' formals '}' @ ID

formals : formal? (',' formal)*

formal : ID
       | ID '?' expr
       | "..."
expr      : lambda_expr
          | assert_expr
          | with_expr
          | let_in_expr
          | if_expr
          | expr_op
  • Parse Expr

Semantic Lowering

Semantic Checking

The new worker, and new controller

Controller

nix-node-eval

  • WIP
@inclyc inclyc added help wanted Extra attention is needed tracking Tracking issue labels Dec 24, 2023
inclyc added a commit that referenced this issue Feb 12, 2024
This is the final implementation for #283, and it is also tracked by it.

Fixes: #283
@inclyc inclyc reopened this Feb 12, 2024
@inclyc inclyc closed this as completed Apr 9, 2024
@inclyc
Copy link
Member Author

inclyc commented Apr 16, 2024

All frontend things done. Waiting for #421 for lsp features impl

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed tracking Tracking issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant