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

Caller of an async local function cannot expect any of the callee’s code to execute #64482

Merged
merged 6 commits into from
Oct 27, 2022
Merged

Caller of an async local function cannot expect any of the callee’s code to execute #64482

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d3ee505
Caller of an async local function cannot expect any of the callee’s c…
AlekseyTs Oct 4, 2022
cc83c79
Merge remote-tracking branch 'dotnet/main' into Issue43697
AlekseyTs Oct 12, 2022
d3416e3
Document a breaking change
AlekseyTs Oct 12, 2022
c7fcfb0
Merge remote-tracking branch 'dotnet/main' into Issue43697
AlekseyTs Oct 13, 2022
8569782
Update the breaking change description
AlekseyTs Oct 13, 2022
3d34864
Merge branch 'main' into Issue43697
AlekseyTs Oct 27, 2022
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
27 changes: 27 additions & 0 deletions docs/compilers/CSharp/Compiler Breaking Changes - DotNet 7.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,32 @@
# This document lists known breaking changes in Roslyn after .NET 6 all the way to .NET 7.

## Caller of an async local function cannot expect any of the callee�s code to execute.
Copy link
Member

@jcouv jcouv Oct 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

nit: GitHub diff shows some unicode character as the apostrophe for "callee's". Is that intentional? #Closed


***Introduced in Visual Studio 2022 version 17.5***

See https://github.com/dotnet/roslyn/issues/43697 for the rational.
Copy link
Member

@jcouv jcouv Oct 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

typo: rationale #Closed

Copy link
Member

@jcouv jcouv Oct 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Consider adding a one sentence summary, maybe something like:
For definite assignment analysis, invocations of async local functions are no longer treated as being awaited and therefore the local function is not considered executed.

Let's send an email as FYI to compat council for new compat break.

It would be good to add a note to the spec even though it's an older/incomplete document, and tag Rex on the edit (so that definite assignment rules in standard can reflect this change as appropriate). #Closed

The code below is now going to report a definite assignment error:
```csharp
public async Task M()
{
bool a;
await M1();
Console.WriteLine(a); // error CS0165: Use of unassigned local variable 'a'

async Task M1()
{
if ("" == String.Empty)
{
throw new Exception();
}
else
{
a = true;
}
}
}
```

## Type tests for `ref` structs are not supported.

***Introduced in Visual Studio 2022 version 17.4***
Expand Down
6 changes: 5 additions & 1 deletion src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1320,7 +1320,11 @@ protected virtual void VisitLocalFunctionUse(
if (isCall)
{
Join(ref State, ref localFunctionState.StateFromBottom);
Meet(ref State, ref localFunctionState.StateFromTop);

if (!symbol.IsAsync)
{
Meet(ref State, ref localFunctionState.StateFromTop);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we have a follow-up issue to track the decision to also perform this Meet when the call is "immediately awaited"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we have a follow-up issue to track the decision to also perform this Meet when the call is "immediately awaited"?

That is a language design change that I am not planning to champion at the moment.

}
}
localFunctionState.Visited = true;
}
Expand Down
3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAwaitUsingTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1481,6 +1481,9 @@ async void L1()
}
}" + IAsyncDisposableDefinition);
comp.VerifyDiagnostics(
// (9,9): error CS0165: Use of unassigned local variable 'x'
// x++;
Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(9, 9),
// (10,9): error CS0165: Use of unassigned local variable 'y'
// y++;
Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(10, 9),
Expand Down
3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Test/Emit2/FlowAnalysis/LocalFunctions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,9 @@ async void L1()
}
}");
comp.VerifyDiagnostics(
// (11,9): error CS0165: Use of unassigned local variable 'x'
// x++;
Diagnostic(ErrorCode.ERR_UseDefViolation, "x").WithArguments("x").WithLocation(11, 9),
// (12,9): error CS0165: Use of unassigned local variable 'y'
// y++;
Diagnostic(ErrorCode.ERR_UseDefViolation, "y").WithArguments("y").WithLocation(12, 9),
Expand Down
184 changes: 184 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/LocalFunctionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10072,5 +10072,189 @@ int P

Assert.Null(model.GetSymbolInfo(node).Symbol);
}

[Fact, WorkItem(43697, "https://github.com/dotnet/roslyn/issues/43697")]
public void DefiniteAssignment_01()
{
var text = @"
using System;
using System.Threading.Tasks;

#pragma warning disable CS4014 // Because this call is not awaited, execution of the current method continues before the call is completed. Consider applying the 'await' operator to the result of the call.
#pragma warning disable CS1998 // This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

public class C
{
public void M()
{
bool a;
M1();
Console.WriteLine(a);
async Task M1()
{
if ("""" == String.Empty)
{
throw new Exception();
}
else
{
a = true;
}
}
}
}
";
CreateCompilation(text).VerifyDiagnostics(
// (14,27): error CS0165: Use of unassigned local variable 'a'
// Console.WriteLine(a);
Diagnostic(ErrorCode.ERR_UseDefViolation, "a").WithArguments("a").WithLocation(14, 27)
);
}

[Fact, WorkItem(43697, "https://github.com/dotnet/roslyn/issues/43697")]
public void DefiniteAssignment_02()
{
var text = @"
using System;
using System.Threading.Tasks;

public class C
{
public void M()
{
bool a;
M1();
Console.WriteLine(a);
Task M1()
{
if ("""" == String.Empty)
{
throw new Exception();
}
else
{
a = true;
}

return null;
}
}
}
";
CreateCompilation(text).VerifyDiagnostics();
}

[Fact, WorkItem(43697, "https://github.com/dotnet/roslyn/issues/43697")]
public void DefiniteAssignment_03()
{
var text = @"
using System;
using System.Threading.Tasks;

#pragma warning disable CS4014 // Because this call is not awaited, execution of the current method continues before the call is completed. Consider applying the 'await' operator to the result of the call.

public class C
{
public void M()
{
bool a;
M1();
Console.WriteLine(a);
async Task M1()
{
await Task.Yield();

if ("""" == String.Empty)
{
throw new Exception();
}
else
{
a = true;
}
}
}
}
";
CreateCompilation(text).VerifyDiagnostics(
// (13,27): error CS0165: Use of unassigned local variable 'a'
// Console.WriteLine(a);
Diagnostic(ErrorCode.ERR_UseDefViolation, "a").WithArguments("a").WithLocation(13, 27)
);
}

[Fact, WorkItem(43697, "https://github.com/dotnet/roslyn/issues/43697")]
public void DefiniteAssignment_04()
{
var text = @"
using System;
using System.Threading.Tasks;

#pragma warning disable CS1998 // This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

public class C
{
public async Task M()
{
bool a;
await M1();
Console.WriteLine(a);
async Task M1()
{
if ("""" == String.Empty)
{
throw new Exception();
}
else
{
a = true;
}
}
}
}
";
CreateCompilation(text).VerifyDiagnostics(
// (13,27): error CS0165: Use of unassigned local variable 'a'
// Console.WriteLine(a);
Diagnostic(ErrorCode.ERR_UseDefViolation, "a").WithArguments("a").WithLocation(13, 27)
);
}

[Fact, WorkItem(43697, "https://github.com/dotnet/roslyn/issues/43697")]
public void DefiniteAssignment_05()
{
var text = @"
using System;
using System.Threading.Tasks;

public class C
{
public async Task M()
{
bool a;
await M1();
Console.WriteLine(a);
async Task M1()
{
await Task.Yield();

if ("""" == String.Empty)
{
throw new Exception();
}
else
{
a = true;
}
}
}
}
";
CreateCompilation(text).VerifyDiagnostics(
// (11,27): error CS0165: Use of unassigned local variable 'a'
// Console.WriteLine(a);
Diagnostic(ErrorCode.ERR_UseDefViolation, "a").WithArguments("a").WithLocation(11, 27)
);
}
}
}