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

internal: Try adding a label to a chumsky parser #4705

Closed
wants to merge 1 commit into from

Conversation

max-sixty
Copy link
Member

Moving this from #4700. Do we know why adding a label removes the error message? Is it something to do with merging multiple labels loses the labels themselves

impl SimpleLabel {
fn merge(self, other: Self) -> Self {
match (self, other) {
(SimpleLabel::Some(a), SimpleLabel::Some(b)) if a == b => SimpleLabel::Some(a),
(SimpleLabel::Some(_), SimpleLabel::Some(_)) => SimpleLabel::Multi,
(SimpleLabel::Multi, _) => SimpleLabel::Multi,
(_, SimpleLabel::Multi) => SimpleLabel::Multi,
(SimpleLabel::None, x) => x,
(x, SimpleLabel::None) => x,
}
}
}
?

Is this the correct design? I would think we want the most specific item, not sure whether that's possible.

(Tagging @m-span as he was the most recent author of the code, but no worries if you're not sure / or if this was the result of a refactor. And having new line isn't important per se, I mean it as a more general question)

@max-sixty max-sixty requested a review from m-span July 8, 2024 23:43
@m-span
Copy link
Collaborator

m-span commented Jul 9, 2024

Do we know why adding a label removes the error message? Is it something to do with merging multiple labels loses the labels themselves

That code is copied from the default Chumsky implementation. My best guess it is so that when an error propagates upwards, only the first, most specific error-label is kept.

@m-span
Copy link
Collaborator

m-span commented Jul 9, 2024

Still, I'll look into this case

@max-sixty
Copy link
Member Author

max-sixty commented Jul 9, 2024

That code is copied from the default Chumsky implementation. My best guess it is so that when an error propagates upwards, only the first, most specific error-label is kept.

Ah great, I see, it's explained here: zesterer/chumsky#238

So it seems like we just need to add labels to the "higher" item so that's reported, rather than nothing. (I need to think through what counts as "higher"...)

Still, I'll look into this case

Sure, if you want to take a look, but no obligation

@max-sixty max-sixty closed this Jul 9, 2024
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.

None yet

2 participants