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

fix position of orelse and catch in precedence table documentation #8892

Merged
merged 2 commits into from
May 25, 2021

Conversation

mattbork
Copy link
Contributor

The documentation lists orelse and catch as having the second lowest precedence, above only assignment operators. However, in the grammar, they are grouped with BitwiseOp, and in parse.zig as well they have the same .prec of 40 that &, ^, and | have. Further, there is a misplaced ! included with the binary multiplication operators, which appears in neither the grammar nor parse.zig. I think this was once the error union type operator, but that has its own line below the suffix operators.

@mattbork
Copy link
Contributor Author

mattbork commented May 25, 2021

Looking at this more, I believe the .* and .? are also in the wrong place. In the grammar they are grouped with the x[] and x.y suffix operators under SuffixOp, and in parse.zig they are similarly parsed together in parseSuffixOp. Thus they should be together on the first line, above the error union type operator. This precedence actually can make a difference in odd uses like

comptime var T: ?type = u32;
const Error = error { Bad };
pub fn stuff() Error!T.? { return 100; }

The return type is parsed like Error!(T.?) rather than (Error!T).?, which is what the current precedence table implies. The initializer list braces should remain below the error union operator, as in something like var x = [_] XYZError!u32 { 100 }; the ! does of course bind tighter than the initializer as otherwise would make no sense.

@andrewrk andrewrk merged commit 5c25160 into ziglang:master May 25, 2021
@andrewrk
Copy link
Member

Thanks!

One thing to keep in mind is #114

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.

2 participants