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: factor binding logic #57318

Merged
merged 33 commits into from
Nov 20, 2021
Merged

List-patterns: factor binding logic #57318

Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
025f398
Address some PROTOTYPE markers
jcouv Oct 20, 2021
00e6ff2
Address feedback
jcouv Oct 26, 2021
b1112ce
Merge remote-tracking branch 'dotnet/features/list-patterns' into lis…
jcouv Oct 27, 2021
8d16d55
Rename
jcouv Nov 1, 2021
05f3611
More rename
jcouv Nov 1, 2021
1c3417a
fix incorrect rename
jcouv Nov 1, 2021
51be8eb
Revert most of the renaming
jcouv Nov 2, 2021
c3f1618
Use-site info
jcouv Nov 2, 2021
ddb617c
Merge remote-tracking branch 'dotnet/features/list-patterns' into lis…
jcouv Nov 8, 2021
d69d920
WIP on refactoring
jcouv Nov 2, 2021
39ea1f6
update tests
jcouv Nov 8, 2021
91a7ffa
CheckValue on placeholder
jcouv Nov 8, 2021
7f36c4f
Merge remote-tracking branch 'dotnet/features/list-patterns' into lis…
jcouv Nov 11, 2021
0f924db
Redo lowering change. Store receiver in IndexerAccess
jcouv Nov 11, 2021
d1288ed
Address feedback
jcouv Nov 12, 2021
adfee13
Address feedback (2)
jcouv Nov 13, 2021
dbdbe39
Address feedback (3)
jcouv Nov 15, 2021
a1a4243
Disable caching that is not necessary during a list pattern evaluation.
AlekseyTs Nov 17, 2021
c9f67d1
Merge remote-tracking branch 'features/list-patterns' into ImplicitIn…
AlekseyTs Nov 17, 2021
05edc54
Address feedback (4)
jcouv Nov 17, 2021
1912f37
Fix NullabilityRewriter
jcouv Nov 17, 2021
cd501b4
tweaks
jcouv Nov 17, 2021
aab5d74
compiler generated
jcouv Nov 17, 2021
8be23fa
or
jcouv Nov 17, 2021
6c132fa
Fix order of evaluation for nullability
jcouv Nov 17, 2021
a4ba54b
Clean up handling of an implicit indexer as a target of an assignment…
AlekseyTs Nov 18, 2021
dff82c5
Address feedback (5)
jcouv Nov 18, 2021
9def40b
Improve SemanticModel handling of receiver under BoundIndexOrRangePat…
AlekseyTs Nov 18, 2021
2965f6d
Check implicit copy directly
jcouv Nov 18, 2021
dea39c5
Address feedback (6)
jcouv Nov 19, 2021
174d3f2
Fix 2 test baselines with missing members
jcouv Nov 19, 2021
bcc3336
tweak
jcouv Nov 20, 2021
76d6d7b
Rename to ImplicitIndexer and IndexerOrSliceAccess
jcouv Nov 20, 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
148 changes: 54 additions & 94 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,16 +399,8 @@ internal bool CheckValueKind(SyntaxNode node, BoundExpression expr, BindValueKin
return CheckPropertyValueKind(node, expr, valueKind, checkingReceiver, diagnostics);

case BoundKind.IndexOrRangePatternIndexerAccess:
var patternIndexer = ((BoundIndexOrRangePatternIndexerAccess)expr);
if (patternIndexer.PatternSymbol.Kind == SymbolKind.Property)
{
// If this is an Index indexer, PatternSymbol should be a property, pointing to the
// pattern indexer. If it's a Range access, it will be a method, pointing to a Slice method
// and it's handled below as part of invocations.
return CheckPropertyValueKind(node, expr, valueKind, checkingReceiver, diagnostics);
}
Debug.Assert(patternIndexer.PatternSymbol.Kind == SymbolKind.Method);
break;
var implicitIndexer = (BoundIndexOrRangePatternIndexerAccess)expr;
return CheckValueKind(node, implicitIndexer.IndexerAccess, valueKind, checkingReceiver, diagnostics);

case BoundKind.EventAccess:
return CheckEventValueKind((BoundEventAccess)expr, valueKind, diagnostics);
Expand Down Expand Up @@ -590,16 +582,13 @@ internal bool CheckValueKind(SyntaxNode node, BoundExpression expr, BindValueKin
diagnostics);

case BoundKind.IndexOrRangePatternIndexerAccess:
var patternIndexer = (BoundIndexOrRangePatternIndexerAccess)expr;
// If we got here this should be a pattern indexer taking a Range,
// meaning that the pattern symbol must be a method (either Slice or Substring)
return CheckMethodReturnValueKind(
(MethodSymbol)patternIndexer.PatternSymbol,
patternIndexer.Syntax,
node,
valueKind,
checkingReceiver,
diagnostics);
throw ExceptionUtilities.Unreachable;

case BoundKind.IndexOrRangeIndexerPatternReceiverPlaceholder:
break;

case BoundKind.DeconstructValuePlaceholder:
break;
Copy link
Contributor

@AlekseyTs AlekseyTs Nov 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

break;

Is this code path covered by tests? It is not obvious why returning false is the right thing to do for it. #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

It's covered by DeconstructRefExtensionMethod. The cases the only need to use the expression as an r-value are handled above (line 418, RequiresRValueOnly case).


case BoundKind.ConditionalOperator:
var conditional = (BoundConditionalOperator)expr;
Expand All @@ -624,6 +613,10 @@ internal bool CheckValueKind(SyntaxNode node, BoundExpression expr, BindValueKin
case BoundKind.AssignmentOperator:
var assignment = (BoundAssignmentOperator)expr;
return CheckSimpleAssignmentValueKind(node, assignment, valueKind, diagnostics);

default:
Debug.Assert(expr is not BoundValuePlaceholderBase, $"Placeholder kind {expr.Kind} should be explicitly handled");
break;
}

// At this point we should have covered all the possible cases for anything that is not a strict RValue.
Expand Down Expand Up @@ -2348,42 +2341,9 @@ internal static bool CheckRefEscape(SyntaxNode node, BoundExpression expr, uint
isRefEscape: true);

case BoundKind.IndexOrRangePatternIndexerAccess:
var patternIndexer = (BoundIndexOrRangePatternIndexerAccess)expr;
RefKind refKind;
ImmutableArray<ParameterSymbol> parameters;

switch (patternIndexer.PatternSymbol)
{
case PropertySymbol p:
refKind = p.RefKind;
parameters = p.Parameters;
break;
case MethodSymbol m:
refKind = m.RefKind;
parameters = m.Parameters;
break;
default:
throw ExceptionUtilities.Unreachable;
}

if (refKind == RefKind.None)
{
break;
}

return CheckInvocationEscape(
patternIndexer.Syntax,
patternIndexer.PatternSymbol,
patternIndexer.Receiver,
parameters,
ImmutableArray.Create<BoundExpression>(patternIndexer.Argument),
default,
default,
checkingReceiver,
escapeFrom,
escapeTo,
diagnostics,
isRefEscape: true);
var implicitIndexerAccess = (BoundIndexOrRangePatternIndexerAccess)expr;
// Note: the LengthOrCountAccess use is purely local
return CheckRefEscape(node, implicitIndexerAccess.IndexerAccess, escapeFrom, escapeTo, checkingReceiver, diagnostics);

case BoundKind.FunctionPointerInvocation:
var functionPointerInvocation = (BoundFunctionPointerInvocation)expr;
Expand Down Expand Up @@ -2618,24 +2578,19 @@ internal static uint GetValEscape(BoundExpression expr, uint scopeOfTheContainin
scopeOfTheContainingExpression,
isRefEscape: false);

case BoundKind.IndexOrRangePatternIndexerAccess:
var patternIndexer = (BoundIndexOrRangePatternIndexerAccess)expr;
var parameters = patternIndexer.PatternSymbol switch
{
PropertySymbol p => p.Parameters,
MethodSymbol m => m.Parameters,
_ => throw ExceptionUtilities.UnexpectedValue(patternIndexer.PatternSymbol)
};
case BoundKind.IndexOrRangeIndexerPatternReceiverPlaceholder:
return ((BoundIndexOrRangeIndexerPatternReceiverPlaceholder)expr).ValEscape;

return GetInvocationEscapeScope(
patternIndexer.PatternSymbol,
patternIndexer.Receiver,
parameters,
default,
default,
default,
scopeOfTheContainingExpression,
isRefEscape: false);
case BoundKind.ListPatternReceiverPlaceholder:
return ((BoundListPatternReceiverPlaceholder)expr).ValEscape;

case BoundKind.SlicePatternReceiverPlaceholder:
return ((BoundSlicePatternReceiverPlaceholder)expr).ValEscape;

case BoundKind.IndexOrRangePatternIndexerAccess:
var implicitIndexerAccess = (BoundIndexOrRangePatternIndexerAccess)expr;
// Note: the LengthOrCountAccess use is purely local
return GetValEscape(implicitIndexerAccess.IndexerAccess, scopeOfTheContainingExpression);

case BoundKind.PropertyAccess:
var propertyAccess = (BoundPropertyAccess)expr;
Expand Down Expand Up @@ -3040,29 +2995,34 @@ internal static bool CheckValEscape(SyntaxNode node, BoundExpression expr, uint
diagnostics,
isRefEscape: false);

case BoundKind.IndexOrRangePatternIndexerAccess:
var patternIndexer = (BoundIndexOrRangePatternIndexerAccess)expr;
var patternSymbol = patternIndexer.PatternSymbol;
var parameters = patternSymbol switch
case BoundKind.IndexOrRangeIndexerPatternReceiverPlaceholder:
if (((BoundIndexOrRangeIndexerPatternReceiverPlaceholder)expr).ValEscape > escapeTo)
{
PropertySymbol p => p.Parameters,
MethodSymbol m => m.Parameters,
_ => throw ExceptionUtilities.Unreachable,
};
Error(diagnostics, ErrorCode.ERR_EscapeLocal, node, expr.Syntax);
return false;
}
return true;

return CheckInvocationEscape(
patternIndexer.Syntax,
patternSymbol,
patternIndexer.Receiver,
parameters,
ImmutableArray.Create(patternIndexer.Argument),
default,
default,
checkingReceiver,
escapeFrom,
escapeTo,
diagnostics,
isRefEscape: false);
case BoundKind.ListPatternReceiverPlaceholder:
if (((BoundListPatternReceiverPlaceholder)expr).ValEscape > escapeTo)
{
Error(diagnostics, ErrorCode.ERR_EscapeLocal, node, expr.Syntax);
return false;
}
return true;

case BoundKind.SlicePatternReceiverPlaceholder:
if (((BoundSlicePatternReceiverPlaceholder)expr).ValEscape > escapeTo)
{
Error(diagnostics, ErrorCode.ERR_EscapeLocal, node, expr.Syntax);
return false;
}
return true;

case BoundKind.IndexOrRangePatternIndexerAccess:
var implicitIndexerAccess = (BoundIndexOrRangePatternIndexerAccess)expr;
// Note: the LengthOrCountAccess use is purely local
return CheckValEscape(node, implicitIndexerAccess.IndexerAccess, escapeFrom, escapeTo, checkingReceiver, diagnostics);

case BoundKind.PropertyAccess:
var propertyAccess = (BoundPropertyAccess)expr;
Expand Down
Loading