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.

Sign up for GitHub

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

Jump to bottom

change ParseOutput::Tree to a Nonterminal instead of a Node #1184

Closed
OmarTawfik opened this issue Dec 10, 2024 · 0 comments · Fixed by #1187
Closed

change ParseOutput::Tree to a Nonterminal instead of a Node #1184

OmarTawfik opened this issue Dec 10, 2024 · 0 comments · Fixed by #1187
Assignees
@beta-ziliani

Comments

@OmarTawfik
Copy link
Contributor

Not blocking to this PR: IIUC, now any parse operation can return a Nonterminal, right?
I wonder if we should also change the type of ParseOutput::tree() from Node to Nonterminal. Users always pass a NonterminalKind to parse(), so it makes sense to expect a Nonterminal with the same kind back. Thoughts?

Originally posted by @OmarTawfik in #1172 (comment)
@OmarTawfik OmarTawfik mentioned this issue Dec 10, 2024
Merged
@beta-ziliani beta-ziliani mentioned this issue Dec 11, 2024
Merged
@beta-ziliani beta-ziliani mentioned this issue Dec 21, 2024
Closed
github-merge-queue bot pushed a commit that referenced this issue Jan 13, 2025
@beta-ziliani
Change ParseOutput to return a NonTerminal instead of a Node (#…
6389361 
…1187)

Solves #1184
To go on top of #1172

This has a big impact in the failing examples, which now return the
expected `NonTerminal`.

This was acknowledgedly coded by playing the whack-a-mole with the type
system. I'm not so happy with the bunch of `Rc::clone()` that leaked
out; I will consider an alternative once we settle this is what we need
and there are no more pressing issues.

Currently doing a full CI run locally to find what more to fix. In the
meantime, I'll mark this as draft.
@beta-ziliani beta-ziliani linked a pull request Jan 14, 2025 that will close this issue
Change ParseOutput to return a NonTerminal instead of a Node #1187
Merged
@github-project-automation github-project-automation bot moved this from ⏳ Todo to ✅ Done in Slang - 2024 H2 Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@beta-ziliani beta-ziliani

Labels
None yet
Projects
Slang - 2024 H2
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Change ParseOutput to return a NonTerminal instead of a Node manastech/slang
2 participants
@beta-ziliani @OmarTawfik