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

Factor ast.Slice out into its own package #416

Merged
merged 3 commits into from
Jan 7, 2025
Merged

Factor ast.Slice out into its own package #416

merged 3 commits into from
Jan 7, 2025

Conversation

mcy
Copy link
Member

@mcy mcy commented Jan 7, 2025

I plan to use this pattern when I implement the IR package, so having it not live in the AST package is a good idea.

I've also refactored the interfaces to be simpler, and to reduce the number of methods that actually need to be implemented: almost all types (except for ast.TypeList) that were previously an ast.Slice now have an accessor for it.

The ast.Slice.Iter function has been dropped, and iteration is now done with dedicated iterators in seq.

Copy link
Member

@jhump jhump left a comment

Choose a reason for hiding this comment

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

Nice.


// Delete deletes the element at the given index.
//
// Should panic if idx < 0 or idx > Len().
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Should panic if idx < 0 or idx > Len().
// Should panic if idx < 0 or idx >= Len().

@mcy mcy enabled auto-merge (squash) January 7, 2025 19:19
@mcy mcy merged commit b04f563 into main Jan 7, 2025
8 checks passed
@mcy mcy deleted the mcy/sliceface branch January 7, 2025 19:21
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.

2 participants