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

Runtime ExpressionParser caching issues #200

Closed
fkleuver opened this issue Sep 27, 2018 · 2 comments
Closed

Runtime ExpressionParser caching issues #200

fkleuver opened this issue Sep 27, 2018 · 2 comments
Assignees

Comments

@fkleuver
Copy link
Member

@bigopon discovered a problem today where trying to parse an interpolation expression will actually return the cached value of a previously parsed non-interpolation expression.

I didn't think the caching through very well at the time I changed it, nor tested it very properly, so after looking at it a bit more closely I think this mechanism is due for a rework. I'm just putting my thoughts out here to give you a heads up on what I'll be working on soonish (I may or may not do some more templating tests first)

I'm considering tackling this together with improving the coverage on the renderer and making targeted instruction types numeric again (using an array as the backing store) so they can more easily be inserted (addressing #182), swapped, and have some cleverness with bitwise ops applied to them when appropriate*.

There would be 3 or so lookups in the expression parser, one for each set of parsing rules (interpolation, iterator statement and normal expressions)

The instruction types would be split up again per binding type / command, each having a unique number. The numbers of the instruction types can then, via bitwise operators, be translated to one out of these cache indices.

With all this in mind there are some const enums that need to be cleaned up. The BindingType and ExpressionKind enums have become an incoherent mess and I'd like to try to just get rid of them. Same goes for some of those direct prototype assignments on instruction classes and AST nodes.

Naturally this would also touch the AST and expression parser and probably break a few thousand tests, so those need to be tidied up as well. So that's a fairly big chunk of work upcoming, but I think things will be much cleaner and easier to understand when it's done.

@EisenbergEffect
Copy link
Contributor

Sounds good.

@bigopon
Copy link
Member

bigopon commented Oct 3, 2018

making targeted instruction types numeric again (using an array as the backing store) so they can more easily be inserted (addressing #182), swapped, and have some cleverness with bitwise ops applied to them when appropriate*.

Will this result in array with holes? Quite curious about the plan

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

No branches or pull requests

3 participants