zls icon indicating copy to clipboard operation
zls copied to clipboard

Ideas for Compile file on save and report errors

Open GoWind opened this issue 3 years ago • 8 comments
trafficstars

I am using the coc-nvim plugin along with zls. I want zls to be able to report compilation errors when I finished editing my file and save it (right now, only Syntax errors are reported).

Why is this important ? I have a program like this, and my editor does not report any errors

const std = @import("std");

fn s(n: usize) void {
    var g = [_]u8{0} ** n;
    std.debug.print("{}\n", .{g[0]});
}

pub fn main() void {
    std.debug.print("{}", .{s()});
}

However, when I try to run this, I get the following error messages:

zig run h.zig
./h.zig:9:30: error: expected 1 argument(s), found 0
    std.debug.print("{}", .{s()});
                             ^
./h.zig:3:1: note: declared here
fn s(n: usize) void {
^
./h.zig:4:25: error: unable to evaluate constant expression
    var g = [_]u8{0} ** n;

To me it looks like the right place to do the changes would be here (applySave function) and this function currently performs some action, only if the file being saved is a build file. I am looking for ideas and suggestions on how to compile a file via a zig compiler API and I can't seem to find any. The refreshDocument handler for example, uses std.zig.parse to find any syntax errors and report them. I would like to contribute for this feature and would love it, if you can provide this feature via zls or some ideas on how to implement one.

GoWind avatar Apr 30 '22 10:04 GoWind

ZLS does not currently compile any code it only parses it.

Zig does not provide a compiler API beyond invocation of the compiler executable. Meaning in order to actually compile code then either zig build needs to be run with the correct flags (which are project dependant) as build.zig may link to libraries, add include paths, pass packages, build options, set targets, modes, and much more. Or a full compiler invocation as generated by zig build would need to be provided to zls for that specific project.

The problem with this feature is it will never be good enough due to zig's laziness and comptime. If ZLS did check if code compiles then the below would be considered valid when targeting linux but invalid when targeting anything else. So what should ZLS do? Recompile the project with every valid combination of build options/targets?

const std = @import("std");
const builtin = @import("builtin");

pub fn main() void {
    if (builtin.os.tag == .linux) {
        std.debug.print("this is fine\n", .{});
    } else {
        std.debug.print("this is isn't\n");
    }
}

leecannon avatar Apr 30 '22 11:04 leecannon

I want zls to be able to report compilation errors when I finished editing my file and save it (right now, only Syntax errors are reported).

"compilation" means "analysis" + "optimizations" + "emitting objects" + "linking", so you probably mean analysis, because zls has and likely will not have optimization integration (meaning roughly only covering Zir, maybe some parts of Air).

"Number of stuff analysis" should be part of AstGen (lowering to Zir), which is attempted in #554. To get there, it is require to refactor many things. Contact @SuperAuguste, if you are interested to help out.

"Constant" and "non-constant" things should be not part of AstGen as type checks comes into play here.

What you are looking for is likely a REPL like watchexec. The Zig REPL is not stabilized yet.

matu3ba avatar Jul 27 '22 17:07 matu3ba

compilation" means "analysis" + "optimizations" + "emitting objects" + "linking", so you probably mean analysis,

Yes, but I think analysis also involves emitting objects , no ? (atleast that's what I saw when reading through the Zig build process, where it create a DAG of Compilation objects and builds the part that has changed).

My idea to do this is very rudimentary :

When zig build is called, the buildRunner already knows how to create a build binary that can then compile the project. This build binary is also aware of the local and global caches, so it knows hows to incrementally compile. Unless the build.zig file itself is changed, zls can recompile the project by invoking the build binary in a sub-process and then serializing the stdout of this build process into LSP client compatible messages.

Unfortunately, a lot of the data structures and fns (such as Compilation, cmdBuild) are outside of the standard library and cannot be imported by zls (and I assume that we cannot rely on the stability of these APIs as they are compiler internals)

That said, if @SuperAuguste has any ideas on how these can be done otherwise, I'd be happy to contribute and help as I find this to be a big blocker for my productivity !

GoWind avatar Jul 30 '22 12:07 GoWind

I just checked out the https://github.com/zigtools/zls/pull/554/files PR and it looks like the same conclusion was reached and to overcome the problem of non-stable, internal, compiler APIs, the code is being simply copied ?

GoWind avatar Jul 30 '22 12:07 GoWind

This build binary is also aware of the local and global caches, so it knows hows to incrementally compile

Take note, that incrementally compilation means storing + computing the ZIR changes to emit objects and link them. Plus letting the linker use and expand the GOT dynamically to patch up object files and executables.

zig fmt changes require a complete reparse, so zls might never gets to the ZIR on errors unless we can 1. fixup the AST ourselves and write it back to AST, 2. have tailored compilation infra (being abit redundant). Or we just accept that the user is forced to always wait that zig fmt + reparse before we can provide zls feedback.

zls can recompile the project by invoking the build binary in a sub-process and then serializing the stdout of this build process into LSP client compatible messages

Does your editor plugin not support parsing + storing compilation errors as jump to labels? Yeah, we should definitely do that at some point.

the code is being simply copied

I can tell you from looking at render.zig (zig fmt to print the AST) that there is no in-place patching of the AST, which would be optimal for minimizing delay. If we can ensure the AST is always formatted, we could use a lookup table + patch the AST in place from user changes for analysis. That would mean making render.zig iterative, having a simple lookup for indent+space and the zig fmt stuff being applied on another AST. However, we would then have duplicated render.zig or made it slower for one-shot formatting.

AstGen with Zir is still changing alot, so no idea about that.

matu3ba avatar Jul 30 '22 13:07 matu3ba

Does your editor plugin not support parsing + storing compilation errors as jump to labels? Yeah, we should definitely do that at some point.

Parsing yes, but it doesn't catch compilation errors(for example wrong type of value assigned to a variable etc).

I can tell you from looking at render.zig (zig fmt to print the AST) that there is no in-place patching of the AST, which would be optimal for minimizing delay. If we can ensure the AST is always formatted, we could use a lookup table + patch the AST in place from user changes for analysis. That would mean making render.zig iterative, having a simple lookup for indent+space and the zig fmt stuff being applied on another AST. However, we would then have duplicated render.zig or made it slower for one-shot formatting.

Understood. I think someone mentioned in a talk/chat/issue providing support for this at the compiler level ? In any case it would be nice if we could tap into Zig compilation like the std library and be able to compile zig code via zls.

GoWind avatar Aug 01 '22 20:08 GoWind

I think someone mentioned in a talk/chat/issue providing support for this at the compiler level ?

Zig itself will provide zls lsp capabilities and more, because its tricky to extract info from a performance optimized datastructure without dedicated interface and its then simpler to maintain upstream.

I have a bunch of questions for my approach on zig-reduce regarding AST locations and AST invalidation on the next compiler meeting, see https://gist.github.com/matu3ba/a6a76dd8309b37a002cd09de79512e99 (still need to check source locations after Air).Maybe some useful preparatory approaches will come out.

Also, aurame showed interest in providing the groundwork for the in-tree server implementation (see https://github.com/ziglang/zig/wiki/Self-Hosted-Compiler-Meetings#2022-07-28).

matu3ba avatar Aug 01 '22 23:08 matu3ba

For zig-reduce I will write src/Ast->src/translate_c/ast conversion routine, which allows to poke directly in the tree.

At least how I understand it, render() in src/translate_c.zig retains the node structure, but breaks the token offset structure with spaces, no leaving out of optional {,} brackets etc.

So at least from my point of view, finding mappings from old AST->new AST in src/Ast->src/translate_c/ast format would be desirable for zls/internal-lsp as this would be independent from token offsets and optional tokens.

However, this still does not handle or describe operating on (intermediate) invalid AST (changing code) or how to proceed with (intermediate) AstGen failures (unused vars).

matu3ba avatar Aug 05 '22 09:08 matu3ba

implemented in #1361

Techatrix avatar Sep 23 '23 21:09 Techatrix