Skip to content

Commit

Permalink
Null inferences do not flow out of a finally block.
Browse files Browse the repository at this point in the history 
Fixes dotnet#34018
  • Loading branch information
@gafter
gafter committed Apr 25, 2019
1 parent c2d0304 commit 4e67ddb
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 38 deletions.
7 changes: 4 additions & 3 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1184,9 +1184,10 @@ private void InheritNullableStateOfMember(int targetContainerSlot, int valueCont
}

/// <summary>
/// Whenever setting the state of a variable, and that variable is not declared at the point the state is being set,
/// Whenever assigning a variable, and that variable is not declared at the point the state is being set,
/// and the new state might be <see cref="NullableFlowState.MaybeNull"/>, this method should be called to perform the
/// state setting and to ensure the mutation is visible outside the finally block when the mutation occurs in a finally block.
/// state setting and to ensure the mutation is visible outside the finally block when the mutation occurs in a
/// finally block.
/// </summary>
private void SetStateAndTrackForFinally(ref LocalState state, int slot, NullableFlowState newState)
{
Expand Down Expand Up @@ -2299,7 +2300,7 @@ private int LearnFromNullTest(int slot, TypeSymbol expressionType, ref LocalStat
{
if (slot > 0 && PossiblyNullableType(expressionType))
{
SetStateAndTrackForFinally(ref state, slot, NullableFlowState.MaybeNull);
state[slot] = NullableFlowState.MaybeNull;
}

return slot;
Expand Down
83 changes: 48 additions & 35 deletions src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47699,8 +47699,10 @@ void F(C c)
);
}

[Fact, WorkItem(33526, "https://github.com/dotnet/roslyn/issues/33526")]
public void ConditionalAccessIsAPureTest_InFinally()
[Fact]
[WorkItem(33526, "https://github.com/dotnet/roslyn/issues/33526")]
[WorkItem(34018, "https://github.com/dotnet/roslyn/issues/34018")]
public void ConditionalAccessIsAPureTest_InFinally_01()
{
var source = @"
class C
Expand All @@ -47716,15 +47718,40 @@ void F(object o)
o.ToString();
}

o.ToString(); // 1
o.ToString();
}
}";
var comp = CreateCompilation(source, options: WithNonNullTypesTrue());
comp.VerifyDiagnostics(
// (15,9): warning CS8602: Dereference of a possibly null reference.
// o.ToString(); // 1
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "o").WithLocation(15, 9)
);
comp.VerifyDiagnostics();
}

[Fact]
[WorkItem(34018, "https://github.com/dotnet/roslyn/issues/34018")]
public void ConditionalAccessIsAPureTest_InFinally_02()
{
var source = @"
class C
{
void F()
{
C? c = null;
try
{
c = new C();
}
finally
{
if (c != null) c.Cleanup();
}

c.Operation(); // ok
}

void Cleanup() {}
void Operation() {}
}";
var comp = CreateCompilation(source, options: WithNonNullTypesTrue());
comp.VerifyDiagnostics();
}

[Fact, WorkItem(33526, "https://github.com/dotnet/roslyn/issues/33526")]
Expand Down Expand Up @@ -90948,7 +90975,9 @@ public static void Test(bool b, object? o, G<string?> g)
Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "g").WithArguments("G<string?>", "G<string>").WithLocation(17, 24));
}

[Fact, WorkItem(33446, "https://github.com/dotnet/roslyn/issues/33446")]
[Fact]
[WorkItem(33446, "https://github.com/dotnet/roslyn/issues/33446")]
[WorkItem(34018, "https://github.com/dotnet/roslyn/issues/34018")]
public void NullInferencesInFinallyClause()
{
var source =
Expand Down Expand Up @@ -90980,7 +91009,7 @@ static void M2()
if (node is null) {} else {}
}

node.ToString(); // 2
node.ToString();
}
static void M3()
{
Expand All @@ -90994,7 +91023,7 @@ static void M3()
node ??= node;
}

node.ToString(); // 3
node.ToString();
}
static void M4()
{
Expand All @@ -91014,7 +91043,7 @@ static void M4()
}
}

node.ToString(); // 4
node.ToString();
}
static void M5()
{
Expand All @@ -91034,7 +91063,7 @@ static void M5()
}
}

node.ToString(); // 5
node.ToString();
}
static void M6()
{
Expand All @@ -91048,14 +91077,14 @@ static void M6()
(node, _) = (null, node);
}

node.ToString(); // 6
node.ToString(); // 2
}
static void MN1()
{
Node? node = null;
Mout(out node);
if (node == null) {} else {}
node.ToString(); // 7
node.ToString(); // 3
}
static void MN2()
{
Expand All @@ -91069,38 +91098,22 @@ static void MN2()
if (node == null) {} else {}
}

node.ToString(); // 8
node.ToString();
}
static void Mout(out Node node) => node = null!;
}
";
var comp = CreateCompilation(new[] { source }, options: WithNonNullTypesTrue());
// https://github.com/dotnet/roslyn/pull/33929: Once we report one of the (NOT YET) warnings, we should report the other
comp.VerifyDiagnostics(
// (15,9): warning CS8602: Dereference of a possibly null reference.
// node.Next.ToString(); // 1
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "node.Next").WithLocation(15, 9),
// (29,9): warning CS8602: Dereference of a possibly null reference.
// node.ToString(); // 2
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "node").WithLocation(29, 9),
// (43,9): warning CS8602: Dereference of a possibly null reference.
// node.ToString(); // 3
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "node").WithLocation(43, 9),
// (63,9): warning CS8602: Dereference of a possibly null reference.
// node.ToString(); // 4
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "node").WithLocation(63, 9),
// (83,9): warning CS8602: Dereference of a possibly null reference.
// node.ToString(); // 5
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "node").WithLocation(83, 9),
// (97,9): warning CS8602: Dereference of a possibly null reference.
// node.ToString(); // 6
// node.ToString(); // 2
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "node").WithLocation(97, 9),
// (104,9): warning CS8602: Dereference of a possibly null reference.
// node.ToString(); // 7
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "node").WithLocation(104, 9),
// (118,9): warning CS8602: Dereference of a possibly null reference.
// node.ToString(); // 8
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "node").WithLocation(118, 9));
// node.ToString(); // 3
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "node").WithLocation(104, 9));
}

[Fact, WorkItem(32934, "https://github.com/dotnet/roslyn/issues/32934")]
Expand Down

0 comments on commit 4e67ddb

Please sign in to comment.