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

Implement pattern-matching via ITuple #28859

Merged
merged 7 commits into from
Aug 14, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ private BoundExpression MakeDeconstructInvocationExpression(
outVars.Add(variable);
}

const string methodName = "Deconstruct";
const string methodName = WellKnownMemberNames.DeconstructMethodName;
var memberAccess = BindInstanceMemberAccess(
rightSyntax, receiverSyntax, receiver, methodName, rightArity: 0,
typeArgumentsSyntax: default(SeparatedSyntaxList<TypeSyntax>), typeArguments: default(ImmutableArray<TypeSymbol>),
Expand Down
149 changes: 146 additions & 3 deletions src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,6 @@ private void BindPatternDesignation(
default:
throw ExceptionUtilities.UnexpectedValue(designation.Kind());
}

}

TypeSymbol BindRecursivePatternType(TypeSyntax typeSyntax, TypeSymbol inputType, DiagnosticBag diagnostics, ref bool hasErrors, out BoundTypeExpression boundDeclType)
Expand Down Expand Up @@ -513,6 +512,11 @@ private BoundPattern BindRecursivePattern(RecursivePatternSyntax node, TypeSymbo
TypeSyntax typeSyntax = node.Type;
TypeSymbol declType = BindRecursivePatternType(typeSyntax, inputType, diagnostics, ref hasErrors, out BoundTypeExpression boundDeclType);

if (ShouldUseITuple(node, declType, diagnostics, out NamedTypeSymbol iTupleType, out MethodSymbol iTupleGetLength, out MethodSymbol iTupleGetItem))
{
return BindITuplePattern(node, inputType, iTupleType, iTupleGetLength, iTupleGetItem, hasErrors, diagnostics);
}

MethodSymbol deconstructMethod = null;
ImmutableArray<BoundSubpattern> deconstructionSubpatterns = default;
if (node.DeconstructionPatternClause != null)
Expand All @@ -532,6 +536,37 @@ private BoundPattern BindRecursivePattern(RecursivePatternSyntax node, TypeSymbo
deconstruction: deconstructionSubpatterns, properties: properties, variable: variableSymbol, variableAccess: variableAccess, hasErrors: hasErrors);
}

private BoundPattern BindITuplePattern(
RecursivePatternSyntax node,
TypeSymbol inputType,
NamedTypeSymbol iTupleType,
MethodSymbol iTupleGetLength,
MethodSymbol iTupleGetItem,
bool hasErrors,
DiagnosticBag diagnostics)
{
var objectType = Compilation.GetSpecialType(SpecialType.System_Object);
var deconstruction = node.DeconstructionPatternClause;
var patterns = ArrayBuilder<BoundSubpattern>.GetInstance(deconstruction.Subpatterns.Count);
foreach (var subpatternSyntax in deconstruction.Subpatterns)
{
TypeSymbol elementType = objectType;
if (subpatternSyntax.NameColon != null)
{
// error: name not permitted in ITuple deconstruction
diagnostics.Add(ErrorCode.ERR_ArgumentNameInITuplePattern, subpatternSyntax.NameColon.Location);
Copy link
Member

@cston cston Aug 1, 2018

Choose a reason for hiding this comment

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

diagnostics.Add(ErrorCode.ERR_ArgumentNameInITuplePattern, subpatternSyntax.NameColon.Location); [](start = 20, length = 96)

Please add a test for this case. #Resolved

}

var boundSubpattern = new BoundSubpattern(
subpatternSyntax,
null,
BindPattern(subpatternSyntax.Pattern, elementType, false, diagnostics));
patterns.Add(boundSubpattern);
}

return new BoundITuplePattern(node, iTupleGetLength, iTupleGetItem, patterns.ToImmutableAndFree(), inputType, hasErrors);
}

private ImmutableArray<BoundSubpattern> BindDeconstructionPatternClause(
DeconstructionPatternClauseSyntax node,
TypeSymbol declType,
Expand All @@ -551,6 +586,7 @@ private ImmutableArray<BoundSubpattern> BindDeconstructionPatternClause(
diagnostics.Add(ErrorCode.ERR_WrongNumberOfSubpatterns, location, declType, elementTypes.Length, node.Subpatterns.Count);
hasErrors = true;
}

for (int i = 0; i < node.Subpatterns.Count; i++)
{
var subpatternSyntax = node.Subpatterns[i];
Expand All @@ -572,7 +608,7 @@ private ImmutableArray<BoundSubpattern> BindDeconstructionPatternClause(
}
else
{
Copy link
Member

@cston cston Aug 1, 2018

Choose a reason for hiding this comment

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

Consider keeping else with a single return patterns.ToImmutableAndFree(); outside of the if / else. #Resolved

// It is not a tuple type. Seek an appropriate Deconstruct method.
// It is not a tuple type or ITuple. Seek an appropriate Deconstruct method.
var inputPlaceholder = new BoundImplicitReceiver(node, declType); // A fake receiver expression to permit us to reuse binding logic
BoundExpression deconstruct = MakeDeconstructInvocationExpression(
node.Subpatterns.Count, inputPlaceholder, node, diagnostics, outPlaceholders: out ImmutableArray<BoundDeconstructValuePlaceholder> outPlaceholders);
Expand Down Expand Up @@ -608,7 +644,8 @@ private ImmutableArray<BoundSubpattern> BindDeconstructionPatternClause(
Debug.Assert(deconstructMethod is ErrorMethodSymbol);
}
}
BoundSubpattern boundSubpattern = new BoundSubpattern(

var boundSubpattern = new BoundSubpattern(
subPattern,
parameter,
BindPattern(subPattern.Pattern, elementType, isError, diagnostics)
Expand All @@ -620,6 +657,112 @@ private ImmutableArray<BoundSubpattern> BindDeconstructionPatternClause(
return patterns.ToImmutableAndFree();
}

private bool ShouldUseITuple(
RecursivePatternSyntax node,
TypeSymbol declType,
DiagnosticBag diagnostics,
out NamedTypeSymbol iTupleType,
out MethodSymbol iTupleGetLength,
out MethodSymbol iTupleGetItem)
{
iTupleType = null;
iTupleGetLength = iTupleGetItem = null;
if (declType.IsTupleType)
{
return false;
}

if (node.Type != null)
{
// ITuple matching only applies if no type is given explicitly.
return false;
}

if (node.PropertyPatternClause != null)
{
// ITuple matching only applies if there is no property pattern part.
return false;
}

if (node.DeconstructionPatternClause == null)
{
// ITuple matching only applies if there is a deconstruction pattern part.
return false;
}

if (node.Designation != null)
{
// ITuple matching only applies if there is no designation (what type would the designation be?)
return false;
}

if (Compilation.LanguageVersion < MessageID.IDS_FeatureRecursivePatterns.RequiredVersion())
{
return false;
}

iTupleType = Compilation.GetWellKnownType(WellKnownType.System_Runtime_CompilerServices_ITuple);
if (iTupleType.TypeKind != TypeKind.Interface)
{
// When compiling to a platform that lacks the interface ITuple (i.e. it is an error type), we simply do not match using it.
return false;
}

// Resolution 2017-11-20 LDM: permit matching via ITuple only for `object`, `ITuple`, and types that are
// declared to implement `ITuple` but contain no `Deconstruct` methods.
if (declType != (object)Compilation.GetSpecialType(SpecialType.System_Object) &&
declType != (object)Compilation.DynamicType &&
declType != (object)iTupleType &&
Copy link
Member

@jcouv jcouv Jul 30, 2018

Choose a reason for hiding this comment

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

!= [](start = 25, length = 2)

I'm surprised we're using reference equality. Shouldn't we be using symbol equality (.Equals)? #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

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

They are the same for "original" type symbols, which all of these are.


In reply to: 206291161 [](ancestors = 206291161)

!hasBaseInterface(declType, iTupleType))
{
return false;
}

// If there are any accessible Deconstruct method members, then we do not permit ITuple
var lookupResult = LookupResult.GetInstance();
try
{
const LookupOptions options = LookupOptions.MustBeInvocableIfMember | LookupOptions.AllMethodsOnArityZero;
HashSet<DiagnosticInfo> useSiteDiagnostics = null;
this.LookupMembersWithFallback(lookupResult, declType, WellKnownMemberNames.DeconstructMethodName, arity: 0, ref useSiteDiagnostics, basesBeingResolved: null, options: options);
diagnostics.Add(node, useSiteDiagnostics);
if (lookupResult.IsMultiViable)
{
foreach (var symbol in lookupResult.Symbols)
{
if (symbol.Kind == SymbolKind.Method)
{
return false;
}
}
}
}
finally
{
lookupResult.Free();
}

// Ensure ITuple has a Length and indexer
iTupleGetLength = (MethodSymbol)Compilation.GetWellKnownTypeMember(WellKnownMember.System_Runtime_CompilerServices_ITuple__get_Length);
iTupleGetItem = (MethodSymbol)Compilation.GetWellKnownTypeMember(WellKnownMember.System_Runtime_CompilerServices_ITuple__get_Item);
if (iTupleGetLength is null || iTupleGetItem is null)
{
// This might not result in an ideal diagnostic
return false;
}

// passed all the filters; permit using ITuple
return true;

bool hasBaseInterface(TypeSymbol type, NamedTypeSymbol possibleBaseInterface)
{
HashSet<DiagnosticInfo> useSiteDiagnostics = null;
var result = Compilation.Conversions.ClassifyBuiltInConversion(type, possibleBaseInterface, ref useSiteDiagnostics).IsImplicit;
diagnostics.Add(node, useSiteDiagnostics);
return result;
}
}

/// <summary>
/// Check that the given name designates a tuple element at the given index, and return that element.
/// </summary>
Expand Down
37 changes: 37 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -285,11 +285,48 @@ private void MakeTestsAndBindings(
case BoundRecursivePattern recursive:
MakeTestsAndBindings(input, recursive, tests, bindings);
break;
case BoundITuplePattern iTuple:
MakeTestsAndBindings(input, iTuple, tests, bindings);
break;
default:
throw new NotImplementedException(pattern.Kind.ToString());
}
}

private void MakeTestsAndBindings(
BoundDagTemp input,
BoundITuplePattern pattern,
ArrayBuilder<BoundDagTest> tests,
ArrayBuilder<BoundPatternBinding> bindings)
{
var syntax = pattern.Syntax;
var patternLength = pattern.Subpatterns.Length;
var objectType = this._compilation.GetSpecialType(SpecialType.System_Object);
var getLengthProperty = (PropertySymbol)pattern.GetLengthMethod.AssociatedSymbol;
Debug.Assert(getLengthProperty.Type.SpecialType == SpecialType.System_Int32);
var getItemProperty = (PropertySymbol)pattern.GetItemMethod.AssociatedSymbol;
var iTupleType = getLengthProperty.ContainingType;
Debug.Assert(iTupleType.Name == "ITuple");

tests.Add(new BoundDagTypeTest(syntax, iTupleType, input));
var valueAsITupleEvaluation = new BoundDagTypeEvaluation(syntax, iTupleType, input);
tests.Add(valueAsITupleEvaluation);
var valueAsITuple = new BoundDagTemp(syntax, iTupleType, valueAsITupleEvaluation, 0);

var lengthEvaluation = new BoundDagPropertyEvaluation(syntax, getLengthProperty, valueAsITuple);
tests.Add(lengthEvaluation);
var lengthTemp = new BoundDagTemp(syntax, this._compilation.GetSpecialType(SpecialType.System_Int32), lengthEvaluation, 0);
tests.Add(new BoundDagValueTest(syntax, ConstantValue.Create(patternLength), lengthTemp));

for (int i = 0; i < patternLength; i++)
{
var indexEvaluation = new BoundDagIndexEvaluation(syntax, getItemProperty, i, valueAsITuple);
tests.Add(indexEvaluation);
var indexTemp = new BoundDagTemp(syntax, objectType, indexEvaluation, 0);
MakeTestsAndBindings(indexTemp, pattern.Subpatterns[i].Pattern, tests, bindings);
}
}

private void MakeTestsAndBindings(
BoundDagTemp input,
BoundDeclarationPattern declaration,
Expand Down
16 changes: 14 additions & 2 deletions src/Compilers/CSharp/Portable/BoundTree/BoundDagEvaluation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace Microsoft.CodeAnalysis.CSharp
partial class BoundDagEvaluation
{
public override bool Equals(object obj) => obj is BoundDagEvaluation other && this.Equals(other);
public bool Equals(BoundDagEvaluation other)
public virtual bool Equals(BoundDagEvaluation other)
{
return other != (object)null && this.Kind == other.Kind && this.Input.Equals(other.Input) && this.Symbol == other.Symbol;
}
Expand All @@ -21,6 +21,7 @@ private Symbol Symbol
case BoundDagPropertyEvaluation e: return e.Property;
case BoundDagTypeEvaluation e: return e.Type;
case BoundDagDeconstructEvaluation e: return e.DeconstructMethod;
case BoundDagIndexEvaluation e: return e.Property;
default: throw ExceptionUtilities.UnexpectedValue(this.Kind);
}
}
Expand All @@ -31,11 +32,22 @@ public override int GetHashCode()
}
public static bool operator ==(BoundDagEvaluation left, BoundDagEvaluation right)
{
return (left == (object)null) ? right == (object)null : left.Equals(right);
return (left is null) ? right is null : left.Equals(right);
}
public static bool operator !=(BoundDagEvaluation left, BoundDagEvaluation right)
{
return !(left == right);
}
}

partial class BoundDagIndexEvaluation
{
public override int GetHashCode() => base.GetHashCode() ^ this.Index;
public override bool Equals(BoundDagEvaluation obj)
{
return base.Equals(obj) &&
// base.Equals checks the kind field, so the following cast is safe
this.Index == ((BoundDagIndexEvaluation)obj).Index;
}
}
}
18 changes: 12 additions & 6 deletions src/Compilers/CSharp/Portable/BoundTree/BoundDecisionDag.cs
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ string tempName(BoundDagTemp t)
}
break;
case BoundWhenDecisionDagNode node:
result.AppendLine($" WhenClause: " + node.WhenExpression.Syntax);
result.AppendLine($" WhenClause: " + node.WhenExpression?.Syntax);
if (node.WhenTrue != null)
{
result.AppendLine($" WhenTrue: {stateIdentifierMap[node.WhenTrue]}");
Expand All @@ -253,6 +253,8 @@ string tempName(BoundDagTemp t)
case BoundLeafDecisionDagNode node:
result.AppendLine($" Case: " + node.Syntax);
break;
default:
throw ExceptionUtilities.UnexpectedValue(state);
}
}

Expand All @@ -265,15 +267,19 @@ string dump(BoundDagTest d)
switch (d)
{
case BoundDagTypeEvaluation a:
return $"t{tempIdentifier(a)}={a.Kind}({a.Type.ToString()})";
return $"t{tempIdentifier(a)}={a.Kind} {tempName(d.Input)} as {a.Type.ToString()}";
case BoundDagPropertyEvaluation e:
return $"t{tempIdentifier(e)}={e.Kind} {tempName(d.Input)}.{e.Property.Name}";
case BoundDagIndexEvaluation i:
return $"t{tempIdentifier(i)}={i.Kind} {tempName(d.Input)}[{i.Index}]";
case BoundDagEvaluation e:
return $"t{tempIdentifier(e)}={e.Kind}";
return $"t{tempIdentifier(e)}={e.Kind}, {tempName(d.Input)}";
case BoundDagTypeTest b:
return $"?{d.Kind}({b.Type.ToString()}, {tempName(d.Input)})";
return $"?{d.Kind} {tempName(d.Input)} is {b.Type.ToString()}";
case BoundDagValueTest v:
return $"?{d.Kind}({v.Value.ToString()}, {tempName(d.Input)})";
return $"?{d.Kind} {v.Value.ToString()} == {tempName(d.Input)}";
default:
return $"?{d.Kind}({tempName(d.Input)})";
throw ExceptionUtilities.UnexpectedValue(d);
}
}
}
Expand Down
10 changes: 10 additions & 0 deletions src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1177,6 +1177,10 @@
<Node Name="BoundDagPropertyEvaluation" Base="BoundDagEvaluation">
<Field Name="Property" Type="PropertySymbol" Null="disallow"/>
</Node>
<Node Name="BoundDagIndexEvaluation" Base="BoundDagEvaluation">
Copy link
Member

@jcouv jcouv Jul 26, 2018

Choose a reason for hiding this comment

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

BoundDagIndexEvaluation [](start = 14, length = 23)

Do we need to customize the Display property for the new bound nodes (in Formatting.cs)? #Closed

Copy link
Member Author

@gafter gafter Jul 26, 2018

Choose a reason for hiding this comment

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

No, that is for nodes that extend BoundExpression but have no type. #Closed

<Field Name="Property" Type="PropertySymbol" Null="disallow"/>
<Field Name="Index" Type="int"/>
</Node>

<Node Name="BoundSwitchSection" Base="BoundStatementList">
<Field Name="Locals" Type="ImmutableArray&lt;LocalSymbol&gt;"/>
Expand Down Expand Up @@ -1778,6 +1782,12 @@
<Field Name="VariableAccess" Type="BoundExpression" Null="allow"/>
</Node>

<Node Name="BoundITuplePattern" Base="BoundPattern">
<Field Name="GetLengthMethod" Type="MethodSymbol" Null="disallow"/>
<Field Name="GetItemMethod" Type="MethodSymbol" Null="disallow"/>
<Field Name="Subpatterns" Type="ImmutableArray&lt;BoundSubpattern&gt;" Null="disallow"/>
</Node>

<!-- A subpattern, either in a positional or property part of a recursive pattern. -->
<Node Name="BoundSubpattern" Base="BoundNode">
<!-- The tuple element or parameter in a positional pattern, or the property or field in a property pattern. -->
Expand Down
9 changes: 9 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -5420,4 +5420,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_PointerTypeInPatternMatching" xml:space="preserve">
<value>Pattern-matching is not permitted for pointer types.</value>
</data>
<data name="ERR_ArgumentNameInITuplePattern" xml:space="preserve">
<value>Element names are not permitted when pattern-matching via 'System.Runtime.CompilerServices.ITuple'.</value>
</data>
</root>
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1606,6 +1606,7 @@ internal enum ErrorCode
WRN_GivenExpressionNeverMatchesPattern = 8419,
WRN_GivenExpressionAlwaysMatchesConstant = 8420,
ERR_PointerTypeInPatternMatching = 8421,
ERR_ArgumentNameInITuplePattern = 8422,
#endregion diagnostics introduced for recursive patterns

}
Expand Down
Loading