ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
33.78k stars 2.47k forks source link

Parser shows error at wrong place if closing parenthesis is missing in if optional #7186

Open frmdstryr opened 3 years ago

frmdstryr commented 3 years ago

I've hit this too many times by accident now... so another minor nitpick/potential error message improvement... or possibly a parser issue.

If you forget to add a closing paren ) in an optional if the error always points to the end of the if block however it should know right at the first | that a paren is missing.

const std = @import("std");

fn getOptional(key: []const u8, default: []const u8) ?[]const u8 {
    if (std.mem.eql(u8, key, "special")) return key;
    return default;  
}

pub fn main() !void {
    if (getOptional("foo") |value| {
        std.log.warn("{}", .{value});
    } else {
        std.log.warn("missing", .{});
    }
}

https://zig.godbolt.org/z/Eb71E5

<source>:11:7: error: expected token ')', found 'else'
    } else {
      ^
ASM generation compiler returned: 1
<source>:11:7: error: expected token ')', found 'else'
    } else {
      ^
Execution build compiler returned: 1

After scanning through the stage2 parser a little it seems like the error is at the end because the Expr seems to be capturing the Payload so any valid expression before the closing paren puts the error at the bottom while.

Edit this might be a parser bug... as https://zig.godbolt.org/z/fh99P1 gives the same error

const std = @import("std");

fn getOptional(key: []const u8, default: []const u8) ?[]const u8 {
    if (std.mem.eql(u8, key, "special")) return key;
    return default;  
}

pub fn main() !void {
    if (getOptional("test", "bar") |t| t |foo| foo |bar| bar |bas| bas |value| {
        std.log.warn("{}", .{value});
    } else {
        std.log.warn("missing", .{});
    }
}
frmdstryr commented 3 years ago

I think the problem is it's treating each pipe as a bitwise or which is fine until it sees a {.

frmdstryr commented 3 years ago

This is not really specific to optionals, even a missing paren in a comparision leads to strange error messages eg https://zig.godbolt.org/z/Wj341P

const std = @import("std");

pub fn main() !void { 
    if ( value > 1 {
        std.log.warn("{}", .{value});
    } 
}

leads to this

<source>:5:37: error: expected token '}', found ';'
        std.log.warn("{}", .{value});
                                    ^
ASM generation compiler returned: 1
<source>:5:37: error: expected token '}', found ';'
        std.log.warn("{}", .{value});
                                    ^
Execution build compiler returned: 1

So it seems like the parser is accepting blocks in places that don't make sense.

Unless there's a block label I don't think it should allow the opening {.