-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Discussion: Active Patterns #277
Comments
This is - by an order of magnitude - my favourite proposal on this repo. 🍾 |
Typically, methods returning Here, odd names (e.g. The above examples could be as easily, and more naturally expressed as extension methods, with proper names:
Also, |
The
By doing so you'd immediately lose all of the benefits of recursive patterns, both in matching the output of the active pattern and in using the active pattern within another pattern. That's the entire point of this proposal. |
I think naming concern is the least important here since we are talking about the general mechanism to be able to define custom patterns. You can group them in a separate class to be not confused with general-purpose extensions. As for your " Another advantage is that |
I have reservations regarding basing active patterns on any public static class StringPatterns {
public static bool operator is Integer(this string input) {
return int.TryParse(input, out var _);
}
public static bool operator is Regex(this string input, string pattern, out string[] captures) {
Match match = Regex.Match(input, pattern);
if (match.Success) {
captures = new string[match.Groups.Count];
for (int i = 0; i < match.Groups.Count; i++) {
captures[i] = match.Groups[i].Value;
}
return true;
}
captures = null;
return false;
}
} That would give the methods a specific shape that would help the compiler/Intellisense narrow down the curated patterns, e.g. With the |
Let's break this apart:
Option 1 has the following disadvantages:
A dedicated syntax instead might better communicate the feature. It could look something like the following:
The above syntax opens the door for other cases as well:
|
That's the responsibility of binding in the use-site, so even with
I don't think introducing a new syntax to address tooling concerns is a sensible approach. An attribute on |
Deconstruction has already taken the shape of the public static class Polar {
public static bool Deconstruct(this Coordinates coordinates, out double r, out double theta) {
r = Math.Sqrt(coordinates.X * coordinates.X + coordinates.Y * coordinates.Y);
theta = Math.Atan2(coordinates.Y, coordinates.X);
return coordinates.X != 0 || coordinates.Y != 0;
}
} But that comes with the following limitations:
That was the impetus behind the referenced proposal on the Roslyn repo. Also, the LDM already considered and dismissed using tuples for deconstruction as that would prevent the ability to overload based on the number of elements. That same problem would apply here.
It's to address tooling and compiler concerns, just as Ultimately those are minor concerns. The bigger questions I think revolve around disambiguating between an expression as input to a pattern vs. recursive constant patterns themselves. See dotnet/roslyn#9005 (comment) |
Regarding dotnet/roslyn#9005 (comment),
After |
|
To make the complaint about allowing any HashSet<int> set = …;
if (set is Add(42)) … This code does not make any sense, |
I'd prefer something like: public static bool operator is Integer(this string str) { .. } rather than a |
@DavidArno My disagreement with an |
Difficult to ague with that! 😀 |
Since |
Or, if we're going the naming-convention route, why not the This will make "Is" methods usable both as extensions, and as patterns. |
The problem with |
The preferred method IMO is a specialized syntax; see my proposal above. |
I agree ... however new syntax is costly to implement. For example, some fancy new syntax around deconstructs would have been nice, but (I'm assuming) it was easier to implement in the messy |
I'm not sure I follow. Arguably The first decision to be made is whether we go with a syntax or a convention. |
It's a question of conjugation. Since patterns already are contained within an |
|
Yes, I realize that. But if the compiler is going to perform such trickery I'd prefer that the shape be more explicit, e.g. a |
With recent updates to recursive patterns this proposal is obsolete. I think we should do it the other way around, i.e. permit a set of expressions (constants) to be parsed as patterns and convert them to expressions after an active pattern is resolved. This is the relevant paragraph from F# spec:
No consideration in the current spec is required to enable this in the future, because constant patterns already cover all the expressions that we need to accept as pattern arguments. |
Active Patterns
Original proposal: dotnet/roslyn#9005
Summary
Lets you define customs patterns to be used in pattern-matching constructs.
Proposal
An active pattern will be declared as a
bool
-returning extension or instance method,An active pattern can be parameterized as well,
You can define output parameters to be matched against other patterns.
In order to be able to use active patterns, we will need to unify pattern and expression syntax. As long as we define the pattern syntax as a subset of expression syntax, there wouldn't be a special parsing rule for patterns and all the other requirements can be checked after this phase.
Note: These are not the exact production rules as they do not represent the precedence.
var
identifiervar!
identifiervar
(
...)
(
argument-list)
[
expression-list]
{
key-value-list}
property pattern
as
identifier|
expression&
expression(
expression)
..
expressionout
identifierThis whole construct can be represented as an "expression pattern" in the syntax tree.
list pattern
It's tempting to use collection initializer's syntax for list patterns but it would be ambigious for an empty list e.g.
() => {}
. However, using brackets as suggested in dotnet/roslyn#6949 can resolve that issue.indexer and property patterns
A key-value is defined as a pair of expressions separated by a colon: expression
:
expression. In a property pattern we require the LHS to be an identifier, and in an indexer pattern we require the LHS be a constant. We decide which pattern is intended if the target expression is pattern compatible with either of patterns. It'd be nice if we could use indexer initializer syntax for indexer patterns:{ [c]: p }
but there is not any corresponding expression form for that syntax since we're using colons.as pattern
As currently proposed, the as pattern variable does not need any token, that in particular makes it hard to keep patterns and expressions' syntax in sync. A disadvantage of using
as
is that in the expression form the RHS is a type and reusing it like this might be confusing. Other languages use sigils like@
which might be preferable overas
.out pattern
Originally proposed in dotnet/roslyn#13400, lets you bind the target value to an existing variable. It's useful to define active patterns that defined as another pattern match.
As for parsing,
out
arguments should be promoted as a general expression just likeref
expressions, but unlikeref
expressions, it would be illegal to use anout
expression outside patterns or argument lists.composite patterns
Note that type, property, indexer and tuple patterns can appear in a single pattern,
We parse that as an object-creation-expression without the
new
keyword. Said construct is proposed as a type invocation in the records proposal. The property pattern part depends on dotnet/roslyn#6949 as it aims to extend object initializers to accept json-like dictionaries.Unresolved questions
What syntax we should be using for the declaration-site? As @HaloFour and @svick pointed out, a regular method can make a lot of meaningless patterns, for instance:
Suggested alternatives
static bool operator is Integer(..)
match
orpattern
[Pattern]
(similar toExtension
in VB)static bool IntegerPattern(..)
static bool IsInteger(..)
Naming-conventions would permit regular usage of said method. The suffix/prefix is required for the method to be used as a pattern but the suffix/prefix would be trimmed in the use-site, e.g.
is Integer(..)
.The text was updated successfully, but these errors were encountered: