Implement parsing through enums/structs instead of vecs/strings
... and adjust rest of workspace to work with it.
Closes #134.
Sorry that this is all one commit, I couldn't think of a way to split it up since basically every part of this depends on another part.
The big change comes in the `rs/parser/src/nodes.rs` file - this implements all the various structs and enums that exist in the grammar, as best as I could determine. When I was converting the main crate over to parsing this AST, I ran into a few situations where the grammar in `glicol.pest` and the way that it was turned into `glicol_synth` nodes seemed to disagree, and I just had to make my best judgement in those situations. Sometimes I modified the struct in the AST, and sometimes I just added a `panic!` with an explicit error message about how that feature/syntax isn't supported yet. If the `panic` message is inaccurate and that feature won't ever be supported, let me know and I'll modify the structs to fail at parsing that sort of thing.
The second biggest changes are in `rs/main/src/{lib,util}.rs`. Two things happen here:
1. In `util.rs`, the AST is converted into nodes to be placed on the graph. This was fairly straightforward, and allowed us to remove a ton of `unimplemented!` calls (since all enums used are now exhaustive and have no invalid states, instead of just using `GlicolPara` as a catch-all).
2. In `lib.rs`, I changed the way we update the engine with code. Previously, we would update the code, then set the flag `needs_update`, then actually diff and update the graph the next time `next_block` was called. I felt like this was not ideal, as it causes the AST and code to be out-of-sync. So instead I switched it to make `update_with_code` actually do the diffing and updating of the syntax tree, and `next_block` just gets the next block of sound.
This required some undesirable combination of methods into one large method that does a bunch of stuff, but I think that it's for the best. It allows us to remove a bunch of members from the `Engine` struct that are only there temporarily for diffing, and put those into another struct that borrows from `self` and the new `YokedAst` for diffing, instead of copying data over to the temporary members of `self` during the diffing process.
I also got rid of the `lcs_diff::diff` method and elected to do diffing manually, as that method required cloning of each AST struct that was passed in, and that's an unnecessary cost to pay. Instead, we just iterate through each new and old node + chain and see what stayed the same, what was removed, and what was added.
A few other minor things:
- `GlicolPara` is now generic over `S` so that we can use it in situations where we want to borrow from something else to create like a `GlicolPara::Reference` to avoid another allocation
- The code that writes an `EngineError` to a `&mut [u8]` is now moved to the `wasm` crate, since that's the only thing that actually needs that code.
- The `wasm` crate is now part of the workspace since it needs to access `pest` (to match details of the `EngineError`, so its profile details have been moved to a profile called `wasm-release` (instead of just using the `release` profile, as was previously done)
- `pest` errors are now boxed in our return types since they are quite large and thus make every `Result` type that contains them quite large as well, incurring a significant cost on method returns even if we don't encounter an error. Now, they shouldn't be quite as large, at the cost of only one allocation *if* we run into an error (as opposed to incurring costs before regardless of if we ran into an error).
The main issue with this all is that I haven't actually tested it yet. Before merging, I'd like to get some more formal tests written inside the parser crate, and I'd also like to just try this out with something like `glicol-cli` to make sure everything seems to still work just fine.
On that note, this should also allow much clearer error messages in `glicol-cli` that can point to exactly where we failed to parse. This should make for a much better experience there, once this gets merged and I can push a PR to `glicol-cli` to work with this new API.
Of course, this will also change the public API, so all these crates should be bumped to `0.14.0` if this gets merged.
If you have any questions, obviously let me know! And please don't merge until I've got some tests in (just as some motivation for myself).
合并状态:已合并 合并于 2024-06-21 关闭于 2024-06-21 14 条评论