Skip to content

R
roslyn

lwm1986 / roslyn
与 Fork 源项目一致

从无法访问的项目Fork

通知 1
Star 0
Fork 0
R
roslyn

体验新版 GitCode，发现更多精彩内容 >>

未验证 提交 74502bae 编写于 作者: J Julien Couvreur 提交者： GitHub
浏览文件
操作

Fix result and slot of conditional receiver (#33997)

上级 69662045
隐藏空白更改
内联 并排
Showing with 315 addition and 84 deletion
src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
浏览文件 @ 74502bae
......@@ -119,6 +119,14 @@ public VisitResult(TypeWithState resultType, TypeSymbolWithAnnotations lValueTyp
/// </summary>
private VisitResult _visitResult;
/// <summary>
/// The visit result of the receiver for the current conditional access.
///
/// For example: A conditional invocation uses a placeholder as a receiver. By storing the
/// visit result from the actual receiver ahead of time, we can give this placeholder a correct result.
/// </summary>
private VisitResult _currentConditionalReceiverVisitResult;
/// <summary>
/// The result type represents the state of the last visited expression.
/// </summary>
......@@ -545,9 +553,7 @@ protected override int MakeSlot(BoundExpression node)
return getPlaceholderSlot(node);
case BoundKind.ConditionalReceiver:
{
int slot = _lastConditionalAccessSlot;
_lastConditionalAccessSlot = -1;
return slot;
return _lastConditionalAccessSlot;
}
default:
// If there was a placeholder local for this node, we should
......@@ -1962,89 +1968,86 @@ static bool isNonNullConstant(BoundExpression expr)
private void GetSlotsToMarkAsNotNullable(BoundExpression operand, ArrayBuilder<int> slotBuilder)
{
Debug.Assert(operand != null);
Debug.Assert(_lastConditionalAccessSlot == -1);
var previousConditionalAccessSlot = _lastConditionalAccessSlot;
while (true)
try
{
// Due to the nature of binding, if there are conditional access they will be at the top of the bound tree,
// potentially with a conversion on top of it. We go through any conditional accesses, adding slots for the
// conditional receivers if they have them. If we ever get to a receiver that MakeSlot doesn't return a slot
// for, nothing underneath is trackable and we bail at that point. Example:
//
// a?.GetB()?.C // a is a field, GetB is a method, and C is a property
//
// The top of the tree is the a?.GetB() conditional call. We'll ask for a slot for a, and we'll get one because
// fields have slots. The AccessExpression of the BoundConditionalAccess is another BoundConditionalAccess, this time
// with a receiver of the GetB() BoundCall. Attempting to get a slot for this receiver will fail, and we'll
// return an array with just the slot for a.
int slot;
switch (operand.Kind)
{
case BoundKind.Conversion:
// https://github.com/dotnet/roslyn/issues/33879 Detect when conversion has a nullable operand
operand = ((BoundConversion)operand).Operand;
continue;
case BoundKind.ConditionalAccess:
var conditional = (BoundConditionalAccess)operand;
slot = MakeSlot(conditional.Receiver);
if (slot > 0)
{
// If we got a slot we must have processed the previous conditional receiver.
Debug.Assert(_lastConditionalAccessSlot == -1);
while (true)
{
// Due to the nature of binding, if there are conditional access they will be at the top of the bound tree,
// potentially with a conversion on top of it. We go through any conditional accesses, adding slots for the
// conditional receivers if they have them. If we ever get to a receiver that MakeSlot doesn't return a slot
// for, nothing underneath is trackable and we bail at that point. Example:
//
// a?.GetB()?.C // a is a field, GetB is a method, and C is a property
//
// The top of the tree is the a?.GetB() conditional call. We'll ask for a slot for a, and we'll get one because
// fields have slots. The AccessExpression of the BoundConditionalAccess is another BoundConditionalAccess, this time
// with a receiver of the GetB() BoundCall. Attempting to get a slot for this receiver will fail, and we'll
// return an array with just the slot for a.
int slot;
switch (operand.Kind)
{
case BoundKind.Conversion:
// https://github.com/dotnet/roslyn/issues/33879 Detect when conversion has a nullable operand
operand = ((BoundConversion)operand).Operand;
continue;
case BoundKind.ConditionalAccess:
var conditional = (BoundConditionalAccess)operand;
// We need to continue the walk regardless of whether the receiver should be updated.
var receiverType = conditional.Receiver.Type;
if (PossiblyNullableType(receiverType))
slot = MakeSlot(conditional.Receiver);
if (slot > 0)
{
slotBuilder.Add(slot);
// We need to continue the walk regardless of whether the receiver should be updated.
var receiverType = conditional.Receiver.Type;
if (PossiblyNullableType(receiverType))
{
slotBuilder.Add(slot);
}
if (receiverType.IsNullableType())
{
slot = GetNullableOfTValueSlot(receiverType, slot, out _);
}
}
if (receiverType.IsNullableType())
if (slot > 0)
{
slot = GetNullableOfTValueSlot(receiverType, slot, out _);
// When MakeSlot is called on the nested AccessExpression, it will recurse through receivers
// until it gets to the BoundConditionalReceiver associated with this node. In our override,
// we substitute this slot when we encounter a BoundConditionalReceiver, and reset the
// _lastConditionalAccess field.
_lastConditionalAccessSlot = slot;
operand = conditional.AccessExpression;
continue;
}
}
if (slot > 0)
{
// When MakeSlot is called on the nested AccessExpression, it will recurse through receivers
// until it gets to the BoundConditionalReceiver associated with this node. In our override,
// we substitute this slot when we encounter a BoundConditionalReceiver, and reset the
// _lastConditionalAccess field.
_lastConditionalAccessSlot = slot;
operand = conditional.AccessExpression;
continue;
}
// If there's no slot for this receiver, there cannot be another slot for any of the remaining
// access expressions.
break;
default:
// Attempt to create a slot for the current thing. If there were any more conditional accesses,
// they would have been on top, so this is the last thing we need to specially handle.
// If there's no slot for this receiver, there cannot be another slot for any of the remaining
// access expressions.
break;
default:
// Attempt to create a slot for the current thing. If there were any more conditional accesses,
// they would have been on top, so this is the last thing we need to specially handle.
// https://github.com/dotnet/roslyn/issues/33879 When we handle unconditional access survival (ie after
// c.D has been invoked, c must be nonnull or we've thrown a NullRef), revisit whether
// we need more special handling here
// https://github.com/dotnet/roslyn/issues/33879 When we handle unconditional access survival (ie after
// c.D has been invoked, c must be nonnull or we've thrown a NullRef), revisit whether
// we need more special handling here
slot = MakeSlot(operand);
if (slot > 0 && PossiblyNullableType(operand.Type))
{
// If we got a slot then all previous BoundCondtionalReceivers must have been handled.
Debug.Assert(_lastConditionalAccessSlot == -1);
slot = MakeSlot(operand);
if (slot > 0 && PossiblyNullableType(operand.Type))
{
slotBuilder.Add(slot);
}
slotBuilder.Add(slot);
}
break;
}
break;
return;
}
// If we didn't get a slot, it's possible that the current _lastConditionalSlot was never processed,
// so we reset before leaving the function.
_lastConditionalAccessSlot = -1;
return;
}
finally
{
_lastConditionalAccessSlot = previousConditionalAccessSlot;
}
}
......@@ -2247,17 +2250,20 @@ public override BoundNode VisitConditionalAccess(BoundConditionalAccess node)
var receiver = node.Receiver;
var receiverType = VisitRvalueWithState(receiver);
_currentConditionalReceiverVisitResult = _visitResult;
var previousConditionalAccessSlot = _lastConditionalAccessSlot;
var receiverState = this.State.Clone();
if (IsConstantNull(node.Receiver))
{
SetUnreachable();
_lastConditionalAccessSlot = -1;
}
else
{
// In the right-hand-side, the left-hand-side is known to be non-null.
// https://github.com/dotnet/roslyn/issues/33347: This should probably be using GetSlotsToMarkAsNotNullable() rather than marking only one slot
int slot = MakeSlot(SkipReferenceConversions(receiver));
_lastConditionalAccessSlot = slot;
if (slot > 0)
{
if (slot >= this.State.Capacity) Normalize(ref this.State);
......@@ -2282,6 +2288,8 @@ public override BoundNode VisitConditionalAccess(BoundConditionalAccess node)
ReportSafetyDiagnostic(ErrorCode.WRN_ConditionalAccessMayReturnNull, node.Syntax, node.Type);
}
_currentConditionalReceiverVisitResult = default;
_lastConditionalAccessSlot = previousConditionalAccessSlot;
ResultType = new TypeWithState(type, resultState);
return null;
}
......@@ -2473,11 +2481,13 @@ private static BoundExpression CreatePlaceholderIfNecessary(BoundExpression expr
public override BoundNode VisitConditionalReceiver(BoundConditionalReceiver node)
{
var result = base.VisitConditionalReceiver(node);
// https://github.com/dotnet/roslyn/issues/29956 ConditionalReceiver does not
// have a result type. Should this be moved to ConditionalAccess?
SetNotNullResult(node);
return result;
var rvalueType = _currentConditionalReceiverVisitResult.RValueType.Type;
if (rvalueType?.IsNullableType() == true)
{
rvalueType = rvalueType.GetNullableUnderlyingType();
}
ResultType = new TypeWithState(rvalueType, NullableFlowState.NotNull);
return null;
}
public override BoundNode VisitCall(BoundCall node)
......
src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs
浏览文件 @ 74502bae
......@@ -88,6 +88,161 @@ public override string ToString()
}
}";
[Fact, WorkItem(33289, "https://github.com/dotnet/roslyn/issues/33289")]
public void ConditionalReceiver()
{
var comp = CreateCompilation(@"
public class Container<T>
{
public T Field = default!;
}
class C
{
static void M(string? s)
{
var x = Create(s);
_ = x /*T:Container<string?>?*/;
x?.Field.ToString(); // 1
}
public static Container<U>? Create<U>(U u) => new Container<U>();
}", options: WithNonNullTypesTrue());
comp.VerifyTypes();
comp.VerifyDiagnostics(
// (13,11): warning CS8602: Possible dereference of a null reference.
// x?.Field.ToString(); // 1
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, ".Field").WithLocation(13, 11)
);
}
[Fact, WorkItem(33289, "https://github.com/dotnet/roslyn/issues/33289")]
public void ConditionalReceiver_Chained()
{
var comp = CreateCompilation(@"
public class Container<T>
{
public T Field = default!;
}
class C
{
static void M(string? s)
{
var x = Create(s);
if (x is null) return;
_ = x /*T:Container<string?>!*/;
x?.Field.ToString(); // 1
x = Create(s);
if (x is null) return;
x.Field?.ToString();
}
public static Container<U>? Create<U>(U u) => new Container<U>();
}", options: WithNonNullTypesTrue());
comp.VerifyTypes();
comp.VerifyDiagnostics(
// (14,11): warning CS8602: Possible dereference of a null reference.
// x?.Field.ToString(); // 1
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, ".Field").WithLocation(14, 11)
);
}
[Fact, WorkItem(33289, "https://github.com/dotnet/roslyn/issues/33289")]
public void ConditionalReceiver_Chained_Inversed()
{
var comp = CreateCompilation(@"
public class Container<T>
{
public T Field = default!;
}
class C
{
static void M(string s)
{
var x = Create(s);
_ = x /*T:Container<string!>?*/;
x?.Field.ToString();
x = Create(s);
x.Field?.ToString(); // 1
}
public static Container<U>? Create<U>(U u) => new Container<U>();
}", options: WithNonNullTypesTrue());
comp.VerifyTypes();
comp.VerifyDiagnostics(
// (16,9): warning CS8602: Possible dereference of a null reference.
// x.Field?.ToString(); // 1
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x").WithLocation(16, 9)
);
}
[Fact, WorkItem(33289, "https://github.com/dotnet/roslyn/issues/33289")]
public void ConditionalReceiver_Nested()
{
var comp = CreateCompilation(@"
public class Container<T>
{
public T Field = default!;
}
class C
{
static void M(string? s1, string s2)
{
var x = Create(s1);
var y = Create(s2);
x?.Field /*1*/
.Extension(y?.Field.ToString());
}
public static Container<U>? Create<U>(U u) => new Container<U>();
}
public static class Extensions
{
public static void Extension(this string s, object? o) => throw null!;
}", options: WithNonNullTypesTrue());
comp.VerifyTypes();
comp.VerifyDiagnostics(
// (13,11): warning CS8604: Possible null reference argument for parameter 's' in 'void Extensions.Extension(string s, object? o)'.
// x?.Field /*1*/
Diagnostic(ErrorCode.WRN_NullReferenceArgument, ".Field").WithArguments("s", "void Extensions.Extension(string s, object? o)").WithLocation(13, 11)
);
}
[Fact, WorkItem(33289, "https://github.com/dotnet/roslyn/issues/33289")]
public void ConditionalReceiver_MiscTypes()
{
var comp = CreateCompilation(@"
public class Container<T>
{
public T M() => default!;
}
class C
{
static void M<T>(int i, Missing m, string s, T t)
{
Create(i)?.M().ToString();
Create(m)?.M().ToString();
Create(s)?.M().ToString();
Create(t)?.M().ToString(); // 1
}
public static Container<U>? Create<U>(U u) => new Container<U>();
}", options: WithNonNullTypesTrue());
comp.VerifyDiagnostics(
// (8,29): error CS0246: The type or namespace name 'Missing' could not be found (are you missing a using directive or an assembly reference?)
// static void M<T>(int i, Missing m, string s, T t)
Diagnostic(ErrorCode.ERR_SingleTypeNameNotFound, "Missing").WithArguments("Missing").WithLocation(8, 29),
// (13,19): warning CS8602: Possible dereference of a null reference.
// Create(t)?.M().ToString(); // 1
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, ".M()").WithLocation(13, 19)
);
}
[Fact, WorkItem(33537, "https://github.com/dotnet/roslyn/issues/33537")]
public void SuppressOnNullLiteralInAs()
{
......@@ -82751,11 +82906,11 @@ static IC<T> CreateC<T>(T t)
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "iy.B(x)").WithLocation(29, 9));
}
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/33347")]
[Fact, WorkItem(33347, "https://github.com/dotnet/roslyn/issues/33347")]
public void NestedNullConditionalAccess()
{
var source =
@"class Node
var source = @"
class Node
{
public Node? Next = null;
void M(Node node) { }
......@@ -82763,12 +82918,78 @@ private static void Test(Node? node)
{
node?.Next?.Next?.M(node.Next);
}
}
";
var comp = CreateCompilation(new[] { source }, options: WithNonNullTypesTrue());
}";
var comp = CreateCompilation(source, options: WithNonNullTypesTrue());
comp.VerifyDiagnostics();
}
[Fact, WorkItem(31909, "https://github.com/dotnet/roslyn/issues/31909")]
public void NestedNullConditionalAccess2()
{
var source = @"
public class C
{
public C? f;
void Test1(C? c) => c?.f.M(c.f.ToString()); // nested use of `c.f` is safe
void Test2(C? c) => c.f.M(c.f.ToString());
void M(string s) => throw null!;
}";
var comp = CreateCompilation(source, options: WithNonNullTypesTrue());
comp.VerifyDiagnostics(
// (5,27): warning CS8602: Possible dereference of a null reference.
// void Test1(C? c) => c?.f.M(c.f.ToString()); // nested use of `c.f` is safe
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, ".f").WithLocation(5, 27),
// (6,25): warning CS8602: Possible dereference of a null reference.
// void Test2(C? c) => c.f.M(c.f.ToString());
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "c").WithLocation(6, 25),
// (6,25): warning CS8602: Possible dereference of a null reference.
// void Test2(C? c) => c.f.M(c.f.ToString());
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "c.f").WithLocation(6, 25)
);
}
[Fact, WorkItem(33347, "https://github.com/dotnet/roslyn/issues/33347")]
public void NestedNullConditionalAccess3()
{
var source = @"
class Node
{
public Node? Next = null;
static Node M2(Node a, Node b) => a;
Node M1() => null!;
private static void Test(Node notNull, Node? possiblyNull)
{
_ = possiblyNull?.Next?.M1() ?? M2(possiblyNull = notNull, possiblyNull.Next = notNull);
possiblyNull.Next.M1(); // incorrect warning
}
}";
var comp = CreateCompilation(source, options: WithNonNullTypesTrue());
comp.VerifyDiagnostics();
}
[Fact, WorkItem(31905, "https://github.com/dotnet/roslyn/issues/31905")]
public void NestedNullConditionalAccess4()
{
var source = @"
public class C
{
public C? Nested;
void Test1(C? c) => c?.Nested?.M(c.Nested.ToString());
void Test2(C? c) => c.Nested?.M(c.Nested.ToString());
void Test3(C c) => c?.Nested?.M(c.Nested.ToString());
void M(string s) => throw null!;
}";
var comp = CreateCompilation(source, options: WithNonNullTypesTrue());
comp.VerifyDiagnostics(
// (7,25): warning CS8602: Possible dereference of a null reference.
// void Test2(C? c) => c.Nested?.M(c.Nested.ToString());
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "c").WithLocation(7, 25)
);
}
[Fact]
public void ConditionalAccess()
{
Markdown is supported
0% 或 .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑！
想要评论请 注册 或