feat(compiler): Enable single-file compilation#2105
Conversation
92e6423 to
89fcb11
Compare
7039e88 to
6e314ce
Compare
marcusroberts
left a comment
There was a problem hiding this comment.
I read this all through, it made sense, code was clear, LGTM!
193e5ec to
6ac38da
Compare
d6920a8 to
e4632f3
Compare
72784ac to
eef00bd
Compare
6e314ce to
9ffb741
Compare
9ffb741 to
3e88dac
Compare
alex-snezhko
left a comment
There was a problem hiding this comment.
Was a bit unclear about some of the changes to the type checking but LGTM overall
| Filepath.to_string(input), | ||
| ) | ||
| ) { | ||
| let compile_typed = file => { |
There was a problem hiding this comment.
This logic with compiling the out of date deps seems to be copy-pasted several times, can move to generic helper? Maybe could also re-instate the boolean flags you removed for resetting compiler state/setting root config in there?
There was a problem hiding this comment.
They're all slightly different between the hooks (which are parametrizable) and compile functions (which aren't really). We could try to find some commonality, but it's mostly just the load_dependency_graph and get_out_of_date_dependencies; after that the caller is responsible for compiling those any way they see fit. I think it's fine that they're separate, personally.
| ~name=filename, | ||
| source, | ||
| ); | ||
| compile(filename, source); |
There was a problem hiding this comment.
Would reset_compiler_state not be needed here?
There was a problem hiding this comment.
Good question. It wasn't before, presumably with the idea that while the LSP is running it's compiling the same stuff. We should probably look into that, but that's a separate issue.
| let stop_found = | ||
| switch (stop) { | ||
| | Some(d) when DV.equal(d, dep) => true | ||
| | _ => false | ||
| }; |
There was a problem hiding this comment.
I assume the code you removed here was due to the stop_found not being used but is it not important?
There was a problem hiding this comment.
It was necessary before because it would iterate to find the next dependency that was out of date so it could be worked on. Now, we just grab all of them at once.
| let clear_persistent_structures = () => { | ||
| Consistbl.clear(crc_units); | ||
| Hashtbl.clear(persistent_structures); | ||
| imported_units := StringSet.empty; |
| imported_units := StringSet.add(s, imported_units^); | ||
| }; | ||
|
|
||
| let imported_opaque_units = ref(StringSet.empty); |
There was a problem hiding this comment.
Could you explain why you removed these opaque unit imports?
There was a problem hiding this comment.
It's just old cruft from back in the day.
| let type_implementation = (prog: Parsetree.parsed_program) => { | ||
| let sourcefile = prog.prog_loc.loc_start.pos_fname; | ||
| let module_name = prog.module_name.txt; | ||
| Env.clear_imports(); |
There was a problem hiding this comment.
This basically used to happen as a part of the full compile pipeline, but now that things are moved around, we reset it where it's relevant.
160be30 to
43c64f0
Compare
Refactors the
DependenciesCompiled,ObjectsLinked,Compiled, andAssembledstates out ofcompile.reand introduces thegraincflag--single-fileto compile a single dependency. The linker is now completely separate from the process of compilation—compiling a file now only means converting source code into an object file. Linking is the combining a number of object files into a wasm file (andgraincwill still do both compiling and linking). This allows build tools to handle compilation and linking separately.The future plan is to make single file mode the default once we have a build tool managing building projects (
silo). I'm not sure that we should get rid of the dependency compilation entirely since it would be useful for development, but that's a decision for later.