quantified-uncertainty / squiggle

An estimation language
https://squiggle-language.com
MIT License
148 stars 22 forks source link

Fix arrow parsing; avoid extra block nodes in AST #3136

Closed berekuk closed 3 months ago

berekuk commented 3 months ago

This fixes #3110, and also fixes #2670, and also simplifies grammar and AST (see tests in diff for examples).

Previously:

$ node dist/cli/index.js parse -e 'x=5'
(Program
  (LetStatement
    :x
    (Block 5)
  )
)

Now:

$ node dist/cli/index.js parse -e 'x=5'
(Program
  (LetStatement :x 5)
)

The arrow bug in particular was caused by parsing let statements as lambdas first, before falling back on arbitrary expressions. I don't know why it was there; there might be some obscure case where this is a technically breaking change, but all tests are parsed and I hope it's fine.

Note that this doesn't give us any significant performance improvements; I already unwrap single-expression blocks during compilation.

(Which is good because otherwise I'd be worried about this change; I remember that some time ago skipping these block nodes wasn't safe).

changeset-bot[bot] commented 3 months ago

🦋 Changeset detected

Latest commit: 4ce0af0ecee10e3eb0e3dcbba2bef1ddfa33840e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages | Name | Type | | ----------------------------------- | ----- | | @quri/squiggle-lang | Patch | | @quri/squiggle-components | Patch | | @quri/prettier-plugin-squiggle | Patch | | @quri/versioned-squiggle-components | Patch | | vscode-squiggle | Patch | | @quri/squiggle-textmate-grammar | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

vercel[bot] commented 3 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

4 Ignored Deployments | Name | Status | Preview | Updated (UTC) | | :--- | :----- | :------ | :------ | | **quri-hub** | ⬜️ Ignored ([Inspect](https://vercel.com/quantified-uncertainty/quri-hub/77FKKM1S7sPZJs2SYVqhRZKHfP7S)) | [Visit Preview](https://quri-hub-git-prettier-bug-quantified-uncertainty.vercel.app) | Mar 29, 2024 2:33am | | **quri-ui** | ⬜️ Ignored ([Inspect](https://vercel.com/quantified-uncertainty/quri-ui/6sAi1uZw9oPPXwN6kPQZpRv9KvVz)) | [Visit Preview](https://quri-ui-git-prettier-bug-quantified-uncertainty.vercel.app) | Mar 29, 2024 2:33am | | **squiggle-components** | ⬜️ Ignored ([Inspect](https://vercel.com/quantified-uncertainty/squiggle-components/3iARj31H3d61sUJhpRKkkxTJF3L1)) | [Visit Preview](https://squiggle-components-git-prettier-bug-quantified-uncertainty.vercel.app) | Mar 29, 2024 2:33am | | **squiggle-website** | ⬜️ Ignored ([Inspect](https://vercel.com/quantified-uncertainty/squiggle-website/HDfknPZJhsbN7Yvr36jEdWPK9Vf9)) | [Visit Preview](https://squiggle-website-git-prettier-bug-quantified-uncertainty.vercel.app) | Mar 29, 2024 2:33am |
OAGr commented 3 months ago

hm...

image

Hub preview is down. Not sure why though.

berekuk commented 3 months ago

Previews on specific commits don't work anymore (I mentioned this in https://quri.slack.com/archives/C06KPA4MGU9/p1711572884389439?thread_ts=1711571253.986579&cid=C06KPA4MGU9 yesterday), but previews on branches do: https://quri-hub-git-prettier-bug-quantified-uncertainty.vercel.app/

OAGr commented 3 months ago

Huh, interesting, thanks!

OAGr commented 3 months ago

Great, I can confirm that this works!