Skip to content

zig ast-check produces an error on well-formed build.zig.zon files #22078

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

Closed
torque opened this issue Nov 26, 2024 · 4 comments · Fixed by #22250
Closed

zig ast-check produces an error on well-formed build.zig.zon files #22078

torque opened this issue Nov 26, 2024 · 4 comments · Fixed by #22250
Labels
bug Observed behavior contradicts documented or intended behavior regression It worked in a previous version of Zig, but stopped working.
Milestone

Comments

@torque
Copy link
Contributor

torque commented Nov 26, 2024

Zig Version

0.14.0-dev.2293+6d781e095

Steps to Reproduce and Observed Behavior

zig ast-check build.zig.zon

on a well-formed build.zig.zon (such as the one in this repository), instead of succeeding, produces the following error message

build.zig.zon:1:1: error: file cannot be a tuple
.{
^

This also happens with zig fmt --ast-check and is likely to be hit if you try to run that on all files in a project with e.g. zig fmt --ast-check --check .

Expected Behavior

zig should not disagree with itself about whether the structure of build.zig.zon is correct.

@torque torque added the bug Observed behavior contradicts documented or intended behavior label Nov 26, 2024
@der-teufel-programming
Copy link
Contributor

build.zig.zon file isn't a well formed Zig file, but a ZON file. ast-check correctly checks that this is not a correct Zig file

@torque
Copy link
Contributor Author

torque commented Nov 27, 2024

While I agree that zon files are indeed not strictly zig files, I think it's pretty reasonable to expect zig's tooling to work with the family of zig file formats. Please see the note about zig fmt automatically operating on .zon files when provided a directory.

This is a "regression" of the previous behavior (i.e. this error is not produced by zig 0.13 or some earlier versions of 0.14-dev). It seems very likely this was a side effect of #21817, but I haven't attempted to bisect it to prove that.

@andrewrk andrewrk added the regression It worked in a previous version of Zig, but stopped working. label Nov 27, 2024
@andrewrk andrewrk added this to the 0.14.0 milestone Nov 27, 2024
torque added a commit to allyourcodebase/libressl that referenced this issue Nov 27, 2024
This works around ziglang/zig#22078 (the ast-check behavior changed
since the last successful CI run). Since we build as part of the job,
this check isn't really necessary anyway, though it certainly is
lighter-weight than trying to compile.
@andrewrk
Copy link
Member

@mlugg would you mind taking a look at this? It's causing damage in the ecosystem which you can see above with the referenced commit "remove --ast-check from zig format". We don't want people to do that.

It's definitely a regression from previous behavior:

image

@mlugg
Copy link
Member

mlugg commented Dec 13, 2024

@andrewrk Sure thing, I'll take a look tomorrow (just heading to bed). The reason this is happening is because ast-check runs AstGen over the file, but ZON files have expressions at the top-level, whereas Zig source files have container declarations at the top level. This worked by accident before, because a Zig source file containing only an expression was parsed as a single-element tuple type; i.e. if the file contained expr, the file evaluated to struct { expr }. Since files are now banned from being tuples, this isn't the case anymore.

The obvious thing is that ZON files shouldn't go through AstGen -- it's just not appropriate, because ZON is not the same thing as Zig. I could just make ast-check on ZON files only validate parsing, but I could also potentially write a ZON validation pass which checks the ZON is well-formed (top-level is a valid expression, and it doesn't use any illegal constructs like struct { ... } or T{ ... }). I'll take a look at Mason's ZON work, since this will tie in to that.

mlugg added a commit to mlugg/zig that referenced this issue Dec 16, 2024
Currently, `zig ast-check` fails on ZON files, because it tries to
interpret the file as Zig source code. This commit introduces a new
verification pass, `std.zig.ZonGen`, which applies to an AST in ZON
mode.

Like `AstGen`, this pass also converts the AST into a more helpful
format. Rather than a sequence of instructions like `Zir`, the output
format of `ZonGen` is a new datastructure called `Zoir`. This type is
essentially a simpler form of AST, containing only the information
required for consumers of ZON. It is also far more compact than
`std.zig.Ast`, with the size generally being comparable to the size of
the well-formatted source file.

The emitted `Zoir` is currently not used aside from the `-t` option to
`ast-check` which causes it to be dumped to stdout. However, in future,
it can be used for comptime `@import` of ZON files, as well as for
simpler handling of files like `build.zig.zon`, and even by other parts
of the Zig Standard Library.

Resolves: ziglang#22078
mlugg added a commit to mlugg/zig that referenced this issue Dec 16, 2024
Currently, `zig ast-check` fails on ZON files, because it tries to
interpret the file as Zig source code. This commit introduces a new
verification pass, `std.zig.ZonGen`, which applies to an AST in ZON
mode.

Like `AstGen`, this pass also converts the AST into a more helpful
format. Rather than a sequence of instructions like `Zir`, the output
format of `ZonGen` is a new datastructure called `Zoir`. This type is
essentially a simpler form of AST, containing only the information
required for consumers of ZON. It is also far more compact than
`std.zig.Ast`, with the size generally being comparable to the size of
the well-formatted source file.

The emitted `Zoir` is currently not used aside from the `-t` option to
`ast-check` which causes it to be dumped to stdout. However, in future,
it can be used for comptime `@import` of ZON files, as well as for
simpler handling of files like `build.zig.zon`, and even by other parts
of the Zig Standard Library.

Resolves: ziglang#22078
mlugg added a commit to mlugg/zig that referenced this issue Dec 16, 2024
Currently, `zig ast-check` fails on ZON files, because it tries to
interpret the file as Zig source code. This commit introduces a new
verification pass, `std.zig.ZonGen`, which applies to an AST in ZON
mode.

Like `AstGen`, this pass also converts the AST into a more helpful
format. Rather than a sequence of instructions like `Zir`, the output
format of `ZonGen` is a new datastructure called `Zoir`. This type is
essentially a simpler form of AST, containing only the information
required for consumers of ZON. It is also far more compact than
`std.zig.Ast`, with the size generally being comparable to the size of
the well-formatted source file.

The emitted `Zoir` is currently not used aside from the `-t` option to
`ast-check` which causes it to be dumped to stdout. However, in future,
it can be used for comptime `@import` of ZON files, as well as for
simpler handling of files like `build.zig.zon`, and even by other parts
of the Zig Standard Library.

Resolves: ziglang#22078
@mlugg mlugg closed this as completed in c7485d7 Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior regression It worked in a previous version of Zig, but stopped working.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants