Skip to content

Allow matching empty sets #4009

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

Merged
merged 4 commits into from
Aug 1, 2024
Merged

Allow matching empty sets #4009

merged 4 commits into from
Aug 1, 2024

Conversation

geo2a
Copy link
Contributor

@geo2a geo2a commented Jul 30, 2024

This PR adds rudimentary matching of internalized sets: only empty sets are allowed to match, and an indeterminate result is returned for non-empty sets.

@geo2a geo2a marked this pull request as ready for review July 30, 2024 17:12
Copy link
Contributor

@goodlyrottenapple goodlyrottenapple left a comment

Choose a reason for hiding this comment

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

LGTM perhaps consider adding a simple unit test here:

and in the other two relevant match test suites...tho given we only handle the most trivial case, not a deal breaker.

Comment on lines +386 to +387
emptySet
emptySet
Copy link
Member

Choose a reason for hiding this comment

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

Need to export emptySet from Test.Booster.Pattern.InternalCollections
Rest LGTM, we just have to fix this test.

@geo2a geo2a force-pushed the georgy/empty-set-matching branch from 54b5482 to 5201abd Compare August 1, 2024 11:59
@geo2a geo2a added the automerge label Aug 1, 2024
@geo2a geo2a force-pushed the georgy/empty-set-matching branch from a673ea7 to 1f149a3 Compare August 1, 2024 12:05
@rv-jenkins rv-jenkins merged commit e4bd3aa into master Aug 1, 2024
6 checks passed
@rv-jenkins rv-jenkins deleted the georgy/empty-set-matching branch August 1, 2024 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants