Alternative extractor implementation

[nystrom]

This is an alternative implementation of extractors that uses Type rather than Val{<:Symbol} as in #114 for the dispatch. If the extract method is defined for the given type, the extractor pattern is used; otherwise, the struct pattern is used.

Fixes #92

[codecov]

Codecov Report

Attention: Patch coverage is 75.64103% with 19 lines in your changes missing coverage. Please review.

Project coverage is 98.29%. Comparing base (4cbd3fd) to head (8cd29d0).
Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
src/binding.jl	80.95%	12 Missing ⚠️
src/pretty.jl	0.00%	6 Missing ⚠️
src/Match.jl	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##              main     #116      +/-   ##
===========================================
- Coverage   100.00%   98.29%   -1.71%     
===========================================
  Files           12       12              
  Lines         1049     1113      +64     
===========================================
+ Hits          1049     1094      +45     
- Misses           0       19      +19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[nystrom]

@gafter Sorry I forgot to mark the PR a draft, so you were reviewing a bit prematurely as I was working out the semantics. This new version uses the extractor function, if it exists (even if it fails), then uses the struct type. I think this is less prone to user error.

[gafter]

I love the idea of using the type instead of a token, for exactly the reason you mention. I will look at this (from scratch) today or tomorrow.

[gafter]

One thing occurs to me. Extractors in this version pay no attention to the arity of the pattern. So, for example, the following would fail

    AddExpr(_, _, _) => true

even though the struct type has three fields, because the extractor produces two values, not three.

Is this a problem? Should the extractor specify the arity of the tuple it will produce? If so, would that interfere with the use of named tuples?

[nystrom]

Right, one issue with extractors is that you lose all the nice error checking on the subpatterns. I was thinking you could pass in the expected arity to the extractor:

# called for pattern AddExpr(x, y)
function extract(::Type{AddExpr}, ::Val{2}, v)
   return (v.left, v.right,)
end

But, I didn't want to complicate things too much.

In the case of named arguments, you could pass in the desired field names (as Val):

# called for pattern AddExpr(left=x, right=y)
function extract(::Type{AddExpr}, ::Val{:left}, ::Val{::right}, v)
    return (; left = v.left, right = v.right)
end

I fear this is getting much too complicated, however.

[nystrom]

@gafter I've implemented arity support.

[gafter]

My point about the named tuples is that if you have the function specify the arity, then we can handle things like Foo(1, 2, 3) nicely (using the type pattern because the user-specified pattern doesn't match the arity). However, if we extend type patterns to handle "keyword argument"-like things like Foo(1, 2; y=7), we can't easily extend this scheme to handle that very well (what is its arity? two? three? what should the function return?).

I'm probably overthinking this.

[nystrom]

I'm not sure how Foo(1, 2; y=7) works when Foo is just a struct. Would you look for a constructor?
I suppose you could make it equivalent to Foo(1,2) && Foo(y=7).

[gafter]

Please bump the version number to 2.3.0 (in Project.toml) as this adds a feature.

[gafter]

Looks great!