Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix lookahead parsing on filtered views #1658

Closed
wants to merge 4 commits into from

Conversation

bbannier
Copy link
Member

Closes #1655.

@bbannier bbannier self-assigned this Jan 23, 2024
We would previously handle `&size` and `&max-size` almost identical
with the only difference that `&max-size` sets up a slightly larger view
to accomodate a sentinel. In particular, we also used identical code to
set up the position where parsing should resume after such a field.

This was incorrect as it is in general impossible to tell where parsing
continues after a field with `&max-size` since it does not signify a
fixed view like `&size`. In this patch we now compute the next position
for a `&max-size` field by inspecting the limited view to detect how
much data was extracted.

Closes #1668.
We would previously assume that a filter would consume all available
data. This only holds if the filter is attached to a top-level unit, but
in general not if some sub-unit uses a filter. With this patch we
explicitly compute how much data is consumed.

Closes #1652.
@bbannier bbannier force-pushed the topic/bbannier/issue-1655 branch from 551667c to 1c368f2 Compare February 7, 2024 14:42
@bbannier
Copy link
Member Author

bbannier commented Feb 7, 2024

I have rebased the fixes for #1668 and #1652, but this still requires work.

builder::id("filtered"));
pstate.cur = builder::deref(pstate.data);
pstate.begin = builder::begin(pstate.cur);
// FIXME(bbannier): set `lahead` and `lahead_end` to refer to the filtered view.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rsmmr I am a little stuck what to pass here, in particular for lahead_end. Do you have an idea?

@bbannier
Copy link
Member Author

bbannier commented Feb 9, 2024

Abandoning this per #1655 (comment).

@bbannier bbannier closed this Feb 9, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Filter generates code with incompatible iterators
1 participant