Commit 911c839e97

Daniel Hooper <danielchasehooper@gmail.com>
2022-03-20 11:55:04
add error when binary ops don't have matching whitespace on both sides
This change also moves the warning about "&&" from the AstGen into the parser so that the "&&" warning can supersede the whitespace warning.
1 parent 0576086
lib/std/zig/Ast.zig
@@ -328,7 +328,12 @@ pub fn renderError(tree: Ast, parse_error: Error, stream: anytype) !void {
         .expected_initializer => {
             return stream.writeAll("expected field initializer");
         },
-
+        .mismatched_binary_op_whitespace => {
+            return stream.print("binary operator `{s}` has whitespace on one side, but not the other.", .{token_tags[parse_error.token].lexeme().?});
+        },
+        .invalid_ampersand_ampersand => {
+            return stream.writeAll("ambiguous use of '&&'; use 'and' for logical AND, or change whitespace to ' & &' for bitwise AND");
+        },
         .previous_field => {
             return stream.writeAll("field before declarations here");
         },
@@ -2534,6 +2539,8 @@ pub const Error = struct {
         expected_comma_after_initializer,
         expected_comma_after_switch_prong,
         expected_initializer,
+        mismatched_binary_op_whitespace,
+        invalid_ampersand_ampersand,
 
         previous_field,
         next_field,
lib/std/zig/parse.zig
@@ -1451,13 +1451,11 @@ const Parser = struct {
             if (info.prec == banned_prec) {
                 return p.fail(.chained_comparison_operators);
             }
+
             const oper_token = p.nextToken();
-            // Special-case handling for "catch" and "&&".
-            switch (tok_tag) {
-                .keyword_catch => {
-                    _ = try p.parsePayload();
-                },
-                else => {},
+            // Special-case handling for "catch"
+            if (tok_tag == .keyword_catch) {
+                _ = try p.parsePayload();
             }
             const rhs = try p.parseExprPrecedence(info.prec + 1);
             if (rhs == 0) {
@@ -1465,6 +1463,19 @@ const Parser = struct {
                 return node;
             }
 
+            {
+                const tok_len = tok_tag.lexeme().?.len;
+                const char_before = p.source[p.token_starts[oper_token] - 1];
+                const char_after = p.source[p.token_starts[oper_token] + tok_len];
+                if (tok_tag == .ampersand and char_after == '&') {
+                    // without types we don't know if '&&' was intended as 'bitwise_and address_of', or a c-style logical_and
+                    // The best the parser can do is recommend changing it to 'and' or ' & &'
+                    try p.warnMsg(.{ .tag = .invalid_ampersand_ampersand, .token = oper_token });
+                } else if (std.ascii.isSpace(char_before) != std.ascii.isSpace(char_after)) {
+                    try p.warnMsg(.{ .tag = .mismatched_binary_op_whitespace, .token = oper_token });
+                }
+            }
+
             node = try p.addNode(.{
                 .tag = info.tag,
                 .main_token = oper_token,
lib/std/zig/parser_test.zig
@@ -4203,7 +4203,7 @@ test "zig fmt: integer literals with underscore separators" {
         \\const
         \\ x     =
         \\ 1_234_567
-        \\ +(0b0_1-0o7_0+0xff_FF ) +  0_0;
+        \\ + (0b0_1-0o7_0+0xff_FF ) +  0_0;
     ,
         \\const x =
         \\    1_234_567 + (0b0_1 - 0o7_0 + 0xff_FF) + 0_0;
@@ -5105,7 +5105,7 @@ test "recovery: missing comma" {
         \\        2 => {}
         \\        3 => {}
         \\        else => {
-        \\            foo && bar +;
+        \\            foo & bar +;
         \\        }
         \\    }
         \\}
@@ -5139,7 +5139,7 @@ test "recovery: extra qualifier" {
 test "recovery: missing return type" {
     try testError(
         \\fn foo() {
-        \\    a && b;
+        \\    a & b;
         \\}
         \\test ""
     , &[_]Error{
@@ -5154,7 +5154,7 @@ test "recovery: continue after invalid decl" {
         \\    inline;
         \\}
         \\pub test "" {
-        \\    async a && b;
+        \\    async a & b;
         \\}
     , &[_]Error{
         .expected_token,
@@ -5163,7 +5163,7 @@ test "recovery: continue after invalid decl" {
     });
     try testError(
         \\threadlocal test "" {
-        \\    @a && b;
+        \\    @a & b;
         \\}
     , &[_]Error{
         .expected_var_decl,
@@ -5173,12 +5173,12 @@ test "recovery: continue after invalid decl" {
 
 test "recovery: invalid extern/inline" {
     try testError(
-        \\inline test "" { a && b; }
+        \\inline test "" { a & b; }
     , &[_]Error{
         .expected_fn,
     });
     try testError(
-        \\extern "" test "" { a && b; }
+        \\extern "" test "" { a & b; }
     , &[_]Error{
         .expected_var_decl_or_fn,
     });
@@ -5187,8 +5187,8 @@ test "recovery: invalid extern/inline" {
 test "recovery: missing semicolon" {
     try testError(
         \\test "" {
-        \\    comptime a && b
-        \\    c && d
+        \\    comptime a & b
+        \\    c & d
         \\    @foo
         \\}
     , &[_]Error{
@@ -5206,7 +5206,7 @@ test "recovery: invalid container members" {
         \\bar@,
         \\while (a == 2) { test "" {}}
         \\test "" {
-        \\    a && b
+        \\    a & b
         \\}
     , &[_]Error{
         .expected_expr,
@@ -5224,7 +5224,7 @@ test "recovery: extra '}' at top level" {
     try testError(
         \\}}}
         \\test "" {
-        \\    a && b;
+        \\    a & b;
         \\}
     , &[_]Error{
         .expected_token,
@@ -5244,7 +5244,7 @@ test "recovery: mismatched bracket at top level" {
 test "recovery: invalid global error set access" {
     try testError(
         \\test "" {
-        \\    error && foo;
+        \\    error & foo;
         \\}
     , &[_]Error{
         .expected_token,
@@ -5259,13 +5259,15 @@ test "recovery: invalid asterisk after pointer dereference" {
         \\}
     , &[_]Error{
         .asterisk_after_ptr_deref,
+        .mismatched_binary_op_whitespace,
     });
     try testError(
         \\test "" {
-        \\    var sequence = "repeat".** 10&&a;
+        \\    var sequence = "repeat".** 10&a;
         \\}
     , &[_]Error{
         .asterisk_after_ptr_deref,
+        .mismatched_binary_op_whitespace,
     });
 }
 
@@ -5275,7 +5277,7 @@ test "recovery: missing semicolon after if, for, while stmt" {
         \\    if (foo) bar
         \\    for (foo) |a| bar
         \\    while (foo) bar
-        \\    a && b;
+        \\    a & b;
         \\}
     , &[_]Error{
         .expected_semi_or_else,
@@ -5373,6 +5375,54 @@ test "recovery: eof in c pointer" {
     });
 }
 
+test "matching whitespace on minus op" {
+    try testError(
+        \\ _ = 2 -1, 
+        \\ _ = 2- 1, 
+        \\ _ = 2-
+        \\     2,
+        \\ _ = 2
+        \\     -2,
+    , &[_]Error{
+        .mismatched_binary_op_whitespace,
+        .mismatched_binary_op_whitespace,
+        .mismatched_binary_op_whitespace,
+        .mismatched_binary_op_whitespace,
+    });
+
+    try testError(
+        \\ _ = - 1,
+        \\ _ = -1,
+        \\ _ = 2 - -1,
+        \\ _ = 2 - 1,
+        \\ _ = 2-1, 
+        \\ _ = 2 -
+        \\1,
+        \\ _ = 2
+        \\     - 1,
+    , &[_]Error{});
+}
+
+test "ampersand" {
+    try testError(
+        \\ _ = bar && foo,
+        \\ _ = bar&&foo, 
+        \\ _ = bar& & foo, 
+        \\ _ = bar& &foo,
+    , &.{
+        .invalid_ampersand_ampersand,
+        .invalid_ampersand_ampersand,
+        .mismatched_binary_op_whitespace,
+        .mismatched_binary_op_whitespace,
+    });
+
+    try testError(
+        \\ _ = bar & &foo, 
+        \\ _ = bar & &&foo, 
+        \\ _ = &&foo, 
+    , &.{});
+}
+
 const std = @import("std");
 const mem = std.mem;
 const print = std.debug.print;
@@ -5466,7 +5516,10 @@ fn testError(source: [:0]const u8, expected_errors: []const Error) !void {
     var tree = try std.zig.parse(std.testing.allocator, source);
     defer tree.deinit(std.testing.allocator);
 
-    try std.testing.expectEqual(expected_errors.len, tree.errors.len);
+    std.testing.expectEqual(expected_errors.len, tree.errors.len) catch |err| {
+        std.debug.print("errors found: {any}\n", .{tree.errors});
+        return err;
+    };
     for (expected_errors) |expected, i| {
         try std.testing.expectEqual(expected, tree.errors[i].tag);
     }
src/AstGen.zig
@@ -669,24 +669,7 @@ fn expr(gz: *GenZir, scope: *Scope, rl: ResultLoc, node: Ast.Node.Index) InnerEr
         .mod      => return simpleBinOp(gz, scope, rl, node, .mod_rem),
         .shl_sat  => return simpleBinOp(gz, scope, rl, node, .shl_sat),
 
-        .bit_and  => {
-            const current_ampersand_token = main_tokens[node];
-            if (token_tags[current_ampersand_token + 1] == .ampersand) {
-                const token_starts = tree.tokens.items(.start);
-                const current_token_offset = token_starts[current_ampersand_token];
-                const next_token_offset = token_starts[current_ampersand_token + 1];
-                if (current_token_offset + 1 == next_token_offset) {
-                    return astgen.failTok(
-                        current_ampersand_token,
-                        "`&&` is invalid; note that `and` is boolean AND",
-                        .{},
-                    );
-                }
-            }
-
-            return simpleBinOp(gz, scope, rl, node, .bit_and);
-        },
-
+        .bit_and          => return simpleBinOp(gz, scope, rl, node, .bit_and),
         .bit_or           => return simpleBinOp(gz, scope, rl, node, .bit_or),
         .bit_xor          => return simpleBinOp(gz, scope, rl, node, .xor),
         .bang_equal       => return simpleBinOp(gz, scope, rl, node, .cmp_neq),
test/stage2/x86_64.zig
@@ -1555,7 +1555,7 @@ pub fn addCases(ctx: *TestContext) !void {
 
             case.addError(
                 \\pub const a = if (true && false) 1 else 2;
-            , &[_][]const u8{":1:24: error: `&&` is invalid; note that `and` is boolean AND"});
+            , &[_][]const u8{":1:24: error: ambiguous use of '&&'; use 'and' for logical AND, or change whitespace to ' & &' for bitwise AND"});
 
             case.addError(
                 \\pub fn main() void {
test/compile_errors.zig
@@ -3259,7 +3259,7 @@ pub fn addCases(ctx: *TestContext) !void {
         \\    return 5678;
         \\}
     , &[_][]const u8{
-        "tmp.zig:2:11: error: `&&` is invalid; note that `and` is boolean AND",
+        "tmp.zig:2:11: error: ambiguous use of '&&'; use 'and' for logical AND, or change whitespace to ' & &' for bitwise AND",
     });
 
     ctx.objErrStage1("attempted `||` on boolean values",