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/tmpl: fix static/dynamic invariants #21

Merged
merged 6 commits into from
Apr 15, 2023
Merged

Conversation

josharian
Copy link
Contributor

I tried to preserve the invariants more carefully during tree construction. And fixed some assorted things that looked wrong while I poked around the code.

So far (about 10M runs), this passes the fuzz test I added in #19. I'll let it run for a while longer.

I had to "fix" a few golden tests, so maybe I busted something, though. I don't really understand the format when it comes to ranges. I'd be curious to see how it works in practice.

internal/tmpl/tree_test.go Outdated Show resolved Hide resolved
internal/tmpl/tree_test.go Outdated Show resolved Hide resolved
@josharian
Copy link
Contributor Author

This is a lot of changes. But note that the core functionality change is actually net negative LOC.

This makes the code better organized,
better documented, and generally clearer.
Hopefully also more correct.
Importantly, it makes WriteTo not mutate the Tree.
It is in its own package because there's no natural
home for it, and putting it directly in internal/tmpl
causes cyclical imports.
Copy link
Contributor

@floodfx floodfx left a comment

Choose a reason for hiding this comment

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

awesome!

@floodfx floodfx merged commit 77edbb0 into main Apr 15, 2023
@floodfx floodfx deleted the josh/tmpl-experiments branch April 15, 2023 18:49
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