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

List patterns: binding and lowering #53299

Merged
merged 43 commits into from
Jun 3, 2021
Merged

List patterns: binding and lowering #53299

Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
fc4539f
WIP
alrz May 10, 2021
2074641
Revert
alrz May 10, 2021
eea5b72
WIP
alrz May 12, 2021
c97dd45
WIP
alrz May 13, 2021
1c9c7dd
Update resources
alrz May 13, 2021
1e060fe
Fixup tests
alrz May 13, 2021
b50d1e5
Fixup
alrz May 13, 2021
10b2329
Verify codegen
alrz May 13, 2021
d1f4931
Rename
alrz May 13, 2021
9f74698
Fixup
alrz May 13, 2021
a6183cd
Cleanup
alrz May 14, 2021
c421256
Address feedback
alrz May 14, 2021
b3580c0
Fixup
alrz May 14, 2021
bddff37
Misc
alrz May 14, 2021
235b333
Typo
alrz May 14, 2021
075e745
Add test
alrz May 14, 2021
4d3f73a
Misc
alrz May 14, 2021
00de503
Fixup
alrz May 14, 2021
53a6659
Revert
alrz May 17, 2021
a7610bd
Fix implicit pattern lookup
alrz May 17, 2021
3da61e9
Address a few comments
alrz May 17, 2021
8247bbc
Remove unused code
alrz May 17, 2021
42e65fd
Revert
alrz May 18, 2021
af9b850
Typo
alrz May 18, 2021
e6bcecd
Move
alrz May 19, 2021
580b508
Add tests
alrz May 19, 2021
c42dbe5
Fixup test
alrz May 19, 2021
794245d
Move
alrz May 19, 2021
b4653c2
Split assertions
alrz May 20, 2021
221805c
Pass IOp verification
alrz May 20, 2021
091d0bf
Remove unused arg
alrz May 25, 2021
f120e17
Address feedback
alrz May 25, 2021
d6b6127
Fixup
alrz May 25, 2021
56c4271
Address feedback
alrz May 27, 2021
835bdc7
Add suggested test
alrz May 27, 2021
f1b9286
Adjust comment
alrz May 27, 2021
e4a5665
Inline
alrz May 27, 2021
e11e345
Add tests
alrz May 28, 2021
edd796d
Typo
alrz May 28, 2021
4ed0275
Renamings
alrz May 28, 2021
4812a57
Reject void Slice
alrz May 28, 2021
4940762
Reflect latest spec changes
alrz May 29, 2021
e6bf9af
Add prototype comments
alrz Jun 2, 2021
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
183 changes: 101 additions & 82 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Reflection;
using System.Runtime.CompilerServices;
Expand Down Expand Up @@ -7843,12 +7844,13 @@ private BoundExpression BindIndexerOrIndexedPropertyAccess(
return propertyAccess;
}

#nullable enable
private bool TryBindIndexOrRangeIndexer(
SyntaxNode syntax,
BoundExpression receiverOpt,
BoundExpression? receiverOpt,
AnalyzedArguments arguments,
BindingDiagnosticBag diagnostics,
out BoundIndexOrRangePatternIndexerAccess patternIndexerAccess)
[NotNullWhen(true)] out BoundIndexOrRangePatternIndexerAccess? patternIndexerAccess)
{
patternIndexerAccess = null;

Expand All @@ -7864,19 +7866,55 @@ private bool TryBindIndexOrRangeIndexer(
var argument = arguments.Arguments[0];

var argType = argument.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) ||
ThreeState argIsIndexNotRange =
TypeSymbol.Equals(argType, Compilation.GetWellKnownType(WellKnownType.System_Index), TypeCompareKind.ConsiderEverything) ? ThreeState.True :
TypeSymbol.Equals(argType, Compilation.GetWellKnownType(WellKnownType.System_Range), TypeCompareKind.ConsiderEverything) ? ThreeState.False :
ThreeState.Unknown;

if (!argIsIndexNotRange.HasValue() ||
!(receiverOpt?.Type is TypeSymbol receiverType))
{
return false;
}

bool argIsIndex = argIsIndexNotRange.Value();
var useSiteInfo = CompoundUseSiteInfo<AssemblySymbol>.Discarded;
if (!TryFindIndexOrRangeIndexerPattern(syntax, receiverOpt, receiverType, argIsIndex: argIsIndex,
out PropertySymbol? lengthOrCountProperty, out Symbol? patternSymbol, diagnostics, ref useSiteInfo))
{
return false;
}

patternIndexerAccess = new BoundIndexOrRangePatternIndexerAccess(
syntax,
receiverOpt,
lengthOrCountProperty,
patternSymbol,
BindToNaturalType(argument, diagnostics),
patternSymbol.GetTypeOrReturnType().Type);

_ = MessageID.IDS_FeatureIndexOperator.CheckFeatureAvailability(diagnostics, syntax);
if (arguments.Names.Count > 0)
{
diagnostics.Add(
argIsIndex
? ErrorCode.ERR_ImplicitIndexIndexerWithName
: ErrorCode.ERR_ImplicitRangeIndexerWithName,
arguments.Names[0].GetLocation());
}
return true;
}

private bool TryFindIndexOrRangeIndexerPattern(
SyntaxNode syntax,
BoundExpression? receiverOpt,
TypeSymbol receiverType,
bool argIsIndex,
[NotNullWhen(true)] out PropertySymbol? lengthOrCountProperty,
[NotNullWhen(true)] out Symbol? patternSymbol,
BindingDiagnosticBag diagnostics,
ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
// SPEC:

// An indexer invocation with a single argument of System.Index or System.Range will
Expand All @@ -7887,21 +7925,31 @@ private bool TryBindIndexOrRangeIndexer(
// 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();
var discardedUseSiteInfo = CompoundUseSiteInfo<AssemblySymbol>.Discarded;

// Look for Length first

if (!tryLookupLengthOrCount(WellKnownMemberNames.LengthPropertyName, out lengthOrCountProperty) &&
!tryLookupLengthOrCount(WellKnownMemberNames.CountPropertyName, out lengthOrCountProperty))
if (TryLookupLengthOrCount(receiverType, lookupResult, out lengthOrCountProperty, ref useSiteInfo) &&
TryFindIndexOrRangeIndexerPattern(syntax, lookupResult, receiverOpt, receiverType, argIsIndex, out patternSymbol, diagnostics, ref useSiteInfo))
{
return false;
CheckImplicitThisCopyInReadOnlyMember(receiverOpt, lengthOrCountProperty.GetMethod, diagnostics);
lookupResult.Free();
return true;
}

Debug.Assert(lengthOrCountProperty is { });
patternSymbol = null;
lookupResult.Free();
return false;
}

private bool TryFindIndexOrRangeIndexerPattern(
SyntaxNode syntax,
LookupResult lookupResult,
BoundExpression? receiverOpt,
TypeSymbol receiverType,
bool argIsIndex,
[NotNullWhen(true)] out Symbol? patternSymbol,
BindingDiagnosticBag diagnostics,
ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
if (argIsIndex)
{
// Look for `T this[int i]` indexer
Expand All @@ -7915,53 +7963,43 @@ private bool TryBindIndexOrRangeIndexer(
LookupOptions.Default,
originalBinder: this,
diagnose: false,
ref discardedUseSiteInfo);
ref useSiteInfo);

if (lookupResult.IsMultiViable)
{
foreach (var candidate in lookupResult.Symbols)
{
if (!candidate.IsStatic &&
candidate is PropertySymbol property &&
IsAccessible(property, ref discardedUseSiteInfo) &&
IsAccessible(property, ref useSiteInfo) &&
property.OriginalDefinition is { ParameterCount: 1 } original &&
isIntNotByRef(original.Parameters[0]))
original.Parameters[0] is { Type: { SpecialType: SpecialType.System_Int32 }, RefKind: RefKind.None })
{
CheckImplicitThisCopyInReadOnlyMember(receiverOpt, lengthOrCountProperty.GetMethod, diagnostics);
// note: implicit copy check on the indexer accessor happens in CheckPropertyValueKind
patternIndexerAccess = new BoundIndexOrRangePatternIndexerAccess(
syntax,
receiverOpt,
lengthOrCountProperty,
property,
BindToNaturalType(argument, diagnostics),
property.Type);
break;
patternSymbol = property;
checkWellKnown(WellKnownMember.System_Index__GetOffset);
return true;
}
}
}
}
else if (receiverType.SpecialType == SpecialType.System_String)
{
Debug.Assert(argIsRange);
Debug.Assert(!argIsIndex);
// Look for Substring
var substring = (MethodSymbol)Compilation.GetSpecialTypeMember(SpecialMember.System_String__Substring);
if (substring is object)
alrz marked this conversation as resolved.
Show resolved Hide resolved
{
patternIndexerAccess = new BoundIndexOrRangePatternIndexerAccess(
syntax,
receiverOpt,
lengthOrCountProperty,
substring,
BindToNaturalType(argument, diagnostics),
substring.ReturnType);
patternSymbol = substring;
checkWellKnown(WellKnownMember.System_Range__get_Start);
checkWellKnown(WellKnownMember.System_Range__get_End);
checkWellKnown(WellKnownMember.System_Index__GetOffset);
return true;
}
}
else
{
Debug.Assert(argIsRange);
Debug.Assert(!argIsIndex);
// Look for `T Slice(int, int)` indexer

LookupMembersInType(
Expand All @@ -7973,63 +8011,33 @@ candidate is PropertySymbol property &&
LookupOptions.Default,
originalBinder: this,
diagnose: false,
ref discardedUseSiteInfo);
ref useSiteInfo);

if (lookupResult.IsMultiViable)
{
foreach (var candidate in lookupResult.Symbols)
{
if (!candidate.IsStatic &&
IsAccessible(candidate, ref discardedUseSiteInfo) &&
IsAccessible(candidate, ref useSiteInfo) &&
candidate is MethodSymbol method &&
method.OriginalDefinition is var original &&
original.ParameterCount == 2 &&
isIntNotByRef(original.Parameters[0]) &&
isIntNotByRef(original.Parameters[1]))
original.Parameters[0] is { Type: { SpecialType: SpecialType.System_Int32 }, RefKind: RefKind.None } &&
original.Parameters[1] is { Type: { SpecialType: SpecialType.System_Int32 }, RefKind: RefKind.None })
{
CheckImplicitThisCopyInReadOnlyMember(receiverOpt, lengthOrCountProperty.GetMethod, diagnostics);
patternSymbol = method;
CheckImplicitThisCopyInReadOnlyMember(receiverOpt, method, diagnostics);
patternIndexerAccess = new BoundIndexOrRangePatternIndexerAccess(
syntax,
receiverOpt,
lengthOrCountProperty,
method,
BindToNaturalType(argument, diagnostics),
method.ReturnType);
checkWellKnown(WellKnownMember.System_Range__get_Start);
checkWellKnown(WellKnownMember.System_Range__get_End);
break;
checkWellKnown(WellKnownMember.System_Index__GetOffset);
return true;
}
}
}
}

cleanup(lookupResult);
if (patternIndexerAccess is null)
{
return false;
}

_ = MessageID.IDS_FeatureIndexOperator.CheckFeatureAvailability(diagnostics, syntax);
checkWellKnown(WellKnownMember.System_Index__GetOffset);
if (arguments.Names.Count > 0)
{
diagnostics.Add(
argIsRange
? ErrorCode.ERR_ImplicitRangeIndexerWithName
: ErrorCode.ERR_ImplicitIndexIndexerWithName,
arguments.Names[0].GetLocation());
}
return true;

static void cleanup(LookupResult lookupResult)
{
lookupResult.Free();
}

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

void checkWellKnown(WellKnownMember member)
{
Expand All @@ -8038,8 +8046,18 @@ void checkWellKnown(WellKnownMember member)
// the user from getting surprising errors when optimizations fail
_ = GetWellKnownTypeMember(member, diagnostics, syntax: syntax);
}
}

private bool TryLookupLengthOrCount(
TypeSymbol receiverType,
LookupResult lookupResult,
[NotNullWhen(true)] out PropertySymbol? lengthOrCountProperty,
ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
return tryLookupLengthOrCount(WellKnownMemberNames.LengthPropertyName, out lengthOrCountProperty, ref useSiteInfo) ||
tryLookupLengthOrCount(WellKnownMemberNames.CountPropertyName, out lengthOrCountProperty, ref useSiteInfo);

bool tryLookupLengthOrCount(string propertyName, out PropertySymbol valid)
bool tryLookupLengthOrCount(string propertyName, out PropertySymbol? valid, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
LookupMembersInType(
lookupResult,
Expand All @@ -8050,15 +8068,15 @@ bool tryLookupLengthOrCount(string propertyName, out PropertySymbol valid)
LookupOptions.Default,
originalBinder: this,
diagnose: false,
useSiteInfo: ref discardedUseSiteInfo);
useSiteInfo: ref useSiteInfo);
alrz marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

useSiteInfo

Consider adding an assert about whether diagnostics and dependencies are being accumulated, or we'll eventually have an issue when a refactor is performed.

Copy link
Member Author

Choose a reason for hiding this comment

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

We've been passing Discard here for ranges too, I followed suit. I can add a prototype comment to revise.


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 discardedUseSiteInfo))
IsAccessible(getMethod, ref useSiteInfo))
{
lookupResult.Clear();
valid = property;
Expand All @@ -8069,6 +8087,7 @@ lookupResult.Symbols[0] is PropertySymbol property &&
return false;
}
}
#nullable disable

private ErrorPropertySymbol CreateErrorPropertySymbol(ImmutableArray<PropertySymbol> propertyGroup)
{
Expand Down
Loading