Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error when binary ops don't have matching whitespace on both sides. #11156

Merged
merged 7 commits into from
Mar 20, 2022

Conversation

danielchasehooper
Copy link
Contributor

@danielchasehooper danielchasehooper commented Mar 14, 2022

Fixes #7399

This change also moves the warning about && from the AstGen into the parser so that the && warning can supersede the whitespace warning.

This is my first PR for Zig, so be gentle :) Let me know if there is anything that can be improved.

…ixes ziglang#7399

This change also moves the warning about "&&" from the AstGen into the parser so that the "&&" warning can supersede the whitespace warning.
Copy link
Member

@ifreund ifreund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think moving the error for && to the parser is ok, at least not with the current implementation. &&foo where the &s are the unary address of operator is valid. See #8046 and #9114. We should really have a test covering that case though...

lib/std/zig/parse.zig Outdated Show resolved Hide resolved
Co-authored-by: Isaac Freund <[email protected]>
@danielchasehooper
Copy link
Contributor Author

danielchasehooper commented Mar 14, 2022

My change doesn't affect &&foo - that works the way it did before since I'm doing the check for && in parseExprPrecedence and not parsePrefixExpr. foo&&bar works the way it did before, however I noticed something: The existing code in AstGen.zig currently doesn't accept bar&&foo and tells you to change it to and, even when you mean bitwise_and address_of. You currently have to write it as bar& &foo to appease the compiler.

I'm working on a change that recommends you either add a space between &&, or change it to and.

@danielchasehooper
Copy link
Contributor Author

danielchasehooper commented Mar 14, 2022

Ok, &&foo works and foo&&bar requires either using and or & &. I added additional parser tests surrounding &&.

@matu3ba
Copy link
Contributor

matu3ba commented Mar 15, 2022

The zig fmt check fails, which is only run in zinc/linux_test.sh. ie ci.ziglang/dron/pr

Is this also failing locally for you?

@danielchasehooper
Copy link
Contributor Author

@matu3ba Couple questions:

  1. (maybe stupid question) I'm on macOS, is zinc/linux_test.sh linux only?
  2. How do I run it? It looks like it depends on specific environment variables/paths to be set up to work
  3. Are the CI tests run on my branch, or on what would result from the merge of this PR into master?
  4. Is there a way to see if zinc/linux_test.sh was passing before my changes?

@andrewrk
Copy link
Member

Hi @danielchasehooper - check CONTRIBUTING.md for some instructions on how to run the tests locally.

@danielchasehooper
Copy link
Contributor Author

danielchasehooper commented Mar 16, 2022

@matu3ba I misread your message at first. Yes it was failing locally for me, but I've run zig fmt and we're all good now.

@andrewrk zig fmt --check doesn't seem to get run in local tests like it is in the zinc/linux_test.sh, so running local tests wouldn't have caught this for me. Does that sound correct to you?

Additionally, zig build test is failing on my arm64 macOS machine as of cb3b1dd (i.e. before any of my changes). So I couldn't even run the full tests to check my changes. Do you know anything about this? Output of failing test below:


$ zig build test

...passing tests removed for brevity...

Test 13/13 stack-trace dumpCurrentStackTrace (Debug)...
========= Expected this output: =========
source.zig:7:8: [address] in foo (test)
    bar();
       ^
source.zig:10:8: [address] in main (test)
    foo();
       ^

================================================

error: TestFailed
/Users/daniel/Developer/zig/lib/std/fs.zig:1285:9: 0x1021a9156 in std.fs.Dir.makeDir (build)
        try os.mkdirat(self.fd, sub_path, default_new_dir_mode);
        ^
/Users/daniel/Developer/zig/lib/std/os.zig:2757:19: 0x1021a95a0 in std.os.mkdiratZ (build)
        .EXIST => return error.PathAlreadyExists,
                  ^
/Users/daniel/Developer/zig/lib/std/os.zig:2717:9: 0x1021a92a8 in std.os.mkdirat (build)
        return mkdiratZ(dir_fd, &sub_dir_path_c, mode);
        ^
/Users/daniel/Developer/zig/lib/std/fs.zig:1285:9: 0x1021a9156 in std.fs.Dir.makeDir (build)
        try os.mkdirat(self.fd, sub_path, default_new_dir_mode);
        ^
/Users/daniel/Developer/zig/lib/std/os.zig:2757:19: 0x1021a95a0 in std.os.mkdiratZ (build)
        .EXIST => return error.PathAlreadyExists,
                  ^
/Users/daniel/Developer/zig/lib/std/os.zig:2717:9: 0x1021a92a8 in std.os.mkdirat (build)
        return mkdiratZ(dir_fd, &sub_dir_path_c, mode);
        ^
/Users/daniel/Developer/zig/lib/std/fs.zig:1285:9: 0x1021a9156 in std.fs.Dir.makeDir (build)
        try os.mkdirat(self.fd, sub_path, default_new_dir_mode);
        ^
/Users/daniel/Developer/zig/test/tests.zig:853:17: 0x102289114 in [email protected] (build)
                return error.TestFailed;
                ^
/Users/daniel/Developer/zig/lib/std/build.zig:3297:9: 0x1021a9ebe in std.build.Step.make (build)
        try self.makeFn(self);
        ^
/Users/daniel/Developer/zig/lib/std/build.zig:506:9: 0x1021a8ad1 in std.build.Builder.makeOneStep (build)
        try s.make();
        ^
/Users/daniel/Developer/zig/lib/std/build.zig:500:17: 0x1021a8a55 in std.build.Builder.makeOneStep (build)
                return err;
                ^
/Users/daniel/Developer/zig/lib/std/build.zig:500:17: 0x1021a8a55 in std.build.Builder.makeOneStep (build)
                return err;
                ^
/Users/daniel/Developer/zig/lib/std/build.zig:500:17: 0x1021a8a55 in std.build.Builder.makeOneStep (build)
                return err;
                ^
/Users/daniel/Developer/zig/lib/std/build.zig:461:13: 0x102198c5b in std.build.Builder.make (build)
            try self.makeOneStep(s);
            ^
/Users/daniel/Developer/zig/lib/std/special/build_runner.zig:209:21: 0x102193dcd in main (build)
            else => return err,
                    ^
/Users/daniel/Developer/zig/lib/std/os.zig:2757:19: 0x1021a95a0 in std.os.mkdiratZ (build)
        .EXIST => return error.PathAlreadyExists,
                  ^
/Users/daniel/Developer/zig/lib/std/os.zig:2717:9: 0x1021a92a8 in std.os.mkdirat (build)
        return mkdiratZ(dir_fd, &sub_dir_path_c, mode);
        ^
/Users/daniel/Developer/zig/lib/std/fs.zig:1285:9: 0x1021a9156 in std.fs.Dir.makeDir (build)
        try os.mkdirat(self.fd, sub_path, default_new_dir_mode);
        ^
/Users/daniel/Developer/zig/lib/std/os.zig:2757:19: 0x1021a95a0 in std.os.mkdiratZ (build)
        .EXIST => return error.PathAlreadyExists,
                  ^
/Users/daniel/Developer/zig/lib/std/os.zig:2717:9: 0x1021a92a8 in std.os.mkdirat (build)
        return mkdiratZ(dir_fd, &sub_dir_path_c, mode);
        ^
/Users/daniel/Developer/zig/lib/std/fs.zig:1285:9: 0x1021a9156 in std.fs.Dir.makeDir (build)
        try os.mkdirat(self.fd, sub_path, default_new_dir_mode);
        ^
/Users/daniel/Developer/zig/lib/std/os.zig:2757:19: 0x1021a95a0 in std.os.mkdiratZ (build)
        .EXIST => return error.PathAlreadyExists,
                  ^
/Users/daniel/Developer/zig/lib/std/os.zig:2717:9: 0x1021a92a8 in std.os.mkdirat (build)
        return mkdiratZ(dir_fd, &sub_dir_path_c, mode);
        ^
/Users/daniel/Developer/zig/lib/std/fs.zig:1285:9: 0x1021a9156 in std.fs.Dir.makeDir (build)
        try os.mkdirat(self.fd, sub_path, default_new_dir_mode);
        ^
/Users/daniel/Developer/zig/lib/std/os.zig:2757:19: 0x1021a95a0 in std.os.mkdiratZ (build)
        .EXIST => return error.PathAlreadyExists,
                  ^
/Users/daniel/Developer/zig/lib/std/os.zig:2717:9: 0x1021a92a8 in std.os.mkdirat (build)
        return mkdiratZ(dir_fd, &sub_dir_path_c, mode);
        ^
/Users/daniel/Developer/zig/lib/std/fs.zig:1285:9: 0x1021a9156 in std.fs.Dir.makeDir (build)
        try os.mkdirat(self.fd, sub_path, default_new_dir_mode);
        ^
/Users/daniel/Developer/zig/lib/std/os.zig:2757:19: 0x1021a95a0 in std.os.mkdiratZ (build)
        .EXIST => return error.PathAlreadyExists,
                  ^
/Users/daniel/Developer/zig/lib/std/os.zig:2717:9: 0x1021a92a8 in std.os.mkdirat (build)
        return mkdiratZ(dir_fd, &sub_dir_path_c, mode);
        ^
/Users/daniel/Developer/zig/lib/std/fs.zig:1285:9: 0x1021a9156 in std.fs.Dir.makeDir (build)
        try os.mkdirat(self.fd, sub_path, default_new_dir_mode);
        ^
/Users/daniel/Developer/zig/lib/std/os.zig:2757:19: 0x1021a95a0 in std.os.mkdiratZ (build)
        .EXIST => return error.PathAlreadyExists,
                  ^
/Users/daniel/Developer/zig/lib/std/os.zig:2717:9: 0x1021a92a8 in std.os.mkdirat (build)
        return mkdiratZ(dir_fd, &sub_dir_path_c, mode);
        ^
error: the following build command failed with exit code 1:
/Users/daniel/Developer/zig/zig-cache/o/0805107800bfebdbbdeee4dd304f1974/build /Users/daniel/Developer/zig/build/zig /Users/daniel/Developer/zig /Users/daniel/Developer/zig/zig-cache /Users/daniel/.cache/zig test

@schmee
Copy link
Contributor

schmee commented Mar 16, 2022

@danielchasehooper zig build test will run all the tests on all the backends in all the release modes, which will probably not work. Feel free to ask build questions in #stage2-meetings in Discord or ping me directly (same name as here) and I'll try to help you get up and running.

lib/std/zig/Ast.zig Outdated Show resolved Hide resolved
@Vexu Vexu merged commit 911c839 into ziglang:master Mar 20, 2022
@danielchasehooper danielchasehooper deleted the whitespace_warn_7399 branch March 20, 2022 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal: Require whitespace on both sides of - or neither
6 participants