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.

Sign up for GitHub

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

Jump to bottom

Implement design changes for "pattern" Index/Range indexers #34897

Merged
merged 7 commits into from Apr 12, 2019
Merged

Implement design changes for "pattern" Index/Range indexers #34897

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ae2c987
Implement design changes for "pattern" Index/Range indexers
agocke Apr 8, 2019
e654064
Respond to PR comments
agocke Apr 11, 2019
91a5491
Respond to more PR comments
agocke Apr 11, 2019
9a11b9a
Fix method baseline
agocke Apr 11, 2019
735de15
Fully fix baseline
agocke Apr 12, 2019
d90d5dd
Move Substring from being a WellKnownMember to a SpecialMember
agocke Apr 12, 2019
39ee17c
Use static local function
agocke Apr 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
260 changes: 243 additions & 17 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
View file
Open in desktop
Expand Up @@ -7188,7 +7188,18 @@ private BoundExpression BindIndexerAccess(ExpressionSyntax node, BoundExpression

if (!lookupResult.IsMultiViable)
{
indexerAccessExpression = BadIndexerExpression(node, expr, analyzedArguments, lookupResult.Error, diagnostics);
if (TryBindIndexOrRangeIndexer(
node,
expr,
Copy link
Contributor

Choose a reason for hiding this comment

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

We commonly style indexer access expressions as receiver[expr], so naming this expr here seems confusing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, indexers can have multiple arguments, so it would be receiver[arg1, arg2, arg3] so using expr as the argument name just means we're inserting an expression (which could be any of those components).

analyzedArguments.Arguments,
out var patternIndexerAccess))
{
indexerAccessExpression = patternIndexerAccess;
}
else
{
indexerAccessExpression = BadIndexerExpression(node, expr, analyzedArguments, lookupResult.Error, diagnostics);
}
}
else
{
Expand Down Expand Up @@ -7350,22 +7361,33 @@ private BoundExpression BindIndexedPropertyAccess(SyntaxNode syntax, BoundExpres

if (!analyzedArguments.HasErrors)
{
// Dev10 uses the "this" keyword as the method name for indexers.
var candidate = candidates[0];
var name = candidate.IsIndexer ? SyntaxFacts.GetText(SyntaxKind.ThisKeyword) : candidate.Name;

overloadResolutionResult.ReportDiagnostics(
binder: this,
location: syntax.Location,
nodeOpt: syntax,
diagnostics: diagnostics,
name: name,
receiver: null,
invokedExpression: null,
arguments: analyzedArguments,
memberGroup: candidates,
typeContainingConstructor: null,
delegateTypeBeingInvoked: null);
if (TryBindIndexOrRangeIndexer(
syntax,
receiverOpt,
analyzedArguments.Arguments,
out var patternIndexerAccess))
{
return patternIndexerAccess;
}
else
{
// Dev10 uses the "this" keyword as the method name for indexers.
var candidate = candidates[0];
var name = candidate.IsIndexer ? SyntaxFacts.GetText(SyntaxKind.ThisKeyword) : candidate.Name;

overloadResolutionResult.ReportDiagnostics(
binder: this,
location: syntax.Location,
nodeOpt: syntax,
diagnostics: diagnostics,
name: name,
receiver: null,
invokedExpression: null,
arguments: analyzedArguments,
memberGroup: candidates,
typeContainingConstructor: null,
delegateTypeBeingInvoked: null);
}
}

ImmutableArray<BoundExpression> arguments = BuildArgumentsForErrorRecovery(analyzedArguments, candidates);
Expand Down Expand Up @@ -7438,6 +7460,210 @@ private BoundExpression BindIndexedPropertyAccess(SyntaxNode syntax, BoundExpres
return propertyAccess;
}

private bool TryBindIndexOrRangeIndexer(
SyntaxNode syntax,
BoundExpression receiverOpt,
ArrayBuilder<BoundExpression> arguments,
out BoundIndexOrRangePatternIndexerAccess patternIndexerAccess)
{
// Verify a few things up-front, namely that we have a single argument
// to this indexer that has an Index or Range type and that there is
// a real receiver with a known type

if (arguments.Count != 1)
{
patternIndexerAccess = null;
return false;
}

var argType = arguments[0].Type;
bool argIsIndex = TypeSymbol.Equals(argType,
Compilation.GetWellKnownType(WellKnownType.System_Index),
TypeCompareKind.ConsiderEverything);
bool argIsRange = !argIsIndex || TypeSymbol.Equals(argType,
Compilation.GetWellKnownType(WellKnownType.System_Range),
TypeCompareKind.ConsiderEverything);

if ((!argIsIndex && !argIsRange) ||
!(receiverOpt?.Type is TypeSymbol receiverType))
{
patternIndexerAccess = null;
return false;
}

// SPEC:

// An indexer invocation with a single argument of System.Index or System.Range will
// succeed if the receiver type conforms to an appropriate pattern, namely

// 1. The receiver type's original definition has an accessible property getter that returns
// an int and has the name Length or Count
// 2. For Index: Has an accessible indexer with a single int parameter
// For Range: Has an accessible Slice method that takes two int parameters

PropertySymbol lengthOrCountProperty;

var lookupResult = LookupResult.GetInstance();
HashSet<DiagnosticInfo> useSiteDiagnostics = null;

// Look for Length first

if (!tryLookupLengthOrCount(WellKnownMemberNames.LengthPropertyName, out lengthOrCountProperty) &&
!tryLookupLengthOrCount(WellKnownMemberNames.CountPropertyName, out lengthOrCountProperty))
{
patternIndexerAccess = null;
return false;
}

Debug.Assert(lengthOrCountProperty is { });

if (argIsIndex)
{
// Look for `T this[int i]` indexer

LookupMembersInType(
lookupResult,
receiverType,
WellKnownMemberNames.Indexer,
arity: 0,
basesBeingResolved: null,
LookupOptions.Default,
originalBinder: this,
diagnose: false,
ref useSiteDiagnostics);

if (lookupResult.IsMultiViable)
{
foreach (var candidate in lookupResult.Symbols)
{
if (!candidate.IsStatic &&
candidate is PropertySymbol property &&
property.GetOwnOrInheritedGetMethod() is MethodSymbol getMethod &&
agocke marked this conversation as resolved.
Show resolved Hide resolved
IsAccessible(getMethod, ref useSiteDiagnostics) &&
getMethod.OriginalDefinition is var original &&
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved
original.ParameterCount == 1 &&
isIntNotByRef(original.Parameters[0]))
{
patternIndexerAccess = new BoundIndexOrRangePatternIndexerAccess(
syntax,
receiverOpt,
lengthOrCountProperty,
getMethod,
arguments[0],
getMethod.ReturnType);
cleanup(lookupResult, ref useSiteDiagnostics);
return true;
}
}
}
}
else if (receiverType.SpecialType == SpecialType.System_String)
{
Debug.Assert(argIsRange);
// Look for Substring
var substring = (MethodSymbol)Compilation.GetSpecialTypeMember(SpecialMember.System_String__Substring);
if (substring is { })
agocke marked this conversation as resolved.
Show resolved Hide resolved
{
patternIndexerAccess = new BoundIndexOrRangePatternIndexerAccess(
syntax,
receiverOpt,
lengthOrCountProperty,
substring,
arguments[0],
substring.ReturnType);
cleanup(lookupResult, ref useSiteDiagnostics);
return true;
}
}
else
{
Debug.Assert(argIsRange);
// Look for `T Slice(int, int)` indexer

LookupMembersInType(
lookupResult,
receiverType,
WellKnownMemberNames.SliceMethodName,
arity: 0,
basesBeingResolved: null,
LookupOptions.Default,
originalBinder: this,
diagnose: false,
ref useSiteDiagnostics);

if (lookupResult.IsMultiViable)
{
foreach (var candidate in lookupResult.Symbols)
{
if (!candidate.IsStatic &&
IsAccessible(candidate, ref useSiteDiagnostics) &&
candidate is MethodSymbol method &&
method.OriginalDefinition is var original &&
original.ParameterCount == 2 &&
isIntNotByRef(original.Parameters[0]) &&
isIntNotByRef(original.Parameters[1]))
{
patternIndexerAccess = new BoundIndexOrRangePatternIndexerAccess(
syntax,
receiverOpt,
lengthOrCountProperty,
method,
arguments[0],
method.ReturnType);
cleanup(lookupResult, ref useSiteDiagnostics);
return true;
}
}
}
}

cleanup(lookupResult, ref useSiteDiagnostics);
patternIndexerAccess = null;
return false;

static void cleanup(LookupResult lookupResult, ref HashSet<DiagnosticInfo> useSiteDiagnostics)
{
lookupResult.Free();
useSiteDiagnostics = null;
}

static bool isIntNotByRef(ParameterSymbol param)
=> param.Type.SpecialType == SpecialType.System_Int32 &&
param.RefKind == RefKind.None;

bool tryLookupLengthOrCount(string propertyName, out PropertySymbol valid)
{
LookupMembersInType(
lookupResult,
receiverType,
propertyName,
arity: 0,
basesBeingResolved: null,
LookupOptions.Default,
originalBinder: this,
diagnose: false,
useSiteDiagnostics: ref useSiteDiagnostics);

if (lookupResult.IsSingleViable &&
lookupResult.Symbols[0] is PropertySymbol property &&
property.GetOwnOrInheritedGetMethod()?.OriginalDefinition is MethodSymbol getMethod &&
getMethod.ReturnType.SpecialType == SpecialType.System_Int32 &&
getMethod.RefKind == RefKind.None &&
!getMethod.IsStatic &&
IsAccessible(getMethod, ref useSiteDiagnostics))
{
lookupResult.Clear();
useSiteDiagnostics = null;
valid = property;
return true;
}
lookupResult.Clear();
useSiteDiagnostics = null;
valid = null;
return false;
}
}

private ErrorPropertySymbol CreateErrorPropertySymbol(ImmutableArray<PropertySymbol> propertyGroup)
{
TypeSymbol propertyType = GetCommonTypeOrReturnType(propertyGroup) ?? CreateErrorType();
Expand Down
9 changes: 9 additions & 0 deletions src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml
View file
Open in desktop
Expand Up @@ -1744,6 +1744,15 @@
<Field Name="UseSetterForDefaultArgumentGeneration" Type="bool"/>
</Node>

<Node Name="BoundIndexOrRangePatternIndexerAccess" Base="BoundExpression">
<Field Name="Type" Type="TypeSymbol" Override="true" Null="disallow" />

<Field Name="Receiver" Type="BoundExpression" Null="disallow" />
<Field Name="LengthOrCountProperty" Type="PropertySymbol" Null="disallow" />
<Field Name="PatternMethod" Type="MethodSymbol" Null="disallow" />
<Field Name="Argument" Type="BoundExpression" Null="disallow" />
</Node>

<Node Name="BoundDynamicIndexerAccess" Base="BoundExpression">
<!-- Non-null type is required for this node kind -->
<Field Name="Type" Type="TypeSymbol" Override="true" Null="disallow"/>
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/BoundTree/Expression.cs
View file
Open in desktop
Expand Up @@ -169,4 +169,9 @@ internal partial class BoundPassByCopy
{
protected override ImmutableArray<BoundNode> Children => ImmutableArray.Create<BoundNode>(this.Expression);
}

internal partial class BoundIndexOrRangePatternIndexerAccess
{
protected override ImmutableArray<BoundNode> Children => ImmutableArray.Create<BoundNode>(Receiver, Argument);
}
}
16 changes: 16 additions & 0 deletions src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs
View file
Open in desktop
Expand Up @@ -1188,6 +1188,22 @@ public override BoundNode VisitIndexerAccess(BoundIndexerAccess node)
return null;
}

public override BoundNode VisitIndexOrRangePatternIndexerAccess(BoundIndexOrRangePatternIndexerAccess node)
{
// Index or Range pattern indexers evaluate the following in order:
// 1. The receiver
// 1. The Count or Length method off the receiver
// 2. The argument to the access
// 3. The pattern method
VisitRvalue(node.Receiver);
var method = GetReadMethod(node.LengthOrCountProperty);
VisitReceiverAfterCall(node.Receiver, method);
VisitRvalue(node.Argument);
VisitReceiverAfterCall(node.Receiver, node.PatternMethod);

return null;
}

public override BoundNode VisitEventAssignmentOperator(BoundEventAssignmentOperator node)
{
VisitRvalue(node.ReceiverOpt);
Expand Down
14 changes: 14 additions & 0 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
View file
Open in desktop
Expand Up @@ -4935,6 +4935,20 @@ public override BoundNode VisitIndexerAccess(BoundIndexerAccess node)
return null;
}

public override BoundNode VisitIndexOrRangePatternIndexerAccess(BoundIndexOrRangePatternIndexerAccess node)
{
var receiverType = VisitRvalueWithState(node.Receiver);
VisitRvalue(node.Argument);
var patternMethod = node.PatternMethod;
if (!receiverType.HasNullType)
{
patternMethod = (MethodSymbol)AsMemberOfType(receiverType.Type, patternMethod);
}

LvalueResultType = patternMethod.ReturnTypeWithAnnotations;
return null;
}

public override BoundNode VisitEventAccess(BoundEventAccess node)
{
VisitMemberAccess(node, node.ReceiverOpt, node.EventSymbol);
Expand Down