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

Add support for subtype matching in patterns. #47

Merged
merged 1 commit into from
Apr 16, 2021
Merged

Conversation

rofinn
Copy link
Member

@rofinn rofinn commented Apr 16, 2021

We sometimes use types in our patterns, so this would allow us to use parent types or Unions to describe a collection of paths to match against.

@codecov
Copy link

codecov bot commented Apr 16, 2021

Codecov Report

Merging #47 (b386dd7) into main (d39b647) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #47   +/-   ##
=======================================
  Coverage   98.88%   98.88%           
=======================================
  Files           7        7           
  Lines         270      270           
=======================================
  Hits          267      267           
  Misses          3        3           
Impacted Files Coverage Δ
src/patterns.jl 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d39b647...b386dd7. Read the comment docs.

@glennmoy
Copy link
Member

Is there any chance of there being an ambiguity?

For instance, if some tuple element contains two types that are both subtypes of T, is there some Pattern that could be given that would not be able to resolve which one to choose? Or does it always pick the first?

I tried pseudo-coding an example but wasn't able to come up with one, so perhaps it's not possible, or maybe it's friday evening brain haze.

@glennmoy
Copy link
Member

FYI you might want to change the repo settings to only allow merge on approval.

@rofinn
Copy link
Member Author

rofinn commented Apr 16, 2021

Same as with the wildcards it'll just pick the first match, which is why we have an OrderedSet to have consistent behaviour. This will likely make the solving #36 a bit harder, but probably worth it.

@rofinn rofinn merged commit 166c053 into main Apr 16, 2021
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