Skip to content

Commit 5bde237

Browse files
authored
Ensure that CheckValue isn't called twice on lambda query receivers (#80010)
* Ensure that CheckValue isn't called twice on lambda query receivers When creating query expressions, most of the time the receiver the query is not checked for being a valid RValue by calling code. However, in one case, it is; this can then cause an assert that we are checking for field-backed property access multiple times. Fixes #80008. * Update crefs * Add assertion on assumption
1 parent 5f81927 commit 5bde237

File tree

4 files changed

+123
-25
lines changed

4 files changed

+123
-25
lines changed

src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7644,7 +7644,7 @@ private BoundExpression BindDynamicMemberAccess(
76447644
/// </summary>
76457645
/// <remarks>
76467646
/// If new checks are added to this method, they will also need to be added to
7647-
/// <see cref="MakeQueryInvocation(CSharpSyntaxNode, BoundExpression, string, TypeSyntax, TypeWithAnnotations, BindingDiagnosticBag, string)"/>.
7647+
/// <see cref="MakeQueryInvocation(CSharpSyntaxNode, BoundExpression, bool, string, SeparatedSyntaxList{TypeSyntax}, ImmutableArray{TypeWithAnnotations}, ImmutableArray{BoundExpression}, BindingDiagnosticBag, string?)"/>.
76487648
/// </remarks>
76497649
#else
76507650
/// <summary>
@@ -7653,7 +7653,7 @@ private BoundExpression BindDynamicMemberAccess(
76537653
/// </summary>
76547654
/// <remarks>
76557655
/// If new checks are added to this method, they will also need to be added to
7656-
/// <see cref="MakeQueryInvocation(CSharpSyntaxNode, BoundExpression, string, TypeSyntax, TypeWithAnnotations, BindingDiagnosticBag)"/>.
7656+
/// <see cref="MakeQueryInvocation(CSharpSyntaxNode, BoundExpression, bool, string, SeparatedSyntaxList{TypeSyntax}, ImmutableArray{TypeWithAnnotations}, ImmutableArray{BoundExpression}, BindingDiagnosticBag)"/>.
76577657
/// </remarks>
76587658
#endif
76597659
private BoundExpression BindMemberAccessWithBoundLeft(

src/Compilers/CSharp/Portable/Binder/Binder_Query.cs

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ internal BoundExpression BindQuery(QueryExpressionSyntax node, BindingDiagnostic
5454
if (fromClause.Type != null)
5555
{
5656
var typeRestriction = BindTypeArgument(fromClause.Type, diagnostics);
57-
cast = MakeQueryInvocation(fromClause, state.fromExpression, "Cast", fromClause.Type, typeRestriction, diagnostics
57+
cast = MakeQueryInvocation(fromClause, state.fromExpression, receiverIsCheckedForRValue: false, "Cast", fromClause.Type, typeRestriction, diagnostics
5858
#if DEBUG
5959
, state.nextInvokedMethodName
6060
#endif
@@ -230,7 +230,7 @@ private BoundExpression FinalTranslation(QueryTranslationState state, BindingDia
230230
var e = state.fromExpression;
231231
var v = selectClause.Expression;
232232
var lambda = MakeQueryUnboundLambda(state.RangeVariableMap(), x, v, diagnostics.AccumulatesDependencies);
233-
var result = MakeQueryInvocation(state.selectOrGroup, e, "Select", lambda, diagnostics
233+
var result = MakeQueryInvocation(state.selectOrGroup, e, receiverIsCheckedForRValue: false, "Select", lambda, diagnostics
234234
#if DEBUG
235235
, state.nextInvokedMethodName
236236
#endif
@@ -260,7 +260,7 @@ private BoundExpression FinalTranslation(QueryTranslationState state, BindingDia
260260
// this is the unoptimized form (when v is not the identifier x)
261261
var d = BindingDiagnosticBag.GetInstance(diagnostics);
262262
BoundExpression lambdaRight = MakeQueryUnboundLambda(state.RangeVariableMap(), x, v, diagnostics.AccumulatesDependencies);
263-
result = MakeQueryInvocation(state.selectOrGroup, e, "GroupBy", ImmutableArray.Create(lambdaLeft, lambdaRight), d
263+
result = MakeQueryInvocation(state.selectOrGroup, e, receiverIsCheckedForRValue: false, "GroupBy", ImmutableArray.Create(lambdaLeft, lambdaRight), d
264264
#if DEBUG
265265
, state.nextInvokedMethodName
266266
#endif
@@ -276,7 +276,7 @@ private BoundExpression FinalTranslation(QueryTranslationState state, BindingDia
276276
{
277277
// The optimized form. We store the unoptimized form for analysis
278278
unoptimizedForm = result;
279-
result = MakeQueryInvocation(state.selectOrGroup, e, "GroupBy", lambdaLeft, diagnostics
279+
result = MakeQueryInvocation(state.selectOrGroup, e, receiverIsCheckedForRValue: false, "GroupBy", lambdaLeft, diagnostics
280280
#if DEBUG
281281
, state.nextInvokedMethodName
282282
#endif
@@ -364,7 +364,7 @@ private void ReduceWhere(WhereClauseSyntax where, QueryTranslationState state, B
364364
// is translated into
365365
// from x in ( e ) . Where ( x => f )
366366
var lambda = MakeQueryUnboundLambda(state.RangeVariableMap(), state.rangeVariable, where.Condition, diagnostics.AccumulatesDependencies);
367-
var invocation = MakeQueryInvocation(where, state.fromExpression, "Where", lambda, diagnostics
367+
var invocation = MakeQueryInvocation(where, state.fromExpression, receiverIsCheckedForRValue: false, "Where", lambda, diagnostics
368368
#if DEBUG
369369
, state.nextInvokedMethodName
370370
#endif
@@ -395,7 +395,7 @@ private void ReduceJoin(JoinClauseSyntax join, QueryTranslationState state, Bind
395395
// is translated into
396396
// join x in ( e ) . Cast < T > ( ) on k1 equals k2
397397
var castType = BindTypeArgument(join.Type, diagnostics);
398-
castInvocation = MakeQueryInvocation(join, inExpression, "Cast", join.Type, castType, diagnostics
398+
castInvocation = MakeQueryInvocation(join, inExpression, receiverIsCheckedForRValue: false, "Cast", join.Type, castType, diagnostics
399399
#if DEBUG
400400
, expectedMethodName: null
401401
#endif
@@ -426,6 +426,7 @@ private void ReduceJoin(JoinClauseSyntax join, QueryTranslationState state, Bind
426426
invocation = MakeQueryInvocation(
427427
join,
428428
state.fromExpression,
429+
receiverIsCheckedForRValue: false,
429430
"Join",
430431
ImmutableArray.Create(inExpression, outerKeySelectorLambda, innerKeySelectorLambda, resultSelectorLambda),
431432
diagnostics
@@ -454,6 +455,7 @@ private void ReduceJoin(JoinClauseSyntax join, QueryTranslationState state, Bind
454455
invocation = MakeQueryInvocation(
455456
join,
456457
state.fromExpression,
458+
receiverIsCheckedForRValue: false,
457459
"GroupJoin",
458460
ImmutableArray.Create(inExpression, outerKeySelectorLambda, innerKeySelectorLambda, resultSelectorLambda),
459461
diagnostics
@@ -494,6 +496,7 @@ private void ReduceJoin(JoinClauseSyntax join, QueryTranslationState state, Bind
494496
invocation = MakeQueryInvocation(
495497
join,
496498
state.fromExpression,
499+
receiverIsCheckedForRValue: false,
497500
"Join",
498501
ImmutableArray.Create(inExpression, outerKeySelectorLambda, innerKeySelectorLambda, resultSelectorLambda),
499502
diagnostics
@@ -524,6 +527,7 @@ private void ReduceJoin(JoinClauseSyntax join, QueryTranslationState state, Bind
524527
invocation = MakeQueryInvocation(
525528
join,
526529
state.fromExpression,
530+
receiverIsCheckedForRValue: false,
527531
"GroupJoin",
528532
ImmutableArray.Create(inExpression, outerKeySelectorLambda, innerKeySelectorLambda, resultSelectorLambda),
529533
diagnostics
@@ -565,7 +569,7 @@ private void ReduceOrderBy(OrderByClauseSyntax orderby, QueryTranslationState st
565569
{
566570
string methodName = (first ? "OrderBy" : "ThenBy") + (ordering.IsKind(SyntaxKind.DescendingOrdering) ? "Descending" : "");
567571
var lambda = MakeQueryUnboundLambda(state.RangeVariableMap(), state.rangeVariable, ordering.Expression, diagnostics.AccumulatesDependencies);
568-
var invocation = MakeQueryInvocation(ordering, state.fromExpression, methodName, lambda, diagnostics
572+
var invocation = MakeQueryInvocation(ordering, state.fromExpression, receiverIsCheckedForRValue: false, methodName, lambda, diagnostics
569573
#if DEBUG
570574
, state.nextInvokedMethodName
571575
#endif
@@ -611,6 +615,7 @@ private void ReduceFrom(FromClauseSyntax from, QueryTranslationState state, Bind
611615
var invocation = MakeQueryInvocation(
612616
from,
613617
state.fromExpression,
618+
receiverIsCheckedForRValue: false,
614619
"SelectMany",
615620
ImmutableArray.Create(collectionSelectorLambda, resultSelectorLambda),
616621
diagnostics
@@ -658,6 +663,7 @@ private void ReduceFrom(FromClauseSyntax from, QueryTranslationState state, Bind
658663
var invocation = MakeQueryInvocation(
659664
from,
660665
state.fromExpression,
666+
receiverIsCheckedForRValue: false,
661667
"SelectMany",
662668
ImmutableArray.Create(collectionSelectorLambda, resultSelectorLambda),
663669
diagnostics
@@ -754,7 +760,7 @@ private void ReduceLet(LetClauseSyntax let, QueryTranslationState state, Binding
754760
state.AddTransparentIdentifier(x.Name);
755761
var y = state.AddRangeVariable(this, let.Identifier, diagnostics);
756762
state.allRangeVariables[y].Add(let.Identifier.ValueText);
757-
var invocation = MakeQueryInvocation(let, state.fromExpression, "Select", lambda, diagnostics
763+
var invocation = MakeQueryInvocation(let, state.fromExpression, receiverIsCheckedForRValue: false, "Select", lambda, diagnostics
758764
#if DEBUG
759765
, state.nextInvokedMethodName
760766
#endif
@@ -847,7 +853,7 @@ private UnboundLambda MakeQueryUnboundLambdaWithCast(RangeVariableMap qvm, Range
847853
BoundExpression boundExpression = lambdaBodyBinder.BindValue(expression, diagnostics, BindValueKind.RValue);
848854

849855
// We transform the expression from "expr" to "expr.Cast<castTypeOpt>()".
850-
boundExpression = lambdaBodyBinder.MakeQueryInvocation(expression, boundExpression, "Cast", castTypeSyntax, castType, diagnostics
856+
boundExpression = lambdaBodyBinder.MakeQueryInvocation(expression, boundExpression, receiverIsCheckedForRValue: true, "Cast", castTypeSyntax, castType, diagnostics
851857
#if DEBUG
852858
, expectedMethodName: null
853859
#endif
@@ -871,46 +877,46 @@ private static UnboundLambda MakeQueryUnboundLambda(CSharpSyntaxNode node, Query
871877
return lambda;
872878
}
873879

874-
protected BoundCall MakeQueryInvocation(CSharpSyntaxNode node, BoundExpression receiver, string methodName, BoundExpression arg, BindingDiagnosticBag diagnostics
880+
protected BoundCall MakeQueryInvocation(CSharpSyntaxNode node, BoundExpression receiver, bool receiverIsCheckedForRValue, string methodName, BoundExpression arg, BindingDiagnosticBag diagnostics
875881
#if DEBUG
876882
, string? expectedMethodName
877883
#endif
878884
)
879885
{
880-
return MakeQueryInvocation(node, receiver, methodName, default(SeparatedSyntaxList<TypeSyntax>), default(ImmutableArray<TypeWithAnnotations>), ImmutableArray.Create(arg), diagnostics
886+
return MakeQueryInvocation(node, receiver, receiverIsCheckedForRValue, methodName, default(SeparatedSyntaxList<TypeSyntax>), default(ImmutableArray<TypeWithAnnotations>), ImmutableArray.Create(arg), diagnostics
881887
#if DEBUG
882888
, expectedMethodName
883889
#endif
884890
);
885891
}
886892

887-
protected BoundCall MakeQueryInvocation(CSharpSyntaxNode node, BoundExpression receiver, string methodName, ImmutableArray<BoundExpression> args, BindingDiagnosticBag diagnostics
893+
protected BoundCall MakeQueryInvocation(CSharpSyntaxNode node, BoundExpression receiver, bool receiverIsCheckedForRValue, string methodName, ImmutableArray<BoundExpression> args, BindingDiagnosticBag diagnostics
888894
#if DEBUG
889895
, string? expectedMethodName
890896
#endif
891897
)
892898
{
893-
return MakeQueryInvocation(node, receiver, methodName, default(SeparatedSyntaxList<TypeSyntax>), default(ImmutableArray<TypeWithAnnotations>), args, diagnostics
899+
return MakeQueryInvocation(node, receiver, receiverIsCheckedForRValue, methodName, default(SeparatedSyntaxList<TypeSyntax>), default(ImmutableArray<TypeWithAnnotations>), args, diagnostics
894900
#if DEBUG
895901
, expectedMethodName
896902
#endif
897903
);
898904
}
899905

900-
protected BoundCall MakeQueryInvocation(CSharpSyntaxNode node, BoundExpression receiver, string methodName, TypeSyntax typeArgSyntax, TypeWithAnnotations typeArg, BindingDiagnosticBag diagnostics
906+
protected BoundCall MakeQueryInvocation(CSharpSyntaxNode node, BoundExpression receiver, bool receiverIsCheckedForRValue, string methodName, TypeSyntax typeArgSyntax, TypeWithAnnotations typeArg, BindingDiagnosticBag diagnostics
901907
#if DEBUG
902908
, string? expectedMethodName
903909
#endif
904910
)
905911
{
906-
return MakeQueryInvocation(node, receiver, methodName, new SeparatedSyntaxList<TypeSyntax>(new SyntaxNodeOrTokenList(typeArgSyntax, 0)), ImmutableArray.Create(typeArg), ImmutableArray<BoundExpression>.Empty, diagnostics
912+
return MakeQueryInvocation(node, receiver, receiverIsCheckedForRValue, methodName, new SeparatedSyntaxList<TypeSyntax>(new SyntaxNodeOrTokenList(typeArgSyntax, 0)), ImmutableArray.Create(typeArg), ImmutableArray<BoundExpression>.Empty, diagnostics
907913
#if DEBUG
908914
, expectedMethodName
909915
#endif
910916
);
911917
}
912918

913-
protected BoundCall MakeQueryInvocation(CSharpSyntaxNode node, BoundExpression receiver, string methodName, SeparatedSyntaxList<TypeSyntax> typeArgsSyntax, ImmutableArray<TypeWithAnnotations> typeArgs, ImmutableArray<BoundExpression> args, BindingDiagnosticBag diagnostics
919+
protected BoundCall MakeQueryInvocation(CSharpSyntaxNode node, BoundExpression receiver, bool receiverIsCheckedForRValue, string methodName, SeparatedSyntaxList<TypeSyntax> typeArgsSyntax, ImmutableArray<TypeWithAnnotations> typeArgs, ImmutableArray<BoundExpression> args, BindingDiagnosticBag diagnostics
914920
#if DEBUG
915921
, string? expectedMethodName
916922
#endif
@@ -982,10 +988,17 @@ protected BoundCall MakeQueryInvocation(CSharpSyntaxNode node, BoundExpression r
982988
}
983989
else
984990
{
985-
var checkedUltimateReceiver = CheckValue(ultimateReceiver, BindValueKind.RValue, diagnostics);
986-
if (checkedUltimateReceiver != ultimateReceiver)
991+
if (!receiverIsCheckedForRValue)
987992
{
988-
receiver = updateUltimateReceiver(receiver, ultimateReceiver, checkedUltimateReceiver);
993+
var checkedUltimateReceiver = CheckValue(ultimateReceiver, BindValueKind.RValue, diagnostics);
994+
if (checkedUltimateReceiver != ultimateReceiver)
995+
{
996+
receiver = updateUltimateReceiver(receiver, ultimateReceiver, checkedUltimateReceiver);
997+
}
998+
}
999+
else
1000+
{
1001+
Debug.Assert(ultimateReceiver is not BoundQueryClause);
9891002
}
9901003
}
9911004

src/Compilers/CSharp/Test/Emit3/Semantics/ExtensionTests.cs

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17616,8 +17616,8 @@ public class C
1761617616
Diagnostic(ErrorCode.ERR_BadDynamicQuery, "where i is not null").WithLocation(2, 9));
1761717617
}
1761817618

17619-
[Fact(Skip = "Tracked by https://github.com/dotnet/roslyn/issues/78968 : WasPropertyBackingFieldAccessChecked asserts that we're setting twice")]
17620-
public void ResolveAll_Query_Cast()
17619+
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/80008")]
17620+
public void ResolveAll_Query_Cast_StaticProperty()
1762117621
{
1762217622
var source = """
1762317623
using System.Linq;
@@ -17643,10 +17643,48 @@ static class E
1764317643
var tree = comp.SyntaxTrees.First();
1764417644
var model = comp.GetSemanticModel(tree);
1764517645
var memberAccess1 = GetSyntax<MemberAccessExpressionSyntax>(tree, "object.M");
17646-
AssertEx.Equal("System.Object[] E.M", model.GetSymbolInfo(memberAccess1).Symbol.ToTestDisplayString());
17646+
AssertEx.Equal("System.Object[] E.<G>$C43E2675C7BBF9284AF22FB8A9BF0280.M { get; }",
17647+
model.GetSymbolInfo(memberAccess1).Symbol.ToTestDisplayString());
1764717648

1764817649
var memberAccess2 = GetSyntax<MemberAccessExpressionSyntax>(tree, "object.M2");
17649-
AssertEx.Equal("System.Object[] E.M2", model.GetSymbolInfo(memberAccess2).Symbol.ToTestDisplayString());
17650+
AssertEx.Equal("System.Object[] E.<G>$C43E2675C7BBF9284AF22FB8A9BF0280.M2 { get; }",
17651+
model.GetSymbolInfo(memberAccess2).Symbol.ToTestDisplayString());
17652+
}
17653+
17654+
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/80008")]
17655+
public void ResolveAll_Query_Cast_InstanceProperty()
17656+
{
17657+
var source = """
17658+
using System.Linq;
17659+
17660+
var o = new object();
17661+
var r = from string s in o.M from string s2 in o.M2 select s.ToString();
17662+
foreach (var x in r)
17663+
{
17664+
System.Console.Write(x.ToString());
17665+
}
17666+
17667+
static class E
17668+
{
17669+
extension(object o)
17670+
{
17671+
public object[] M => ["ran"];
17672+
public object[] M2 => [""];
17673+
}
17674+
}
17675+
""";
17676+
var comp = CreateCompilation(source);
17677+
CompileAndVerify(comp, expectedOutput: "ran").VerifyDiagnostics();
17678+
17679+
var tree = comp.SyntaxTrees.First();
17680+
var model = comp.GetSemanticModel(tree);
17681+
var memberAccess1 = GetSyntax<MemberAccessExpressionSyntax>(tree, "o.M");
17682+
AssertEx.Equal("System.Object[] E.<G>$C43E2675C7BBF9284AF22FB8A9BF0280.M { get; }",
17683+
model.GetSymbolInfo(memberAccess1).Symbol.ToTestDisplayString());
17684+
17685+
var memberAccess2 = GetSyntax<MemberAccessExpressionSyntax>(tree, "o.M2");
17686+
AssertEx.Equal("System.Object[] E.<G>$C43E2675C7BBF9284AF22FB8A9BF0280.M2 { get; }",
17687+
model.GetSymbolInfo(memberAccess2).Symbol.ToTestDisplayString());
1765017688
}
1765117689

1765217690
[Fact]

src/Compilers/CSharp/Test/Semantic/Semantics/QueryTests.cs

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4739,5 +4739,52 @@ struct V
47394739
// select r.V.F++;
47404740
Diagnostic(ErrorCode.ERR_AssignReadonlyNotField2, "r.V.F").WithArguments("field", "V").WithLocation(6, 12));
47414741
}
4742+
4743+
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/80008")]
4744+
public void PropertyAccess_Instance()
4745+
{
4746+
var code = """
4747+
using System.Linq;
4748+
4749+
var f = new F();
4750+
var r = from string s in f.M from string s2 in f.M2 select s.ToString();
4751+
foreach (var x in r)
4752+
{
4753+
System.Console.Write(x.ToString());
4754+
}
4755+
4756+
public class F
4757+
{
4758+
public object[] M => ["ran"];
4759+
public object[] M2 => [""];
4760+
}
4761+
""";
4762+
4763+
var comp = CreateCompilation(code);
4764+
CompileAndVerify(code, expectedOutput: "ran").VerifyDiagnostics();
4765+
}
4766+
4767+
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/80008")]
4768+
public void PropertyAccess_Static()
4769+
{
4770+
var code = """
4771+
using System.Linq;
4772+
4773+
var r = from string s in F.M from string s2 in F.M2 select s.ToString();
4774+
foreach (var x in r)
4775+
{
4776+
System.Console.Write(x.ToString());
4777+
}
4778+
4779+
public static class F
4780+
{
4781+
public static object[] M => ["ran"];
4782+
public static object[] M2 => [""];
4783+
}
4784+
""";
4785+
4786+
var comp = CreateCompilation(code);
4787+
CompileAndVerify(code, expectedOutput: "ran").VerifyDiagnostics();
4788+
}
47424789
}
47434790
}

0 commit comments

Comments
 (0)