Skip to content

Conversation

llogick
Copy link
Contributor

@llogick llogick commented Oct 17, 2023

Feeling out how disruptive this would be

Ignoring missing semicolons (https://github.com/ziglang/zig/blob/eb5276c94eaab238551fdae9a2e77b0133e31cfb/lib/std/zig/Parse.zig#L876) solves

const E = enum {
    foo,
    bar,
    baz,
    fn foo(e: E) void {
        switch (e) {.<cursor>};
    }
};

pub const bar = struct {
    pub const baz = struct {
        pub const Foo = struct {
            a: u32 = 0,
        };

        pub fn qux() u32 {
            return Foo{
                .<cursor>
            };
        }
    };
};

pub var state: S = undefined;
pub const S = struct { foo: u32 };

pub fn main() void {
    state.<cursor>
    {
        _ = state.foo;
    }
    state.foo;
}

Haven't observed any side effects and the number and type of errors is the same.

Somewhat of a changelog
> Added Ast.zig, Parse.zig, parser_test.zig, render.zig and tokenizer.zig

DocumentStore.zig
        const Parser = @import("stage2/Ast.zig");

        var zls_ast = try Parser.parse(self.allocator, text, .zig);
        errdefer zls_ast.deinit(self.allocator);
        var tree = Ast{
            .source = zls_ast.source,
            .tokens = zls_ast.tokens,
            .nodes = zls_ast.nodes,
            .extra_data = zls_ast.extra_data,
            .errors = zls_ast.errors,
        };
        
stage2/Ast.zig
        tokens: std.zig.Ast.TokenList.Slice,
        nodes: std.zig.Ast.NodeList.Slice,
        errors: []const std.zig.Ast.Error,
        
> All tests pass (as expected)
> Changed stage2/Parse.zig:876 to     try p.expectSemicolon(.expected_semi_after_decl, true);
> Tests that failed (I think the parser has a point here)
    try testCompletion(
        \\const S = struct { alpha: u32 };
        \\fn foo(comptime T: type) T {}
        \\const s = foo(S);
        \\const foo = s.<cursor>
    , &.{
        .{ .label = "alpha", .kind = .Field, .detail = "alpha: u32" },
    });
    try testCompletion(
        \\const S = struct { alpha: u32 };
        \\fn foo(any: anytype, comptime T: type) T {}
        \\const s = foo(null, S);
        \\const foo = s.<cursor>
    , &.{
        .{ .label = "alpha", .kind = .Field, .detail = "alpha: u32" },
    });
    
> Fixed up the tests
> These cases now work (num errors is the same which is ok and better than I expected)
pub const bar = struct {
    pub const baz = struct {
        pub const Foo = struct {
            a: u32 = 0,
        };

        pub fn qux() u32 {
            return Foo{
                .<cursor>
            };
        }
    };
};

pub var state: S = undefined;
pub const S = struct { foo: u32 };

pub fn main() void {
    state.<cursor>
    {
        _ = state.foo;
    }
    state.foo;
}

> Can this work?
        fn alias() void {
            var s = Alias{.<cursor>}; // Def node is after the current node
        }
        pub const Outer = struct {
            pub const Inner = struct {
                isf1: bool = true,
                isf2: bool = false,
            };
        };
        const Alias0 = Outer.Inner;
        const Alias = Alias0;

> Changed stage2/Parse.zig:2966 to
            else => return p.addNode(.{
                .tag = .unreachable_literal,
                .main_token = p.tok_i,
                .data = .{
                    .lhs = undefined,
                    .rhs = undefined,
                },
            }), //return null_node,
> now it does, but Sema prob won't like this (custom node_tags?)

> Some tests failed, the change revealed that those statements should be within a `test{..}` block.. another point for a working parser

> dot compl for the struct fields above don't work
> tweaked stage2/Parse.zig:3315
    {
        try p.warn(.expected_initializer); // return p.fail(.expected_initializer);
        p.tok_i += 1;
    } else p.tok_i += 3;
    
> result
            else => return p.addNode(.{.<nope>
                .tag = .unreachable_literal,.<works>
                .main_token = p.tok_i,.<works>
                .data = .{.<nope>
                    .lhs = undefined,.<works>
                    .rhs = undefined,.<works>
                    .<nope>
                },
                .<nope>
            }), //return null_node,
> works only if surrounded by existing fields...

> zig test src/stage2/Parse.zig 
All 340 tests passed.

> checked
const World = struct {
	name: []const u8,
	placeHolderOtherStructFields: [33693888]u8 = undefined,

	pub fn init(self: World, name: []const u8) void {
		self = .{};
	}
};
> works! but not if `self: *World`(check completions logic, not a parser issue)

fixes #1112
fixes #1525
fixes #1535
fixes #1549

@Techatrix
Copy link
Member

Would you consider to try and upstream these changes? I think these changes quite different from a PR like ziglang/zig#14391 where a feature that only gets used by third party projects is being added.

@llogick
Copy link
Contributor Author

llogick commented Oct 21, 2023

// Test for `fnCall(.{.})` and `fnCall(Parser.Node{. .some})` because they are handled in different places
// Test for completions after `.{` and every `,`
pub fn gamma(p: Parser) void {
    return p.addNode(.{
        .tag = 1,
        .main_token = 1,
        .data = .{
            .lhs = undefined,
            .rhs = p.addNode(Parser.Node{
                .tag,
            }),
        },
    });
}
const Parser = struct {
    const Node = struct {
        tag: u32,
        main_token: u32,
        data: struct {
            lhs: u32,
            rhs: u32,
        },
    };

    pub fn addNode(_: Node) u32 {}
};

fn foo(e: Enum) void {
    switch (e) {}
}

const Enum = enum {
    foo,
    bar,
    baz,
};

@llogick
Copy link
Contributor Author

llogick commented Nov 15, 2023

Upkeep's been quite light, the only change being ziglang/zig@f258a39

Copy link

codecov bot commented Nov 21, 2023

Codecov Report

Attention: Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 78.45%. Comparing base (80ddf7b) to head (c341967).
Report is 802 commits behind head on master.

Files with missing lines Patch % Lines
src/DocumentStore.zig 80.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1536   +/-   ##
=======================================
  Coverage   78.45%   78.45%           
=======================================
  Files          35       35           
  Lines       10549    10552    +3     
=======================================
+ Hits         8276     8279    +3     
  Misses       2273     2273           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Techatrix
Copy link
Member

Isn't only the Parse.zig and the parse function from Ast.zig needed? That could all be moved into Parse.zig to have only a single file.

@llogick
Copy link
Contributor Author

llogick commented Mar 2, 2024

Isn't only the Parse.zig and the parse function from Ast.zig needed? That could all be moved into Parse.zig to have only a single file.

The intention is to keep it (close to) a carbon copy as it makes it easy to update by just overwriting a/the file(s) and looking at the overlapping sections.

@llogick llogick force-pushed the parser branch 2 times, most recently from 6261c74 to f6bdfc6 Compare March 2, 2024 01:43
@Techatrix Techatrix added this to the 0.12.0 milestone Mar 2, 2024
@Techatrix Techatrix removed this from the 0.12.0 milestone Apr 20, 2024
@llogick
Copy link
Contributor Author

llogick commented Sep 7, 2025

I'm not sure why this PR is still open, but I'm growing fond of it and updated it for it's upcoming anniversary 😋

Cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants