Skip to content
Permalink
Browse files

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

Implements most of the design changes specified in
https://github.com/dotnet/csharplang/blob/c229cae634bd59a6a13b9ed464a4cab782a95e5d/proposals/index-range-changes.md

This PR focuses on getting the simple end-to-end scenario working,
not focusing entirely on codegen quality. I expect to follow-up later
with the optimizations mentioned about eliminating use of the
Index/Range helpers entirely if they can be elided.
  • Loading branch information...
agocke committed Apr 12, 2019
1 parent bb94e70 commit 357fa7e9dc82e5c8c2ca49e3033362e90ac9f287
Showing with 1,355 additions and 130 deletions.
  1. +243 −17 src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
  2. +9 −0 src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml
  3. +5 −0 src/Compilers/CSharp/Portable/BoundTree/Expression.cs
  4. +16 −0 src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs
  5. +14 −0 src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
  6. +75 −0 src/Compilers/CSharp/Portable/Generated/BoundNodes.xml.Generated.cs
  7. +105 −1 src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_IndexerAccess.cs
  8. +5 −0 src/Compilers/CSharp/Portable/Lowering/SyntheticBoundNodeFactory.cs
  9. +1 −0 src/Compilers/CSharp/Portable/Operations/CSharpOperationFactory.cs
  10. +366 −46 src/Compilers/CSharp/Test/Emit/CodeGen/IndexAndRangeTests.cs
  11. +96 −0 ...ilers/CSharp/Test/IOperation/IOperation/IOperationTests_IFromEndIndexOperation_IRangeOperation.cs
  12. +326 −28 src/Compilers/CSharp/Test/Semantic/Semantics/IndexAndRangeTests.cs
  13. +2 −3 src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs
  14. +39 −0 src/Compilers/CSharp/Test/Symbol/Compilation/SemanticModelAPITests.cs
  15. +2 −0 src/Compilers/CSharp/Test/Symbol/Symbols/MissingSpecialMember.cs
  16. +3 −0 src/Compilers/Core/Portable/PublicAPI.Unshipped.txt
  17. +1 −0 src/Compilers/Core/Portable/SpecialMember.cs
  18. +10 −0 src/Compilers/Core/Portable/SpecialMembers.cs
  19. +15 −0 src/Compilers/Core/Portable/Symbols/WellKnownMemberNames.cs
  20. +2 −0 src/Compilers/Core/Portable/WellKnownMember.cs
  21. +16 −0 src/Compilers/Core/Portable/WellKnownMembers.cs
  22. +0 −35 src/Compilers/Test/Utilities/CSharp/TestSources.cs
  23. +4 −0 src/Compilers/VisualBasic/Test/Symbol/SymbolsTests/WellKnownTypeValidationTests.vb
@@ -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,
analyzedArguments.Arguments,
out var patternIndexerAccess))
{
indexerAccessExpression = patternIndexerAccess;
}
else
{
indexerAccessExpression = BadIndexerExpression(node, expr, analyzedArguments, lookupResult.Error, diagnostics);
}
}
else
{
@@ -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);
@@ -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 &&
IsAccessible(getMethod, ref useSiteDiagnostics) &&
getMethod.OriginalDefinition is var original &&
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 { })
{
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();
@@ -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"/>
@@ -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);
}
}
@@ -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);
@@ -4941,6 +4941,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);
Oops, something went wrong.

0 comments on commit 357fa7e

Please sign in to comment.
You can’t perform that action at this time.